From dda1d9544cb8b078fd24a11b50fb54ade2a55677 Mon Sep 17 00:00:00 2001 From: "W.C.A. Wijngaards" Date: Tue, 8 Nov 2022 15:04:05 +0100 Subject: [PATCH] - Fix #775: libunbound: subprocess reap causes parent process reap to hang. --- doc/Changelog | 2 ++ libunbound/context.c | 1 + libunbound/context.h | 6 ++++++ libunbound/libunbound.c | 37 ++++++++++++++++++++++++++++++++++++- libunbound/libworker.c | 1 + 5 files changed, 46 insertions(+), 1 deletion(-) diff --git a/doc/Changelog b/doc/Changelog index 23209dabf..3c0a0a99a 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -1,6 +1,8 @@ 8 November 2022: Wouter - Fix to ignore tcp events for closed comm points. - Fix to make sure to not read again after a tcp comm point is closed. + - Fix #775: libunbound: subprocess reap causes parent process reap + to hang. 21 October 2022: George - Merge #767 from jonathangray: consistently use IPv4/IPv6 in diff --git a/libunbound/context.c b/libunbound/context.c index c8d911f13..f7c0a2cd5 100644 --- a/libunbound/context.c +++ b/libunbound/context.c @@ -70,6 +70,7 @@ context_finalize(struct ub_ctx* ctx) } else { log_init(cfg->logfile, cfg->use_syslog, NULL); } + ctx->pipe_pid = getpid(); cfg_apply_local_port_policy(cfg, 65536); config_apply(cfg); if(!modstack_setup(&ctx->mods, cfg->module_conf, ctx->env)) diff --git a/libunbound/context.h b/libunbound/context.h index c0c86fb52..c0fc80e57 100644 --- a/libunbound/context.h +++ b/libunbound/context.h @@ -89,6 +89,12 @@ struct ub_ctx { pid_t bg_pid; /** tid of bg worker thread */ ub_thread_type bg_tid; + /** pid when pipes are created. This was the process when the + * setup was called. Helps with clean up, so we can tell after a fork + * which side of the fork the delete is on. */ + pid_t pipe_pid; + /** when threaded, the worker that exists in the created thread. */ + struct libworker* thread_worker; /** do threading (instead of forking) for async resolution */ int dothread; diff --git a/libunbound/libunbound.c b/libunbound/libunbound.c index ea5ef24bb..225457e73 100644 --- a/libunbound/libunbound.c +++ b/libunbound/libunbound.c @@ -305,11 +305,29 @@ ub_ctx_delete(struct ub_ctx* ctx) int do_stop = 1; if(!ctx) return; + /* if the delete is called but it has forked, and before the fork + * the context was finalized, then the bg worker is not stopped + * from here. There is one worker, but two contexts that refer to + * it and only one should clean up, the one with getpid == pipe_pid.*/ + if(ctx->created_bg && ctx->pipe_pid != getpid()) { + do_stop = 0; + /* Stop events from getting deregistered, if the backend is + * epoll, the epoll fd is the same as the other process. + * That process should deregister them. */ + if(ctx->qq_pipe->listen_com) + ctx->qq_pipe->listen_com->event_added = 0; + if(ctx->qq_pipe->res_com) + ctx->qq_pipe->res_com->event_added = 0; + if(ctx->rr_pipe->listen_com) + ctx->rr_pipe->listen_com->event_added = 0; + if(ctx->rr_pipe->res_com) + ctx->rr_pipe->res_com->event_added = 0; + } /* see if bg thread is created and if threads have been killed */ /* no locks, because those may be held by terminated threads */ /* for processes the read pipe is closed and we see that on read */ #ifdef HAVE_PTHREAD - if(ctx->created_bg && ctx->dothread) { + if(ctx->created_bg && ctx->dothread && do_stop) { if(pthread_kill(ctx->bg_tid, 0) == ESRCH) { /* thread has been killed */ do_stop = 0; @@ -318,6 +336,23 @@ ub_ctx_delete(struct ub_ctx* ctx) #endif /* HAVE_PTHREAD */ if(do_stop) ub_stop_bg(ctx); + if(ctx->created_bg && ctx->pipe_pid != getpid() && ctx->thread_worker) { + /* This delete is happening from a different process. Delete + * the thread worker from this process memory space. The + * thread is not there to do so, so it is freed here. */ + struct ub_event_base* evbase = comm_base_internal( + ctx->thread_worker->base); + libworker_delete_event(ctx->thread_worker); + ctx->thread_worker = NULL; +#ifdef USE_MINI_EVENT + ub_event_base_free(evbase); +#else + /* cannot event_base_free, because the epoll_fd cleanup + * in libevent could stop the original event_base in the + * other process from working. */ + free(evbase); +#endif + } libworker_delete_event(ctx->event_worker); modstack_desetup(&ctx->mods, ctx->env); diff --git a/libunbound/libworker.c b/libunbound/libworker.c index 11bf5f9db..b9ef02217 100644 --- a/libunbound/libworker.c +++ b/libunbound/libworker.c @@ -395,6 +395,7 @@ int libworker_bg(struct ub_ctx* ctx) w = libworker_setup(ctx, 1, NULL); if(!w) return UB_NOMEM; w->is_bg_thread = 1; + ctx->thread_worker = w; #ifdef ENABLE_LOCK_CHECKS w->thread_num = 1; /* for nicer DEBUG checklocks */ #endif -- 2.47.3