From f56b8d58d3e32662517e0637ac54c548ed50b98e Mon Sep 17 00:00:00 2001 From: William Brown Date: Jun 23 2017 01:14:07 +0000 Subject: Ticket 49289 - Improve result handling from connections with NS Bug Description: In debug builds with replication it was noticed that we aren't closing connections at shutdown. This has always been the case but now with nunc-stans we are seeing it. To make it worse, in a debug build this would trigger an assertion that caused the server to abrt(). Fix Description: Change the result return type from nunc-stans from simple "true false",to an enum, that allows us to detect more conditions and make informed decisions as a caller. In the future we need to close connections at shutdown, but this requires much more work around the connection process and has been delayed for 1.4.0. See also: https://pagure.io/389-ds-base/issue/48184 https://pagure.io/389-ds-base/issue/49289 Author: wibrown Review by: tbordaz (Thanks!) --- diff --git a/ldap/servers/slapd/daemon.c b/ldap/servers/slapd/daemon.c index e63c26a..d0a4c6e 100644 --- a/ldap/servers/slapd/daemon.c +++ b/ldap/servers/slapd/daemon.c @@ -1084,8 +1084,11 @@ void slapd_daemon( daemon_ports_t *ports, ns_thrpool_t *tp ) setup_pr_read_pds(the_connection_table,n_tcps,s_tcps,i_unix,&num_poll); for (size_t ii = 0; ii < listeners; ++ii) { listener_idxs[ii].ct = the_connection_table; /* to pass to handle_new_connection */ - ns_add_io_job(tp, listener_idxs[ii].listenfd, NS_JOB_ACCEPT|NS_JOB_PERSIST|NS_JOB_PRESERVE_FD, + ns_result_t result = ns_add_io_job(tp, listener_idxs[ii].listenfd, NS_JOB_ACCEPT|NS_JOB_PERSIST|NS_JOB_PRESERVE_FD, ns_handle_new_connection, &listener_idxs[ii], &(listener_idxs[ii].ns_job)); + if (result != NS_SUCCESS) { + slapi_log_err(SLAPI_LOG_CRIT, "slapd_daemon", "ns_add_io_job failed to create add acceptor %d\n", result); + } } } @@ -1775,14 +1778,16 @@ ns_connection_post_io_or_closing(Connection *conn) tv.tv_usec = slapd_wakeup_timer * 1000; conn->c_ns_close_jobs++; /* now 1 active closure job */ connection_acquire_nolock_ext(conn, 1 /* allow acquire even when closing */); /* event framework now has a reference */ - PRStatus job_result = ns_add_timeout_job(conn->c_tp, &tv, NS_JOB_TIMER, + ns_result_t job_result = ns_add_timeout_job(conn->c_tp, &tv, NS_JOB_TIMER, ns_handle_closure, conn, NULL); -#ifdef DEBUG - PR_ASSERT(job_result == PR_SUCCESS); -#endif - if (job_result != PR_SUCCESS) { - slapi_log_err(SLAPI_LOG_WARNING, "ns_connection_post_io_or_closing", "post closure job " - "for conn %" PRIu64 " for fd=%d failed to be added to event queue\n", conn->c_connid, conn->c_sd); + if (job_result != NS_SUCCESS) { + if (job_result == NS_SHUTDOWN) { + slapi_log_err(SLAPI_LOG_INFO, "ns_connection_post_io_or_closing", "post closure job " + "for conn %" PRIu64 " for fd=%d failed to be added to event queue as server is shutting down\n", conn->c_connid, conn->c_sd); + } else { + slapi_log_err(SLAPI_LOG_ERR, "ns_connection_post_io_or_closing", "post closure job " + "for conn %" PRIu64 " for fd=%d failed to be added to event queue %d\n", conn->c_connid, conn->c_sd, job_result); + } } else { slapi_log_err(SLAPI_LOG_CONNS, "ns_connection_post_io_or_closing", "post closure job " "for conn %" PRIu64 " for fd=%d\n", conn->c_connid, conn->c_sd); @@ -1814,18 +1819,20 @@ ns_connection_post_io_or_closing(Connection *conn) return; } #endif - PRStatus job_result = ns_add_io_timeout_job(conn->c_tp, conn->c_prfd, &tv, + ns_result_t job_result = ns_add_io_timeout_job(conn->c_tp, conn->c_prfd, &tv, NS_JOB_READ|NS_JOB_PRESERVE_FD, ns_handle_pr_read_ready, conn, NULL); -#ifdef DEBUG - PR_ASSERT(job_result == PR_SUCCESS); -#endif - if (job_result != PR_SUCCESS) { - slapi_log_err(SLAPI_LOG_WARNING, "ns_connection_post_io_or_closing", "post I/O job for " - "conn %" PRIu64 " for fd=%d failed to be added to event queue\n", conn->c_connid, conn->c_sd); + if (job_result != NS_SUCCESS) { + if (job_result == NS_SHUTDOWN) { + slapi_log_err(SLAPI_LOG_INFO, "ns_connection_post_io_or_closing", "post I/O job for " + "conn %" PRIu64 " for fd=%d failed to be added to event queue as server is shutting down\n", conn->c_connid, conn->c_sd); + } else { + slapi_log_err(SLAPI_LOG_ERR, "ns_connection_post_io_or_closing", "post I/O job for " + "conn %" PRIu64 " for fd=%d failed to be added to event queue %d\n", conn->c_connid, conn->c_sd, job_result); + } } else { slapi_log_err(SLAPI_LOG_CONNS, "ns_connection_post_io_or_closing", "post I/O job for " - "conn %" PRIu64 " for fd=%d\n", conn->c_connid, conn->c_sd); + "conn %" PRIu64 " for fd=%d added to event queue\n", conn->c_connid, conn->c_sd); } } } diff --git a/src/nunc-stans/include/nunc-stans.h b/src/nunc-stans/include/nunc-stans.h index 7ce0582..6e82849 100644 --- a/src/nunc-stans/include/nunc-stans.h +++ b/src/nunc-stans/include/nunc-stans.h @@ -46,6 +46,40 @@ #include "nspr.h" /** + * ns_result_t encapsulates the set of results that can occur when interacting + * with nunc_stans. This is better than a simple status, because we can indicate + * why a failure occured, and allow the caller to handle it gracefully. + */ +typedef enum _ns_result_t { + /** + * Indicate the operation succeded. + */ + NS_SUCCESS = 0, + /** + * indicate that the event loop is shutting down, so we may reject operations. + */ + NS_SHUTDOWN = 1, + /** + * We failed to allocate resources as needed. + */ + NS_ALLOCATION_FAILURE = 2, + /** + * An invalid request was made to the API, you should probably check your + * call. + */ + NS_INVALID_REQUEST = 3, + /** + * You made a request against a job that would violate the safety of the job + * state machine. + */ + NS_INVALID_STATE = 4, + /** + * This occurs when a lower level OS issue occurs, generally thread related. + */ + NS_THREAD_FAILURE = 5, +} ns_result_t; + +/** * Forward declaration of the thread pool struct * * The actual struct is opaque to applications. The forward declaration is here @@ -417,12 +451,13 @@ void ns_thrpool_config_init(struct ns_thrpool_config *tp_config); * } * \endcode * \param job the job to clean up - * \retval PR_SUCCESS Job was successfully queued for removal. - * \retval PR_FAILURE Failed to mark job for removal. Likely the job is ARMED! + * \retval NS_SUCCESS Job was successfully queued for removal. + * \retval NS_INVALID_STATE Failed to mark job for removal. Likely the job is ARMED! * We cannot remove jobs that are armed due to the race conditions it can cause. + * \retval NS_INVALID_REQUEST No job was provided to remove (IE NULL request) * \sa ns_job_t, ns_job_get_data, NS_JOB_PERSIST, NS_JOB_PRESERVE_FD */ -PRStatus ns_job_done(struct ns_job_t *job); +ns_result_t ns_job_done(struct ns_job_t *job); /** * Create a new job which is not yet armed. @@ -448,13 +483,16 @@ PRStatus ns_job_done(struct ns_job_t *job); * \param job_type A set of flags that indicates the job type. * \param func The callback function to call when processing the job. * \param[out] job The address of a job pointer that will be filled in once the job is allocated. - * \retval PR_SUCCESS Job was successfully added. - * \retval PR_FAILURE Failed to add job. + * \retval NS_SUCCESS Job was successfully added. + * \retval NS_ALLOCATION_FAILURE Failed to allocate job. + * \retval NS_INVALID_REQUEST As create job does not create armed, if you make a request + * with a NULL job parameter, this would create a memory leak. As a result, we fail if + * the request is NULL. * \warning The thread pool will not allow a job to be added when it has been signaled * to shutdown. It will return PR_FAILURE in that case. * \sa ns_job_t, ns_job_get_data, NS_JOB_READ, NS_JOB_WRITE, NS_JOB_ACCEPT, NS_JOB_CONNECT, NS_JOB_IS_IO, ns_job_done */ -PRStatus +ns_result_t ns_create_job(struct ns_thrpool_t *tp, ns_job_type_t job_type, ns_job_func_t func, struct ns_job_t **job); /** @@ -486,13 +524,15 @@ ns_create_job(struct ns_thrpool_t *tp, ns_job_type_t job_type, ns_job_func_t fun * \param data Arbitrary data that will be available to the job callback function. * \param[out] job The address of a job pointer that will be filled in once the job is allocated. * \c NULL can be passed if a pointer to the job is not needed. - * \retval PR_SUCCESS Job was successfully added. - * \retval PR_FAILURE Failed to add job. + * \retval NS_SUCCESS Job was successfully added. + * \retval NS_ALLOCATION_FAILURE Failed to allocate job. + * \retval NS_INVALID_REQUEST An invalid job request was made: likely you asked for an + * accept job to be threaded, which is currently invalid. * \warning The thread pool will not allow a job to be added when it has been signaled * to shutdown. It will return PR_FAILURE in that case. * \sa ns_job_t, ns_job_get_data, NS_JOB_READ, NS_JOB_WRITE, NS_JOB_ACCEPT, NS_JOB_CONNECT, NS_JOB_IS_IO, ns_job_done */ -PRStatus ns_add_io_job(struct ns_thrpool_t *tp, +ns_result_t ns_add_io_job(struct ns_thrpool_t *tp, PRFileDesc *fd, ns_job_type_t job_type, ns_job_func_t func, @@ -511,13 +551,15 @@ PRStatus ns_add_io_job(struct ns_thrpool_t *tp, * \param data Arbitrary data that will be available to the job callback function. * \param[out] job The address of a job pointer that will be filled in once the job is allocated. * \c NULL can be passed if a pointer to the job is not needed. - * \retval PR_SUCCESS Job was successfully added. - * \retval PR_FAILURE Failed to add job. + * \retval NS_SUCCESS Job was successfully added. + * \retval NS_ALLOCATION_FAILURE Failed to allocate job. + * \retval NS_INVALID_REQUEST An invalid job request was made: likely you asked for a + * timeout that is not valid (negative integer). * \warning The thread pool will not allow a job to be added when it has been signaled * to shutdown. It will return PR_FAILURE in that case. * \sa ns_job_t, ns_job_get_data, NS_JOB_TIMER, NS_JOB_IS_TIMER, ns_job_done */ -PRStatus ns_add_timeout_job(struct ns_thrpool_t *tp, +ns_result_t ns_add_timeout_job(struct ns_thrpool_t *tp, struct timeval *tv, ns_job_type_t job_type, ns_job_func_t func, @@ -559,13 +601,16 @@ PRStatus ns_add_timeout_job(struct ns_thrpool_t *tp, * \param data Arbitrary data that will be available to the job callback function. * \param[out] job The address of a job pointer that will be filled in once the job is allocated. * \c NULL can be passed if a pointer to the job is not needed. - * \retval PR_SUCCESS Job was successfully added. - * \retval PR_FAILURE Failed to add job. + * \retval NS_SUCCESS Job was successfully added. + * \retval NS_ALLOCATION_FAILURE Failed to allocate job. + * \retval NS_INVALID_REQUEST An invalid job request was made: likely you asked for a + * timeout that is not valid (negative integer). Another failure is you requested a + * threaded accept job. * \warning The thread pool will not allow a job to be added when it has been signaled - * to shutdown. It will return PR_FAILURE in that case. + * to shutdown. It will return NS_SHUTDOWN in that case. * \sa ns_job_t, ns_job_get_data, NS_JOB_READ, NS_JOB_WRITE, NS_JOB_ACCEPT, NS_JOB_CONNECT, NS_JOB_IS_IO, ns_job_done, NS_JOB_TIMER, NS_JOB_IS_TIMER */ -PRStatus ns_add_io_timeout_job(struct ns_thrpool_t *tp, +ns_result_t ns_add_io_timeout_job(struct ns_thrpool_t *tp, PRFileDesc *fd, struct timeval *tv, ns_job_type_t job_type, @@ -585,14 +630,14 @@ PRStatus ns_add_io_timeout_job(struct ns_thrpool_t *tp, * \param data Arbitrary data that will be available to the job callback function. * \param[out] job The address of a job pointer that will be filled in once the job is allocated. * \c NULL can be passed if a pointer to the job is not needed. - * \retval PR_SUCCESS Job was successfully added. - * \retval PR_FAILURE Failed to add job. + * \retval NS_SUCCESS Job was successfully added. + * \retval NS_ALLOCATION_FAILURE Failed to allocate job. * \warning The thread pool will not allow a job to be added when it has been signaled * to shutdown. It will return PR_FAILURE in that case. * \sa ns_job_t, ns_job_get_data, NS_JOB_SIGNAL, NS_JOB_IS_SIGNAL */ -PRStatus ns_add_signal_job(ns_thrpool_t *tp, - PRInt32 signum, +ns_result_t ns_add_signal_job(ns_thrpool_t *tp, + int32_t signum, ns_job_type_t job_type, ns_job_func_t func, void *data, @@ -619,13 +664,13 @@ PRStatus ns_add_signal_job(ns_thrpool_t *tp, * \param data Arbitrary data that will be available to the job callback function. * \param[out] job The address of a job pointer that will be filled in once the job is allocated. * \c NULL can be passed if a pointer to the job is not needed. - * \retval PR_SUCCESS Job was successfully added. - * \retval PR_FAILURE Failed to add job. + * \retval NS_SUCCESS Job was successfully added. + * \retval NS_ALLOCATION_FAILURE Failed to allocate job. * \warning The thread pool will not allow a job to be added when it has been signaled * to shutdown. It will return PR_FAILURE in that case. * \sa ns_job_t, ns_job_get_data, NS_JOB_NONE, NS_JOB_THREAD */ -PRStatus ns_add_job(ns_thrpool_t *tp, ns_job_type_t job_type, ns_job_func_t func, void *data, struct ns_job_t **job); +ns_result_t ns_add_job(ns_thrpool_t *tp, ns_job_type_t job_type, ns_job_func_t func, void *data, struct ns_job_t **job); /** * Allows the callback to access the file descriptor for an I/O job @@ -697,10 +742,11 @@ void *ns_job_get_data(struct ns_job_t *job); * * \param job The job to set the data for * \param data The void * pointer to the data to set - * \return if the set data succeeded + * \retval NS_SUCCESS Job was modified correctly. + * \retval NS_INVALID_STATE Failed to modify the job as this may be unsafe. * \sa ns_job_t */ -PRStatus ns_job_set_data(struct ns_job_t *job, void *data); +ns_result_t ns_job_set_data(struct ns_job_t *job, void *data); /** * Allows the callback to access the job type flags. @@ -781,12 +827,13 @@ ns_job_type_t ns_job_get_output_type(struct ns_job_t *job); * free(ns_job_get_data(job)); * } * \endcode - * \return status if the set succeeded. Always check this! * \param job The job to set the callback for. * \param func The callback function, to be called when ns_job_done is triggered. + * \retval NS_SUCCESS Job was modified correctly. + * \retval NS_INVALID_STATE Failed to modify the job as this may be unsafe. * \sa ns_job_t, ns_job_done */ -PRStatus ns_job_set_done_cb(struct ns_job_t *job, ns_job_func_t func); +ns_result_t ns_job_set_done_cb(struct ns_job_t *job, ns_job_func_t func); /** * Creates a new thread pool @@ -858,7 +905,7 @@ void ns_thrpool_shutdown(struct ns_thrpool_t *tp); * \retval 1 if the thread pool is shutting down. * \sa ns_thrpool_shutdown */ -PRInt32 ns_thrpool_is_shutdown(struct ns_thrpool_t *tp); +int32_t ns_thrpool_is_shutdown(struct ns_thrpool_t *tp); /** * Waits for all threads in the pool to exit @@ -872,11 +919,11 @@ PRInt32 ns_thrpool_is_shutdown(struct ns_thrpool_t *tp); * never return. * * \param tp The thread pool to wait for. - * \retval PR_SUCCESS The thread pool threads completed successfully - * \retval PR_FAILURE Failure waiting for the thread pool threads to terminate + * \retval NS_SUCCESS The thread pool threads completed successfully + * \retval NS_THREAD_FAILURE Failure waiting for a thread to rejoin. * \sa ns_thrpool_config, ns_thrpool_config_init, ns_thrpool_new, ns_thrpool_destroy, ns_thrpool_shutdown */ -PRStatus ns_thrpool_wait(struct ns_thrpool_t *tp); +ns_result_t ns_thrpool_wait(struct ns_thrpool_t *tp); /** @@ -890,8 +937,13 @@ PRStatus ns_thrpool_wait(struct ns_thrpool_t *tp); * refer to job after calling ns_job_rearm(). * \note Do not call ns_job_done() with a job if using ns_job_rearm() with the job * \param job The job to re-arm + * \retval NS_SUCCESS The job was queued correctly. + * \retval NS_SHUTDOWN The job was not able to be queued as the server is in the procees + * of shutting down. + * \retval NS_INVALID_STATE The job was not able to be queued as it is in an invalid state + * \retval NS_INVALID_REQUEST The job to be queued is invalid. * \sa ns_job_t, ns_job_done, NS_JOB_PERSIST, NS_JOB_THREAD */ -PRStatus ns_job_rearm(struct ns_job_t *job); +ns_result_t ns_job_rearm(struct ns_job_t *job); #endif /* NS_THRPOOL_H */ diff --git a/src/nunc-stans/ns/ns_event_fw_event.c b/src/nunc-stans/ns/ns_event_fw_event.c index 76936de..98cd4c5 100644 --- a/src/nunc-stans/ns/ns_event_fw_event.c +++ b/src/nunc-stans/ns/ns_event_fw_event.c @@ -142,9 +142,9 @@ ns_event_fw_init(void) } static void -ns_event_fw_destroy(ns_event_fw_ctx_t *ns_event_fw_ctx_t) +ns_event_fw_destroy(ns_event_fw_ctx_t *event_ctx_t) { - event_base_free(ns_event_fw_ctx_t); + event_base_free(event_ctx_t); } /* diff --git a/src/nunc-stans/ns/ns_thrpool.c b/src/nunc-stans/ns/ns_thrpool.c index 435aeb9..8713735 100644 --- a/src/nunc-stans/ns/ns_thrpool.c +++ b/src/nunc-stans/ns/ns_thrpool.c @@ -726,12 +726,12 @@ alloc_signal_context(ns_thrpool_t *tp, PRInt32 signum, ns_job_type_t job_type, return job; } -PRStatus +ns_result_t ns_job_done(ns_job_t *job) { PR_ASSERT(job); if (job == NULL) { - return PR_FAILURE; + return NS_INVALID_REQUEST; } /* Get the shutdown state ONCE at the start, atomically */ @@ -745,7 +745,7 @@ ns_job_done(ns_job_t *job) ns_log(LOG_DEBUG, "ns_job_done %x tp shutdown -> %x state %d return early\n", job, shutdown_state, job->state); #endif pthread_mutex_unlock(job->monitor); - return PR_SUCCESS; + return NS_SUCCESS; } /* Do not allow an armed job to be removed UNLESS the server is shutting down */ @@ -754,7 +754,7 @@ ns_job_done(ns_job_t *job) ns_log(LOG_DEBUG, "ns_job_done %x tp shutdown -> false state %d failed to mark as done\n", job, job->state); #endif pthread_mutex_unlock(job->monitor); - return PR_FAILURE; + return NS_INVALID_STATE; } if (job->state == NS_JOB_RUNNING || job->state == NS_JOB_NEEDS_ARM) { @@ -781,31 +781,31 @@ ns_job_done(ns_job_t *job) pthread_mutex_unlock(job->monitor); internal_ns_job_done(job); } - return PR_SUCCESS; + return NS_SUCCESS; } -PRStatus +ns_result_t ns_create_job(struct ns_thrpool_t *tp, ns_job_type_t job_type, ns_job_func_t func, struct ns_job_t **job) { if (job == NULL) { /* This won't queue the job, so to pass NULL makes no sense */ - return PR_FAILURE; + return NS_INVALID_REQUEST; } if (ns_thrpool_is_shutdown(tp)) { - return PR_FAILURE; + return NS_SHUTDOWN; } *job = new_ns_job(tp, NULL, job_type, func, NULL); if (*job == NULL) { - return PR_FAILURE; + return NS_ALLOCATION_FAILURE; } - return PR_SUCCESS; + return NS_SUCCESS; } /* queue a file descriptor to listen for and accept new connections */ -PRStatus +ns_result_t ns_add_io_job(ns_thrpool_t *tp, PRFileDesc *fd, ns_job_type_t job_type, ns_job_func_t func, void *data, ns_job_t **job) { @@ -817,7 +817,7 @@ ns_add_io_job(ns_thrpool_t *tp, PRFileDesc *fd, ns_job_type_t job_type, /* Don't allow a job to be added if the threadpool is being shut down. */ if (ns_thrpool_is_shutdown(tp)) { - return PR_FAILURE; + return NS_SHUTDOWN; } /* Don't allow an accept job to be run outside of the event thread. @@ -832,13 +832,13 @@ ns_add_io_job(ns_thrpool_t *tp, PRFileDesc *fd, ns_job_type_t job_type, * */ if (NS_JOB_IS_ACCEPT(job_type) && NS_JOB_IS_THREAD(job_type)) { - return PR_FAILURE; + return NS_INVALID_REQUEST; } /* get an event context for an accept */ _job = alloc_io_context(tp, fd, job_type, func, data); if (!_job) { - return PR_FAILURE; + return NS_ALLOCATION_FAILURE; } pthread_mutex_lock(_job->monitor); @@ -854,10 +854,10 @@ ns_add_io_job(ns_thrpool_t *tp, PRFileDesc *fd, ns_job_type_t job_type, *job = _job; } - return PR_SUCCESS; + return NS_SUCCESS; } -PRStatus +ns_result_t ns_add_timeout_job(ns_thrpool_t *tp, struct timeval *tv, ns_job_type_t job_type, ns_job_func_t func, void *data, ns_job_t **job) { @@ -869,17 +869,17 @@ ns_add_timeout_job(ns_thrpool_t *tp, struct timeval *tv, ns_job_type_t job_type, /* Don't allow a job to be added if the threadpool is being shut down. */ if (ns_thrpool_is_shutdown(tp)) { - return PR_FAILURE; + return NS_SHUTDOWN; } if (validate_event_timeout(tv)) { - return PR_FAILURE; + return NS_INVALID_REQUEST; } /* get an event context for a timer job */ _job = alloc_timeout_context(tp, tv, job_type, func, data); if (!_job) { - return PR_FAILURE; + return NS_ALLOCATION_FAILURE; } pthread_mutex_lock(_job->monitor); @@ -895,11 +895,11 @@ ns_add_timeout_job(ns_thrpool_t *tp, struct timeval *tv, ns_job_type_t job_type, *job = _job; } - return PR_SUCCESS; + return NS_SUCCESS; } /* queue a file descriptor to listen for and accept new connections */ -PRStatus +ns_result_t ns_add_io_timeout_job(ns_thrpool_t *tp, PRFileDesc *fd, struct timeval *tv, ns_job_type_t job_type, ns_job_func_t func, void *data, ns_job_t **job) { @@ -911,11 +911,11 @@ ns_add_io_timeout_job(ns_thrpool_t *tp, PRFileDesc *fd, struct timeval *tv, /* Don't allow a job to be added if the threadpool is being shut down. */ if (ns_thrpool_is_shutdown(tp)) { - return PR_FAILURE; + return NS_SHUTDOWN; } if (validate_event_timeout(tv)) { - return PR_FAILURE; + return NS_INVALID_REQUEST; } /* Don't allow an accept job to be run outside of the event thread. @@ -930,13 +930,13 @@ ns_add_io_timeout_job(ns_thrpool_t *tp, PRFileDesc *fd, struct timeval *tv, * */ if (NS_JOB_IS_ACCEPT(job_type) && NS_JOB_IS_THREAD(job_type)) { - return PR_FAILURE; + return NS_INVALID_REQUEST; } /* get an event context for an accept */ _job = alloc_io_context(tp, fd, job_type|NS_JOB_TIMER, func, data); if (!_job) { - return PR_FAILURE; + return NS_ALLOCATION_FAILURE; } pthread_mutex_lock(_job->monitor); _job->tv = *tv; @@ -953,11 +953,11 @@ ns_add_io_timeout_job(ns_thrpool_t *tp, PRFileDesc *fd, struct timeval *tv, *job = _job; } - return PR_SUCCESS; + return NS_SUCCESS; } -PRStatus -ns_add_signal_job(ns_thrpool_t *tp, PRInt32 signum, ns_job_type_t job_type, +ns_result_t +ns_add_signal_job(ns_thrpool_t *tp, int32_t signum, ns_job_type_t job_type, ns_job_func_t func, void *data, ns_job_t **job) { ns_job_t *_job = NULL; @@ -968,13 +968,13 @@ ns_add_signal_job(ns_thrpool_t *tp, PRInt32 signum, ns_job_type_t job_type, /* Don't allow a job to be added if the threadpool is being shut down. */ if (ns_thrpool_is_shutdown(tp)) { - return PR_FAILURE; + return NS_SHUTDOWN; } /* get an event context for a signal job */ _job = alloc_signal_context(tp, signum, job_type, func, data); if (!_job) { - return PR_FAILURE; + return NS_ALLOCATION_FAILURE; } pthread_mutex_lock(_job->monitor); @@ -990,10 +990,10 @@ ns_add_signal_job(ns_thrpool_t *tp, PRInt32 signum, ns_job_type_t job_type, *job = _job; } - return PR_SUCCESS; + return NS_SUCCESS; } -PRStatus +ns_result_t ns_add_job(ns_thrpool_t *tp, ns_job_type_t job_type, ns_job_func_t func, void *data, ns_job_t **job) { ns_job_t *_job = NULL; @@ -1004,12 +1004,12 @@ ns_add_job(ns_thrpool_t *tp, ns_job_type_t job_type, ns_job_func_t func, void *d /* Don't allow a job to be added if the threadpool is being shut down. */ if (ns_thrpool_is_shutdown(tp)) { - return PR_FAILURE; + return NS_SHUTDOWN; } _job = new_ns_job(tp, NULL, job_type, func, data); if (!_job) { - return PR_FAILURE; + return NS_ALLOCATION_FAILURE; } /* fill in a pointer to the job for the caller if requested */ if (job) { @@ -1022,21 +1022,21 @@ ns_add_job(ns_thrpool_t *tp, ns_job_type_t job_type, ns_job_func_t func, void *d _job->state = NS_JOB_NEEDS_ARM; internal_ns_job_rearm(_job); - return PR_SUCCESS; + return NS_SUCCESS; } -PRStatus +ns_result_t ns_add_shutdown_job(ns_thrpool_t *tp) { ns_job_t *_job = NULL; _job = new_ns_job(tp, NULL, NS_JOB_SHUTDOWN_WORKER, NULL, NULL); if (!_job) { - return PR_FAILURE; + return NS_ALLOCATION_FAILURE; } pthread_mutex_lock(_job->monitor); _job->state = NS_JOB_NEEDS_ARM; pthread_mutex_unlock(_job->monitor); internal_ns_job_rearm(_job); - return PR_SUCCESS; + return NS_SUCCESS; } /* @@ -1066,7 +1066,7 @@ ns_job_get_data(ns_job_t *job) } } -PRStatus +ns_result_t ns_job_set_data(ns_job_t *job, void *data) { PR_ASSERT(job); @@ -1075,10 +1075,10 @@ ns_job_set_data(ns_job_t *job, void *data) if (job->state == NS_JOB_WAITING || job->state == NS_JOB_RUNNING ) { job->data = data; pthread_mutex_unlock(job->monitor); - return PR_SUCCESS; + return NS_SUCCESS; } else { pthread_mutex_unlock(job->monitor); - return PR_FAILURE; + return NS_INVALID_STATE; } } @@ -1142,7 +1142,7 @@ ns_job_get_fd(ns_job_t *job) } } -PRStatus +ns_result_t ns_job_set_done_cb(struct ns_job_t *job, ns_job_func_t func) { PR_ASSERT(job); @@ -1151,10 +1151,10 @@ ns_job_set_done_cb(struct ns_job_t *job, ns_job_func_t func) if (job->state == NS_JOB_WAITING || job->state == NS_JOB_RUNNING ) { job->done_cb = func; pthread_mutex_unlock(job->monitor); - return PR_SUCCESS; + return NS_SUCCESS; } else { pthread_mutex_unlock(job->monitor); - return PR_FAILURE; + return NS_INVALID_STATE; } } @@ -1163,7 +1163,7 @@ ns_job_set_done_cb(struct ns_job_t *job, ns_job_func_t func) * This is a convenience function - use if you need to re-arm the same event * usually not needed for persistent jobs */ -PRStatus +ns_result_t ns_job_rearm(ns_job_t *job) { PR_ASSERT(job); @@ -1171,7 +1171,7 @@ ns_job_rearm(ns_job_t *job) PR_ASSERT(job->state == NS_JOB_WAITING || job->state == NS_JOB_RUNNING); if (ns_thrpool_is_shutdown(job->tp)) { - return PR_FAILURE; + return NS_SHUTDOWN; } if (job->state == NS_JOB_WAITING) { @@ -1181,7 +1181,7 @@ ns_job_rearm(ns_job_t *job) job->state = NS_JOB_NEEDS_ARM; internal_ns_job_rearm(job); pthread_mutex_unlock(job->monitor); - return PR_SUCCESS; + return NS_SUCCESS; } else if ( !NS_JOB_IS_PERSIST(job->job_type) && job->state == NS_JOB_RUNNING) { /* For this to be called, and NS_JOB_RUNNING, we *must* be the callback thread! */ /* Just mark it (ie do nothing), the work_job_execute function will trigger internal_ns_job_rearm */ @@ -1190,13 +1190,13 @@ ns_job_rearm(ns_job_t *job) #endif job->state = NS_JOB_NEEDS_ARM; pthread_mutex_unlock(job->monitor); - return PR_SUCCESS; + return NS_SUCCESS; } else { pthread_mutex_unlock(job->monitor); - return PR_FAILURE; + return NS_INVALID_STATE; } /* Unreachable code .... */ - return PR_FAILURE; + return NS_INVALID_REQUEST; } static void @@ -1286,12 +1286,12 @@ ns_thrpool_config_init(struct ns_thrpool_config *tp_config) /* * Process the config and set the pluggable function pointers */ -static int +static ns_result_t ns_thrpool_process_config(struct ns_thrpool_config *tp_config) { /* Check that the config has been properly initialized */ if (!tp_config || tp_config->init_flag != NS_INIT_MAGIC){ - return -1; + return NS_INVALID_REQUEST; } /* * Assign our logging function pointers @@ -1351,7 +1351,7 @@ ns_thrpool_process_config(struct ns_thrpool_config *tp_config) free_fct = os_free; } - return 0; + return NS_SUCCESS; } ns_thrpool_t * @@ -1362,7 +1362,7 @@ ns_thrpool_new(struct ns_thrpool_config *tp_config) ns_thread_t *thr; size_t ii; - if(ns_thrpool_process_config(tp_config) == -1){ + if(ns_thrpool_process_config(tp_config) != NS_SUCCESS){ ns_log(LOG_ERR, "ns_thrpool_new(): config has not been properly initialized\n"); goto failed; } @@ -1540,8 +1540,8 @@ ns_thrpool_shutdown(struct ns_thrpool_t *tp) * currently queued jobs to complete. */ for (size_t i = 0; i < tp->thread_count; i++) { - PRStatus result = ns_add_shutdown_job(tp); - PR_ASSERT(result == PR_SUCCESS); + ns_result_t result = ns_add_shutdown_job(tp); + PR_ASSERT(result == NS_SUCCESS); } /* Make sure all threads are woken up to their shutdown jobs. */ pthread_mutex_lock(&(tp->work_q_lock)); @@ -1549,13 +1549,13 @@ ns_thrpool_shutdown(struct ns_thrpool_t *tp) pthread_mutex_unlock(&(tp->work_q_lock)); } -PRStatus +ns_result_t ns_thrpool_wait(ns_thrpool_t *tp) { #ifdef DEBUG ns_log(LOG_DEBUG, "ns_thrpool_wait has begun\n"); #endif - PRStatus retval = PR_SUCCESS; + ns_result_t retval = NS_SUCCESS; ns_thread_t *thr; while (sds_queue_dequeue(tp->thread_stack, (void **)&thr) == SDS_SUCCESS) @@ -1568,7 +1568,7 @@ ns_thrpool_wait(ns_thrpool_t *tp) if (rc != 0) { /* NGK TODO - this is unused right now. */ ns_log(LOG_ERR, "ns_thrpool_wait, failed to join thread %d", rc); - retval = PR_FAILURE; + retval = NS_THREAD_FAILURE; } ns_free(thr); } diff --git a/src/nunc-stans/test/test_nuncstans.c b/src/nunc-stans/test/test_nuncstans.c index 0077457..b3a6ae6 100644 --- a/src/nunc-stans/test/test_nuncstans.c +++ b/src/nunc-stans/test/test_nuncstans.c @@ -123,7 +123,7 @@ ns_init_test_job_cb(struct ns_job_t *job __attribute__((unused))) static void ns_init_disarm_job_cb(struct ns_job_t *job) { - if (ns_job_done(job) == PR_SUCCESS) { + if (ns_job_done(job) == NS_SUCCESS) { cb_check = 1; } else { assert_int_equal(1,0); @@ -160,7 +160,7 @@ ns_init_test(void **state) /* Once the job is done, it's not in the event queue, and it's complete */ /* We have to stall momentarily to let the work_job_execute release the job to us */ PR_Sleep(PR_SecondsToInterval(1)); - assert_int_equal(ns_job_done(job), 0); + assert_int_equal(ns_job_done(job), NS_SUCCESS); } static void @@ -177,7 +177,7 @@ ns_set_data_test(void **state) PR_Lock(cb_lock); assert_int_equal( ns_add_job(tp, NS_JOB_NONE|NS_JOB_THREAD, ns_init_test_job_cb, data, &job), - 0); + NS_SUCCESS); /* Let the job run */ PR_WaitCondVar(cb_cond, PR_SecondsToInterval(1)); @@ -222,7 +222,7 @@ ns_set_data_test(void **state) PR_Sleep(PR_MillisecondsToInterval(50)); } - assert_int_equal(ns_job_done(job), 0); + assert_int_equal(ns_job_done(job), NS_SUCCESS); } static void @@ -234,11 +234,11 @@ ns_job_done_cb_test(void **state) PR_Lock(cb_lock); assert_int_equal( ns_create_job(tp, NS_JOB_NONE|NS_JOB_THREAD, ns_init_do_nothing_cb, &job), - 0); + NS_SUCCESS); ns_job_set_done_cb(job, ns_init_test_job_cb); /* Remove it */ - assert_int_equal(ns_job_done(job), 0); + assert_int_equal(ns_job_done(job), NS_SUCCESS); PR_WaitCondVar(cb_cond, PR_SecondsToInterval(1)); PR_Unlock(cb_lock); @@ -250,10 +250,10 @@ ns_job_done_cb_test(void **state) static void ns_init_rearm_job_cb(struct ns_job_t *job) { - if (ns_job_rearm(job) == PR_FAILURE) { + if (ns_job_rearm(job) != NS_SUCCESS) { cb_check = 1; /* we failed to re-arm as expected, let's go away ... */ - assert_int_equal(ns_job_done(job), 0); + assert_int_equal(ns_job_done(job), NS_SUCCESS); } else { assert_int_equal(1, 0); } @@ -273,7 +273,7 @@ ns_job_persist_rearm_ignore_test(void **state) PR_Lock(cb_lock); assert_int_equal( ns_create_job(tp, NS_JOB_NONE|NS_JOB_THREAD|NS_JOB_PERSIST, ns_init_rearm_job_cb, &job), - 0); + NS_SUCCESS); /* This *will* arm the job, and will trigger the cb. */ assert_int_equal(ns_job_rearm(job), 0); @@ -299,9 +299,9 @@ ns_job_persist_disarm_test(void **state) assert_int_equal( ns_create_job(tp, NS_JOB_NONE|NS_JOB_PERSIST, ns_init_disarm_job_cb, &job), - 0); + NS_SUCCESS); - assert_int_equal(ns_job_rearm(job), 0); + assert_int_equal(ns_job_rearm(job), NS_SUCCESS); /* In the callback it should disarm */ PR_Lock(cb_lock); @@ -350,7 +350,7 @@ ns_job_race_done_test(void **state) PR_Lock(cb_lock); assert_int_equal( ns_add_job(tp, NS_JOB_NONE|NS_JOB_THREAD, ns_init_race_done_job_cb, NULL, &job), - 0); + NS_SUCCESS); PR_WaitCondVar(cb_cond, PR_SecondsToInterval(5)); PR_Unlock(cb_lock); @@ -372,7 +372,7 @@ ns_job_signal_cb_test(void **state) PR_Lock(cb_lock); assert_int_equal( ns_add_signal_job(tp, SIGUSR1, NS_JOB_SIGNAL, ns_init_test_job_cb, NULL, &job), - 0); + NS_SUCCESS); /* The addition of the signal job to the event fw is async */ PR_Sleep(PR_SecondsToInterval(2)); @@ -385,7 +385,7 @@ ns_job_signal_cb_test(void **state) assert_int_equal(cb_check, 1); /* Remove the signal job now */ - assert_int_equal(ns_job_done(job), 0); + assert_int_equal(ns_job_done(job), NS_SUCCESS); } /* @@ -399,9 +399,9 @@ ns_job_neg_timeout_test(void **state) struct timeval tv = { -1, 0 }; - PR_ASSERT(PR_FAILURE == ns_add_io_timeout_job(tp, 0, &tv, NS_JOB_THREAD, ns_init_do_nothing_cb, NULL, NULL)); + PR_ASSERT(NS_INVALID_REQUEST == ns_add_io_timeout_job(tp, 0, &tv, NS_JOB_THREAD, ns_init_do_nothing_cb, NULL, NULL)); - PR_ASSERT(PR_FAILURE == ns_add_timeout_job(tp, &tv, NS_JOB_THREAD, ns_init_do_nothing_cb, NULL, NULL)); + PR_ASSERT(NS_INVALID_REQUEST == ns_add_timeout_job(tp, &tv, NS_JOB_THREAD, ns_init_do_nothing_cb, NULL, NULL)); } @@ -428,7 +428,7 @@ ns_job_timer_test(void **state) struct timeval tv = { 2, 0 }; PR_Lock(cb_lock); - assert_true(ns_add_timeout_job(tp, &tv, NS_JOB_THREAD, ns_timer_job_cb, NULL, &job) == PR_SUCCESS); + assert_true(ns_add_timeout_job(tp, &tv, NS_JOB_THREAD, ns_timer_job_cb, NULL, &job) == NS_SUCCESS); PR_WaitCondVar(cb_cond, PR_SecondsToInterval(1)); assert_int_equal(cb_check, 0); @@ -462,7 +462,7 @@ ns_job_timer_persist_test(void **state) struct timeval tv = { 1, 0 }; PR_Lock(cb_lock); - assert_true(ns_add_timeout_job(tp, &tv, NS_JOB_THREAD, ns_timer_persist_job_cb, NULL, &job) == PR_SUCCESS); + assert_true(ns_add_timeout_job(tp, &tv, NS_JOB_THREAD, ns_timer_persist_job_cb, NULL, &job) == NS_SUCCESS); PR_Sleep(PR_SecondsToInterval(5));