]> git.ipfire.org Git - thirdparty/unbound.git/commitdiff
- Fix #775: libunbound: subprocess reap causes parent process reap
authorW.C.A. Wijngaards <wouter@nlnetlabs.nl>
Tue, 8 Nov 2022 14:04:05 +0000 (15:04 +0100)
committerW.C.A. Wijngaards <wouter@nlnetlabs.nl>
Tue, 8 Nov 2022 14:04:05 +0000 (15:04 +0100)
  to hang.

doc/Changelog
libunbound/context.c
libunbound/context.h
libunbound/libunbound.c
libunbound/libworker.c

index 23209dabffa75c4aab21e44f5901d68f1e2a8bce..3c0a0a99a0d020147e473ddf3e4215b83bb43a2e 100644 (file)
@@ -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
index c8d911f13c7f9b90fa4c605313fb89b5612c7e63..f7c0a2cd5fae99996da6bc3dd4f0966d4e4a9800 100644 (file)
@@ -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))
index c0c86fb526978e0f142cdd4bb9e367a98e5b859a..c0fc80e57be4d2e425cf9f1b65d10aeb7e142bbe 100644 (file)
@@ -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;
index ea5ef24bb01ca0acb412d1be2375f7267bcfd03e..225457e733822b65ff8b142fdc2f90692bba85ed 100644 (file)
@@ -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);
index 11bf5f9db555d0f9e69c4547fdd6287fcff24394..b9ef02217a2fd85a9448ea7893a4ce1b18a2c7b4 100644 (file)
@@ -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