Merge e2bebc73e3 into 69db501c68

* Fix fork locking on win32

- __enable_threads / set __threaded in __proc_setup as threads are required for
  win32 subprocess management
- move mmi/fds locking out of pthread_atfork.c into fork.c so it's done anytime
  __threaded is set instead of being dependent of pthreads
- explicitly yoink _pthread_onfork_prepare, _pthread_onfork_parent, and
  _pthread_onfork_child in pthread_create.c so they are linked in in-case they
  are separated from _pthread_atfork

Big Thanks to @dfyz for help with locating the issue, testing, and devising a fix!

* fix child processes not being able to open files, initialize all necessary locks on fork

libc/intrin/pthreadlock.c

@@ -20,6 +20,10 @@
 
 pthread_spinlock_t _pthread_lock_obj;
 
+void _pthread_init(void) {
+  (void)pthread_spin_init(&_pthread_lock_obj, 0);
+}
+
 void _pthread_lock(void) {
   pthread_spin_lock(&_pthread_lock_obj);
 }

libc/proc/fork.c

@@ -18,6 +18,7 @@
 ╚─────────────────────────────────────────────────────────────────────────────*/
 #include "libc/assert.h"
 #include "libc/calls/calls.h"
+#include "libc/calls/state.internal.h"
 #include "libc/calls/struct/sigset.h"
 #include "libc/calls/struct/sigset.internal.h"
 #include "libc/calls/syscall-nt.internal.h"
@@ -34,12 +35,45 @@
 #include "libc/nt/thread.h"
 #include "libc/proc/proc.internal.h"
 #include "libc/runtime/internal.h"
+#include "libc/runtime/memtrack.internal.h"
 #include "libc/runtime/runtime.h"
 #include "libc/runtime/syslib.internal.h"
 #include "libc/sysv/consts/sig.h"
 #include "libc/thread/posixthread.internal.h"
 #include "libc/thread/tls.h"
 
+static void _onfork_prepare(void) {
+  if (_weaken(_pthread_onfork_prepare)) {
+    _weaken(_pthread_onfork_prepare)();
+  }
+  _pthread_lock();
+  __fds_lock();
+  __mmi_lock();
+}
+
+static void _onfork_parent(void) {
+  __mmi_unlock();
+  __fds_unlock();
+  _pthread_unlock();
+  if (_weaken(_pthread_onfork_parent)) {
+    _weaken(_pthread_onfork_parent)();
+  }
+}
+
+static void _onfork_child(void) {
+  pthread_mutexattr_t attr;
+  pthread_mutexattr_init(&attr);
+  pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
+  extern pthread_mutex_t __mmi_lock_obj;
+  pthread_mutex_init(&__mmi_lock_obj, &attr);
+  pthread_mutex_init(&__fds_lock_obj, &attr);
+  pthread_mutexattr_destroy(&attr);
+  _pthread_init();
+  if (_weaken(_pthread_onfork_child)) {
+    _weaken(_pthread_onfork_child)();
+  }
+}
+
 int _fork(uint32_t dwCreationFlags) {
   struct Dll *e;
   int ax, dx, tid, parent;
@@ -47,8 +81,8 @@ int _fork(uint32_t dwCreationFlags) {
   BLOCK_SIGNALS;
   if (IsWindows())
     __proc_lock();
-  if (__threaded && _weaken(_pthread_onfork_prepare)) {
-    _weaken(_pthread_onfork_prepare)();
+  if (__threaded) {
+    _onfork_prepare();
   }
   if (!IsWindows()) {
     ax = sys_fork();
@@ -99,14 +133,14 @@ int _fork(uint32_t dwCreationFlags) {
     atomic_store_explicit(&pt->pt_canceled, false, memory_order_relaxed);
 
     // run user fork callbacks
-    if (__threaded && _weaken(_pthread_onfork_child)) {
-      _weaken(_pthread_onfork_child)();
+    if (__threaded) {
+      _onfork_child();
     }
     STRACE("fork() → 0 (child of %d)", parent);
   } else {
     // this is the parent process
-    if (__threaded && _weaken(_pthread_onfork_parent)) {
-      _weaken(_pthread_onfork_parent)();
+    if (__threaded) {
+      _onfork_parent();
     }
     if (IsWindows())
       __proc_unlock();

libc/proc/proc.c

@@ -233,6 +233,7 @@ static textwindows dontinstrument uint32_t __proc_worker(void *arg) {
  * Lazy initializes process tracker data structures and worker.
  */
 static textwindows void __proc_setup(void) {
+  __enable_threads();
   __proc.onbirth = CreateEvent(0, 0, 0, 0);     // auto reset
   __proc.haszombies = CreateEvent(0, 1, 0, 0);  // manual reset
   __proc.thread = CreateThread(0, 65536, __proc_worker, 0,

libc/stdio/fgets_unlocked.c

@@ -57,11 +57,7 @@ char *fgets_unlocked(char *s, int size, FILE *f) {
           break;
       } else {
         if ((c = fgetc_unlocked(f)) == -1) {
-          if (ferror_unlocked(f) == EINTR) {
-            continue;
-          } else {
-            break;
-          }
+          break;
         }
         *p++ = c & 255;
         if (c == '\n')

libc/thread/posixthread.internal.h

@@ -105,6 +105,7 @@ intptr_t _pthread_syshand(struct PosixThread *) libcesque;
 long _pthread_cancel_ack(void) libcesque;
 void _pthread_decimate(void) libcesque;
 void _pthread_free(struct PosixThread *, bool) libcesque;
+void _pthread_init(void) libcesque;
 void _pthread_lock(void) libcesque;
 void _pthread_onfork_child(void) libcesque;
 void _pthread_onfork_parent(void) libcesque;

libc/thread/pthread_atfork.c

@@ -28,7 +28,6 @@
 #include "libc/macros.internal.h"
 #include "libc/mem/mem.h"
 #include "libc/proc/proc.internal.h"
-#include "libc/runtime/memtrack.internal.h"
 #include "libc/runtime/runtime.h"
 #include "libc/str/str.h"
 #include "libc/thread/posixthread.internal.h"
@@ -47,8 +46,6 @@ static struct AtForks {
   atomic_int allocated;
 } _atforks;
 
-extern pthread_spinlock_t _pthread_lock_obj;
-
 static void _pthread_onfork(int i) {
   struct AtFork *a;
   unassert(0 <= i && i <= 2);
@@ -65,27 +62,13 @@ static void _pthread_onfork(int i) {
 
 void _pthread_onfork_prepare(void) {
   _pthread_onfork(0);
-  _pthread_lock();
-  __fds_lock();
-  __mmi_lock();
 }
 
 void _pthread_onfork_parent(void) {
-  __mmi_unlock();
-  __fds_unlock();
-  _pthread_unlock();
   _pthread_onfork(1);
 }
 
 void _pthread_onfork_child(void) {
-  pthread_mutexattr_t attr;
-  pthread_mutexattr_init(&attr);
-  pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
-  extern pthread_mutex_t __mmi_lock_obj;
-  pthread_mutex_init(&__mmi_lock_obj, &attr);
-  pthread_mutex_init(&__fds_lock_obj, &attr);
-  pthread_mutexattr_destroy(&attr);
-  (void)pthread_spin_init(&_pthread_lock_obj, 0);
   _pthread_onfork(2);
 }
 

libc/thread/pthread_create.c

@@ -60,6 +60,9 @@ __static_yoink("nsync_mu_trylock");
 __static_yoink("nsync_mu_rlock");
 __static_yoink("nsync_mu_runlock");
 __static_yoink("_pthread_atfork");
+__static_yoink("_pthread_onfork_prepare");
+__static_yoink("_pthread_onfork_parent");
+__static_yoink("_pthread_onfork_child");
 
 #define MAP_ANON_OPENBSD  0x1000
 #define MAP_STACK_OPENBSD 0x4000

test/libc/stdio/fgets_interrupt_test.c

@@ -0,0 +1,119 @@
+/*-*- mode:c;indent-tabs-mode:nil;c-basic-offset:2;tab-width:8;coding:utf-8 -*-│
+│ vi: set et ft=c ts=2 sts=2 sw=2 fenc=utf-8                               :vi │
+╞══════════════════════════════════════════════════════════════════════════════╡
+│ Copyright 2024 Cadence Ember                                                 │
+│                                                                              │
+│ Permission to use, copy, modify, and/or distribute this software for         │
+│ any purpose with or without fee is hereby granted, provided that the         │
+│ above copyright notice and this permission notice appear in all copies.      │
+│                                                                              │
+│ THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL                │
+│ WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED                │
+│ WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE             │
+│ AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL         │
+│ DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR        │
+│ PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER               │
+│ TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR             │
+│ PERFORMANCE OF THIS SOFTWARE.                                                │
+╚─────────────────────────────────────────────────────────────────────────────*/
+#include "libc/isystem/errno.h"
+#include "libc/isystem/signal.h"
+#include "libc/isystem/stddef.h"
+#include "libc/isystem/unistd.h"
+#include "libc/stdio/stdio.h"
+#include "libc/testlib/testlib.h"
+
+#define MY_TEST_STRING_1 "He"
+#define MY_TEST_STRING_2 "llo world!"
+
+char buf[20] = {0};
+int pipes[2];
+int pid;
+int got_sigusr1 = 0;
+
+void sigusr1_handler(int) {
+  got_sigusr1 = 1;
+}
+
+void write_pipe(int send_signal_before_end) {
+  // Set up pipe for writing
+  close(pipes[0]);
+  FILE *stream = fdopen(pipes[1], "w");
+
+  // Start writing the first part of the stream
+  fputs(MY_TEST_STRING_1, stream);
+
+  // Send SIGUSR1 to parent (if we're currently testing that)
+  if (send_signal_before_end) {
+    kill(getppid(), SIGUSR1);
+  }
+
+  // Send rest of stream
+  fputs(MY_TEST_STRING_2, stream);
+
+  // Close stream - this will cause the parent's fgets to end
+  fclose(stream);
+}
+
+void read_pipe() {
+  // Set up pipe for reading
+  close(pipes[1]);
+  FILE *stream = fdopen(pipes[0], "r");
+
+  // Read with fgets
+  fgets(buf, 20, stream);
+
+  // Tidy up
+  fclose(stream);
+}
+
+void setup_signal_and_pipe(uint64_t sa_flags) {
+  // Set up SIGUSR1 handler
+  struct sigaction sa = {.sa_handler = sigusr1_handler, .sa_flags = sa_flags};
+  if (sigaction(SIGUSR1, &sa, NULL) == -1) {
+    perror("sigaction");
+    _exit(1);
+  }
+
+  // Set up pipe between parent and child
+  if (pipe(pipes) == -1) {
+    perror("pipe");
+    _exit(1);
+  }
+}
+
+TEST(fgets_eintr, testThatFgetsReadsFromPipeNormally) {
+  setup_signal_and_pipe(0);  // 0 = no SA_RESTART
+  ASSERT_NE(-1, (pid = fork()));
+  if (!pid) {
+    write_pipe(0);  // 0 = no signal
+    _exit(0);
+  }
+  read_pipe();
+  EXPECT_STREQ(MY_TEST_STRING_1 MY_TEST_STRING_2, buf);
+}
+
+TEST(fgets_eintr, testThatTheSignalInterruptsFgets) {
+  setup_signal_and_pipe(0);  // 0 = no SA_RESTART
+  ASSERT_NE(-1, (pid = fork()));
+  if (!pid) {
+    write_pipe(1);  // 1 = signal
+    _exit(0);
+  }
+  read_pipe();
+  EXPECT_STRNE(MY_TEST_STRING_1 MY_TEST_STRING_2, buf);
+  EXPECT_EQ(EINTR, errno);
+}
+
+TEST(fgets_eintr, testThatFgetsRestartsWhenSaRestartIsSet) {
+  setup_signal_and_pipe(SA_RESTART);  // SA_RESTART
+  ASSERT_NE(-1, (pid = fork()));
+  if (!pid) {
+    write_pipe(1);  // 1 = signal
+    _exit(0);
+  }
+  read_pipe();
+  EXPECT_STREQ(MY_TEST_STRING_1 MY_TEST_STRING_2, buf);
+  EXPECT_NE(EINTR, errno);
+  EXPECT_EQ(1, got_sigusr1);
+}