From f369dbb9e67eb5ef336944af63039b6d8f838384 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Thu, 12 Sep 2019 10:35:46 -0400 Subject: [PATCH 1/3] Ensure context is running prior to calling isc_app_ctxsuspend Add a release note. includes/omapip/isclib.h Added actx_running flag to global context, dhcp_gbl_ctx omapip/isclib.c set_ctx_running() - new function used as the ctxonrun callback dhcp_context_create() - installs set_ctx_running callback dhcp_signal_handler() - modified to use act_running flag to determine is context is running and should be suspended Upstream-Status: Backport [https://gitlab.isc.org/isc-projects/dhcp.git] Signed-off-by: Ovidiu Panait --- RELNOTES | 7 +++++ includes/omapip/isclib.h | 3 ++- omapip/isclib.c | 57 +++++++++++++++++++++++++++++++++------- 3 files changed, 57 insertions(+), 10 deletions(-) diff --git a/RELNOTES b/RELNOTES index f10305d..1730473 100644 --- a/RELNOTES +++ b/RELNOTES @@ -6,6 +6,13 @@ NEW FEATURES +- Closed a small window of time between the installation of graceful + shutdown signal handlers and application context startup, during which + the receipt of shutdown signal would cause a REQUIRE() assertion to + occur. Note this issue is only visible when compiling with + ENABLE_GENTLE_SHUTDOWN defined. + [Gitlab #53,!18 git TBD] + Please note that that ISC DHCP is now licensed under the Mozilla Public License, MPL 2.0. Please see https://www.mozilla.org/en-US/MPL/2.0/ to read the MPL 2.0 license terms. diff --git a/includes/omapip/isclib.h b/includes/omapip/isclib.h index 6c20584..af6a6fc 100644 --- a/includes/omapip/isclib.h +++ b/includes/omapip/isclib.h @@ -94,7 +94,8 @@ typedef struct dhcp_context { isc_mem_t *mctx; isc_appctx_t *actx; - int actx_started; + int actx_started; // ISC_TRUE if ctxstart has been called + int actx_running; // ISC_TRUE if ctxrun has been called isc_taskmgr_t *taskmgr; isc_task_t *task; isc_socketmgr_t *socketmgr; diff --git a/omapip/isclib.c b/omapip/isclib.c index ce4b4a1..73e017c 100644 --- a/omapip/isclib.c +++ b/omapip/isclib.c @@ -134,6 +134,35 @@ handle_signal(int sig, void (*handler)(int)) { } } +/* Callback passed to isc_app_ctxonrun + * + * BIND9 context code will invoke this handler once the context has + * entered the running state. We use it to set a global marker so that + * we can tell if the context is running. Several of the isc_app_ + * calls REQUIRE that the context is running and we need a way to + * know that. + * + * We also check to see if we received a shutdown signal prior to + * the context entering the run state. If we did, then we can just + * simply shut the context down now. This closes the relatively + * small window between start up and entering run via the call + * to dispatch(). + * + */ +static void +set_ctx_running(isc_task_t *task, isc_event_t *event) { + task = task; // unused; + dhcp_gbl_ctx.actx_running = ISC_TRUE; + + if (shutdown_signal) { + // We got signaled shutdown before we entered running state. + // Now that we've reached running state, shut'er down. + isc_app_ctxsuspend(dhcp_gbl_ctx.actx); + } + + isc_event_free(&event); +} + isc_result_t dhcp_context_create(int flags, struct in_addr *local4, @@ -141,6 +170,9 @@ dhcp_context_create(int flags, isc_result_t result; if ((flags & DHCP_CONTEXT_PRE_DB) != 0) { + dhcp_gbl_ctx.actx_started = ISC_FALSE; + dhcp_gbl_ctx.actx_running = ISC_FALSE; + /* * Set up the error messages, this isn't the right place * for this call but it is convienent for now. @@ -204,15 +236,24 @@ dhcp_context_create(int flags, if (result != ISC_R_SUCCESS) goto cleanup; - result = isc_task_create(dhcp_gbl_ctx.taskmgr, 0, &dhcp_gbl_ctx.task); + result = isc_task_create(dhcp_gbl_ctx.taskmgr, 0, + &dhcp_gbl_ctx.task); if (result != ISC_R_SUCCESS) goto cleanup; result = isc_app_ctxstart(dhcp_gbl_ctx.actx); if (result != ISC_R_SUCCESS) - return (result); + goto cleanup; + dhcp_gbl_ctx.actx_started = ISC_TRUE; + // Install the onrun callback. + result = isc_app_ctxonrun(dhcp_gbl_ctx.actx, dhcp_gbl_ctx.mctx, + dhcp_gbl_ctx.task, set_ctx_running, + dhcp_gbl_ctx.actx); + if (result != ISC_R_SUCCESS) + goto cleanup; + /* Not all OSs support suppressing SIGPIPE through socket * options, so set the sigal action to be ignore. This allows * broken connections to fail gracefully with EPIPE on writes */ @@ -335,19 +376,17 @@ isclib_make_dst_key(char *inname, * @param signal signal code that we received */ void dhcp_signal_handler(int signal) { - isc_appctx_t *ctx = dhcp_gbl_ctx.actx; - int prev = shutdown_signal; - - if (prev != 0) { + if (shutdown_signal != 0) { /* Already in shutdown. */ return; } + /* Possible race but does it matter? */ shutdown_signal = signal; - /* Use reload (aka suspend) for easier dispatch() reenter. */ - if (ctx && ctx->methods && ctx->methods->ctxsuspend) { - (void) isc_app_ctxsuspend(ctx); + /* If the application context is running tell it to shut down */ + if (dhcp_gbl_ctx.actx_running == ISC_TRUE) { + (void) isc_app_ctxsuspend(dhcp_gbl_ctx.actx); } } -- 2.23.0