Join getaddrinfo and getnameinfo threads before fork

A getaddrinfo thread calls getaddrinfo(3) inside rb_thread_prevent_fork,
but on glibc its thread exit takes the resolver configuration lock again
after the thread has released the fork lock. A fork that lands then
leaves the child with that lock held, and every later name lookup in the
child blocks forever. getnameinfo(3) attaches the same resolver state,
even for a name in /etc/hosts, and its thread ran outside
rb_thread_prevent_fork altogether.

Make the lookup threads joinable and have fork join any that may still
be exiting. As a lookup thread finishes, still inside
rb_thread_prevent_fork, it swaps itself into a single slot with an
atomic exchange and joins the thread it displaced, which has usually
exited by then, and rb_thread_acquire_fork_lock joins whatever is left
in the slot. Once lookups settle, at most one finished thread is left
unjoined. The slot holds pthread_t as a pointer, with NULL for empty,
which assumes pthread_t is scalar and never 0.

getnameinfo threads now run within rb_thread_prevent_fork and register
as they exit, as getaddrinfo threads do, and where getnameinfo runs
without a helper thread it is called within rb_thread_prevent_fork too.
So fork also waits for a reverse lookup in progress, as it already did
for a forward one. raddrinfo_pthread_create now always creates joinable
threads, which drops its pthread_detach fallback for platforms without
pthread_attr_setdetachstate.

The slot is kept across fork, so every fork path that keeps running
Ruby code has to take the fork lock. On the BSDs, Process.daemon forks
with daemon(3) and did not take it, so the daemon would inherit the
registered thread, and its next fork would abort in pthread_join with
ESRCH. Take and release the fork lock around daemon(3) as rb_fork_ruby
does. daemon(3) can fail in the child as well as in the parent, so
compare pids to tell which process returned.

[Bug #22406]

ext/socket/raddrinfo.c

@@ -14,7 +14,7 @@
 // GETADDRINFO_IMPL == 0 : call getaddrinfo/getnameinfo directly
 // GETADDRINFO_IMPL == 1 : call getaddrinfo/getnameinfo without gvl (but uncancellable)
 // GETADDRINFO_IMPL == 2 : call getaddrinfo/getnameinfo in a dedicated pthread
-//                         (and if the call is interrupted, the pthread is detached)
+//                         (and if the call is interrupted, the pthread is left running)
 
 #ifndef GETADDRINFO_IMPL
 #  ifdef GETADDRINFO_EMU
@@ -457,6 +457,8 @@ do_getaddrinfo(void *ptr)
 
     if (need_free) free_getaddrinfo_arg(arg);
 
+    rb_thread_register_exiting();
+
     return 0;
 }
 
@@ -496,6 +498,7 @@ cancel_getaddrinfo(void *ptr)
     rb_nativethread_lock_unlock(&arg->lock);
 }
 
+// start_routine must call rb_thread_register_exiting(), or the thread is never joined.
 int
 raddrinfo_pthread_create(pthread_t *th, void *(*start_routine) (void *), void *arg)
 {
@@ -516,7 +519,7 @@ retry_attr_init:
         }
         return err;
     }
-    if ((err = pthread_attr_setdetachstate(attr_p, PTHREAD_CREATE_DETACHED)) != 0) {
+    if ((err = pthread_attr_setdetachstate(attr_p, PTHREAD_CREATE_JOINABLE)) != 0) {
         saved_errno = errno;
         pthread_attr_destroy(attr_p);
         errno = saved_errno;
@@ -538,10 +541,6 @@ retry_attr_init:
     if (ret != 0) {
         errno = saved_errno;
     }
-#else
-    if (ret == 0) {
-        pthread_detach(th); // this can race with shutdown routine of thread in some glibc versions
-    }
 #endif
     return ret;
 }
@@ -657,6 +656,13 @@ nogvl_getnameinfo(void *arg)
                                       ptr->serv, (socklen_t)ptr->servlen,
                                       ptr->flags);
 }
+
+static void *
+fork_safe_getnameinfo(void *arg)
+{
+    return rb_thread_prevent_fork(nogvl_getnameinfo, arg);
+}
+
 int
 rb_getnameinfo(const struct sockaddr *sa, socklen_t salen,
                char *host, size_t hostlen,
@@ -675,7 +681,7 @@ rb_getnameinfo(const struct sockaddr *sa, socklen_t salen,
     arg.serv = serv;
     arg.servlen = servlen;
     arg.flags = flags;
-    ret = (int)(VALUE)rb_thread_call_without_gvl(nogvl_getnameinfo, &arg, RUBY_UBF_IO, 0);
+    ret = (int)(VALUE)rb_thread_call_without_gvl(fork_safe_getnameinfo, &arg, RUBY_UBF_IO, 0);
     return ret;
 }
 
@@ -760,6 +766,8 @@ do_getnameinfo(void *ptr)
 
     if (need_free) free_getnameinfo_arg(arg);
 
+    rb_thread_register_exiting();
+
     return 0;
 }
 
@@ -785,6 +793,12 @@ cancel_getnameinfo(void *ptr)
     rb_nativethread_lock_unlock(&arg->lock);
 }
 
+static void *
+fork_safe_do_getnameinfo(void *ptr)
+{
+    return rb_thread_prevent_fork(do_getnameinfo, ptr);
+}
+
 int
 rb_getnameinfo(const struct sockaddr *sa, socklen_t salen,
                char *host, size_t hostlen,
@@ -807,7 +821,7 @@ start:
     }
 
     pthread_t th;
-    if (raddrinfo_pthread_create(&th, do_getnameinfo, arg) != 0) {
+    if (raddrinfo_pthread_create(&th, fork_safe_do_getnameinfo, arg) != 0) {
         int err = errno;
         free_getnameinfo_arg(arg);
         errno = err;
@@ -3184,6 +3198,8 @@ do_fast_fallback_getaddrinfo(void *ptr)
         free_fast_fallback_getaddrinfo_shared(&shared);
     }
 
+    rb_thread_register_exiting();
+
     return 0;
 }
 

internal/thread.h

@@ -61,6 +61,7 @@ void rb_thread_free_body(void *th);            /* vm.c */
 RUBY_SYMBOL_EXPORT_BEGIN
 
 void *rb_thread_prevent_fork(void *(*func)(void *), void *data); /* for ext/socket/raddrinfo.c */
+void rb_thread_register_exiting(void); /* for ext/socket/raddrinfo.c */
 
 /* Temporary.  This API will be removed (renamed). */
 VALUE rb_thread_io_blocking_region(struct rb_io *io, rb_blocking_function_t *func, void *data1);

process.c

@@ -1587,6 +1587,21 @@ after_fork_ruby(rb_pid_t pid)
         after_exec();
     }
 }
+
+static void
+release_fork_lock_after_fork(rb_pid_t pid)
+{
+    if (
+#if defined(__FreeBSD__)
+        pid != 0 &&
+#endif
+        true) {
+        rb_thread_release_fork_lock();
+    }
+    if (pid == 0) {
+        rb_thread_reset_fork_lock();
+    }
+}
 #endif
 
 #if defined(HAVE_WORKING_FORK)
@@ -4083,16 +4098,7 @@ rb_fork_ruby(int *status)
         }
 
         disable_child_handler_fork_parent(&old); /* yes, bad name */
-        if (
-#if defined(__FreeBSD__)
-            pid != 0 &&
-#endif
-            true) {
-            rb_thread_release_fork_lock();
-        }
-        if (pid == 0) {
-            rb_thread_reset_fork_lock();
-        }
+        release_fork_lock_after_fork(pid);
         after_fork_ruby(pid);
 
         /* repeat while fork failed but retryable */
@@ -6910,8 +6916,12 @@ rb_daemon(int nochdir, int noclose)
 {
     int err = 0;
 #ifdef HAVE_DAEMON
+    rb_pid_t pid = getpid();
     before_fork_ruby();
+    rb_thread_acquire_fork_lock();
     err = daemon(nochdir, noclose);
+    /* daemon(3) can fail in the child as well, so compare pids */
+    release_fork_lock_after_fork(getpid() == pid ? pid : 0);
     after_fork_ruby(0);
 #else
     int n;

test/ruby/test_process.rb

@@ -1947,6 +1947,32 @@ class TestProcess < Test::Unit::TestCase
       assert_not_equal(cpid, dpid)
     end
 
+    def test_daemon_after_getaddrinfo
+      data = IO.popen("-", "r+") do |f|
+        break f.read if f
+        require "socket"
+        Addrinfo.getaddrinfo("localhost", 80, :INET, :STREAM)
+        Process.daemon(true, true)
+        Addrinfo.getaddrinfo("localhost", 80, :INET, :STREAM)
+        Process.wait(fork {})
+        puts "ok"
+      end
+      assert_equal("ok\n", data, "[Bug #22406]")
+    end
+
+    def test_daemon_after_getnameinfo
+      data = IO.popen("-", "r+") do |f|
+        break f.read if f
+        require "socket"
+        Socket.getnameinfo(["AF_INET", 80, "127.0.0.1"], Socket::NI_NUMERICHOST)
+        Process.daemon(true, true)
+        Socket.getnameinfo(["AF_INET", 80, "127.0.0.1"], Socket::NI_NUMERICHOST)
+        Process.wait(fork {})
+        puts "ok"
+      end
+      assert_equal("ok\n", data, "[Bug #22406]")
+    end
+
     def test_daemon_detached
       IO.popen("-", "r+") do |f|
         if f

thread_none.c

@@ -349,6 +349,12 @@ rb_thread_prevent_fork(void *(*func)(void *), void *data)
     return func(data);
 }
 
+void
+rb_thread_register_exiting(void)
+{
+    // do nothing
+}
+
 void
 rb_thread_malloc_stack_set(rb_thread_t *th, void *stack, size_t stack_size)
 {

thread_pthread.c

@@ -1442,6 +1442,10 @@ ruby_ppoll(struct pollfd *fds, nfds_t nfds,
 // fork read-write lock (only for pthread)
 static pthread_rwlock_t rb_thread_fork_rw_lock = PTHREAD_RWLOCK_INITIALIZER;
 
+// the registered thread waiting to be joined, or NULL; no pthread_t is 0
+static void *last_exiting_thread;
+STATIC_ASSERT(pthread_t_fits_in_pointer, sizeof(pthread_t) <= sizeof(void *));
+
 void
 rb_thread_release_fork_lock(void)
 {
@@ -1476,6 +1480,26 @@ rb_thread_prevent_fork(void *(*func)(void *), void *data)
     return result;
 }
 
+static void
+thread_register_exiting(void *thread)
+{
+    void *previous_thread = RUBY_ATOMIC_PTR_EXCHANGE(last_exiting_thread, thread);
+    if (previous_thread) {
+        int r;
+        if ((r = pthread_join((pthread_t)(uintptr_t)previous_thread, NULL))) {
+            rb_bug_errno("pthread_join", r);
+        }
+    }
+}
+
+// Called within rb_thread_prevent_fork() by a joinable thread just before it
+// exits.  The thread it replaces in the slot has usually exited by then.
+void
+rb_thread_register_exiting(void)
+{
+    thread_register_exiting((void *)(uintptr_t)pthread_self());
+}
+
 void
 rb_thread_acquire_fork_lock(void)
 {
@@ -1483,6 +1507,11 @@ rb_thread_acquire_fork_lock(void)
     if ((r = pthread_rwlock_wrlock(&rb_thread_fork_rw_lock))) {
         rb_bug_errno("pthread_rwlock_wrlock", r);
     }
+
+    // A registered thread may still be exiting, and glibc's thread exit takes
+    // the resolver configuration lock, which fork does not reset.  None can
+    // register while this lock is held.
+    thread_register_exiting(NULL);
 }
 
 // thread internal event hooks (only for pthread)

thread_win32.c

@@ -1001,4 +1001,10 @@ rb_thread_prevent_fork(void *(*func)(void *), void *data)
     return func(data);
 }
 
+void
+rb_thread_register_exiting(void)
+{
+    // nothing to do: there is no fork
+}
+
 #endif /* THREAD_SYSTEM_DEPENDENT_IMPLEMENTATION */