Make system() and popen() thread safe

jart · jart · commit f52f65b2e351 · 2022-10-13T15:54:05.000-07:00
diff --git a/libc/calls/__sig2.c b/libc/calls/__sig2.c
@@ -66,21 +66,19 @@ static inline textwindows int __sig_is_masked(int sig) {
 }
 
 textwindows int __sig_is_applicable(struct Signal *s) {
-  return (s->tid <= 0 || s->tid == gettid()) && !__sig_is_masked(s->sig);
+  return s->tid <= 0 || s->tid == gettid();
 }
 
 /**
  * Dequeues signal that isn't masked.
  * @return signal or null if empty or none unmasked
  */
 static textwindows struct Signal *__sig_remove(void) {
-  int tid;
   struct Signal *prev, *res;
   if (__sig.queue) {
-    tid = gettid();
     __sig_lock();
     for (prev = 0, res = __sig.queue; res; prev = res, res = res->next) {
-      if (__sig_is_applicable(res)) {
+      if (__sig_is_applicable(res) && !__sig_is_masked(res->sig)) {
         if (res == __sig.queue) {
           __sig.queue = res->next;
         } else if (prev) {
diff --git a/libc/calls/siglock.c b/libc/calls/siglock.c
@@ -19,8 +19,6 @@
 #include "libc/calls/state.internal.h"
 #include "libc/thread/thread.h"
 
-static pthread_mutex_t __sig_lock_obj;
-
 void(__sig_lock)(void) {
   pthread_mutex_lock(&__sig_lock_obj);
 }
diff --git a/libc/calls/siglock_obj.c b/libc/calls/siglock_obj.c
@@ -16,17 +16,7 @@
 │ TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR             │
 │ PERFORMANCE OF THIS SOFTWARE.                                                │
 ╚─────────────────────────────────────────────────────────────────────────────*/
-#include "libc/calls/struct/sigaction.h"
-#include "libc/intrin/intrin.h"
-#include "libc/intrin/kprintf.h"
-#include "libc/sysv/consts/sig.h"
-#include "libc/testlib/testlib.h"
+#include "libc/calls/state.internal.h"
+#include "libc/thread/thread.h"
 
-void OnSig(int sig) {
-  kprintf("got sig\n");
-}
-
-TEST(gettid, test) {
-  signal(SIGTRAP, OnSig);
-  DebugBreak();
-}
+pthread_mutex_t __sig_lock_obj;
diff --git a/libc/calls/state.internal.h b/libc/calls/state.internal.h
@@ -11,6 +11,7 @@ hidden extern bool __time_critical;
 hidden extern unsigned __sighandrvas[NSIG];
 hidden extern unsigned __sighandflags[NSIG];
 hidden extern pthread_mutex_t __fds_lock_obj;
+hidden extern pthread_mutex_t __sig_lock_obj;
 hidden extern const struct NtSecurityAttributes kNtIsInheritable;
 
 void __fds_lock(void);
diff --git a/libc/intrin/fds_lock_obj.c b/libc/intrin/fds_lock_obj.c
@@ -0,0 +1,22 @@
+/*-*- mode:c;indent-tabs-mode:nil;c-basic-offset:2;tab-width:8;coding:utf-8 -*-│
+│vi: set net ft=c ts=2 sts=2 sw=2 fenc=utf-8                                :vi│
+╞══════════════════════════════════════════════════════════════════════════════╡
+│ Copyright 2022 Justine Alexandra Roberts Tunney                              │
+│                                                                              │
+│ 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/calls/state.internal.h"
+#include "libc/thread/thread.h"
+
+pthread_mutex_t __fds_lock_obj;
diff --git a/libc/intrin/flushers.c b/libc/intrin/flushers.c
@@ -18,4 +18,5 @@
 ╚─────────────────────────────────────────────────────────────────────────────*/
 #include "libc/stdio/fflush.internal.h"
 
+pthread_mutex_t __fflush_lock_obj;
 struct StdioFlush __fflush;
diff --git a/libc/intrin/g_fds.c b/libc/intrin/g_fds.c
@@ -30,7 +30,6 @@
 STATIC_YOINK("_init_g_fds");
 
 struct Fds g_fds;
-pthread_mutex_t __fds_lock_obj;
 
 static textwindows dontinline void SetupWinStd(struct Fds *fds, int i, int x) {
   int64_t h;
diff --git a/libc/runtime/fork.c b/libc/runtime/fork.c
@@ -21,19 +21,23 @@
 #include "libc/calls/struct/sigset.internal.h"
 #include "libc/calls/syscall-nt.internal.h"
 #include "libc/calls/syscall-sysv.internal.h"
-#include "libc/calls/syscall_support-sysv.internal.h"
 #include "libc/dce.h"
 #include "libc/intrin/atomic.h"
 #include "libc/intrin/strace.internal.h"
+#include "libc/intrin/weaken.h"
 #include "libc/nt/process.h"
 #include "libc/runtime/internal.h"
 #include "libc/sysv/consts/sig.h"
+#include "libc/thread/posixthread.internal.h"
 #include "libc/thread/tls.h"
 
 int _fork(uint32_t dwCreationFlags) {
   axdx_t ad;
   sigset_t old, all;
-  int ax, dx, parent;
+  int ax, dx, parent, parent_tid = 0;
+  if (_weaken(_pthread_atfork)) {
+    parent_tid = gettid();
+  }
   if (!IsWindows()) {
     sigfillset(&all);
     sys_sigprocmask(SIG_BLOCK, &all, &old);
@@ -61,6 +65,9 @@ int _fork(uint32_t dwCreationFlags) {
                             IsLinux() ? dx : sys_gettid(),
                             memory_order_relaxed);
     }
+    if (_weaken(_pthread_atfork)) {
+      _weaken(_pthread_atfork)(parent_tid);
+    }
     if (!IsWindows()) sys_sigprocmask(SIG_SETMASK, &old, 0);
     STRACE("fork() → 0 (child of %d)", parent);
   } else {
diff --git a/libc/stdio/fflush.internal.h b/libc/stdio/fflush.internal.h
@@ -18,6 +18,7 @@ struct StdioFlush {
 };
 
 hidden extern struct StdioFlush __fflush;
+hidden extern pthread_mutex_t __fflush_lock_obj;
 
 void __fflush_lock(void);
 void __fflush_unlock(void);
diff --git a/libc/stdio/fflush_unlocked.c b/libc/stdio/fflush_unlocked.c
@@ -19,7 +19,6 @@
 #include "libc/calls/calls.h"
 #include "libc/errno.h"
 #include "libc/intrin/bits.h"
-#include "libc/thread/thread.h"
 #include "libc/intrin/pushpop.h"
 #include "libc/macros.internal.h"
 #include "libc/mem/arraylist.internal.h"
@@ -29,8 +28,7 @@
 #include "libc/stdio/internal.h"
 #include "libc/stdio/stdio.h"
 #include "libc/sysv/consts/o.h"
-
-static pthread_mutex_t __fflush_lock_obj;
+#include "libc/thread/thread.h"
 
 void(__fflush_lock)(void) {
   pthread_mutex_lock(&__fflush_lock_obj);
diff --git a/libc/stdio/popen.c b/libc/stdio/popen.c
@@ -35,6 +35,7 @@
  * Bourne-like syntax on all platforms including Windows.
  *
  * @see pclose()
+ * @threadsafe
  */
 FILE *popen(const char *cmdline, const char *mode) {
   FILE *f;
diff --git a/libc/stdio/posix_spawn.c b/libc/stdio/posix_spawn.c
@@ -20,9 +20,12 @@
 #include "libc/calls/struct/sigaction.h"
 #include "libc/calls/struct/sigset.h"
 #include "libc/errno.h"
+#include "libc/intrin/weaken.h"
 #include "libc/runtime/runtime.h"
 #include "libc/stdio/posix_spawn.h"
 #include "libc/stdio/posix_spawn.internal.h"
+#include "libc/thread/thread.h"
+#include "libc/thread/tls.h"
 
 static int RunFileActions(struct _posix_faction *a) {
   int t;
@@ -68,7 +71,7 @@ int posix_spawn(int *pid, const char *path,
   int s, child;
   sigset_t allsigs;
   struct sigaction dfl;
-  if (!(child = vfork())) {
+  if (!(child = _weaken(pthread_create) ? fork() : vfork())) {
     if (attrp && *attrp) {
       if ((*attrp)->flags & POSIX_SPAWN_SETPGROUP) {
         if (setpgid(0, (*attrp)->pgroup)) _Exit(127);
diff --git a/libc/stdio/system.c b/libc/stdio/system.c
@@ -37,6 +37,7 @@
  * @param cmdline is an interpreted Turing-complete command
  * @return -1 if child process couldn't be created, otherwise a wait
  *     status that can be accessed using macros like WEXITSTATUS(s)
+ * @threadsafe
  */
 int system(const char *cmdline) {
   int pid, wstatus;
diff --git a/libc/thread/posixthread.internal.h b/libc/thread/posixthread.internal.h
@@ -82,6 +82,8 @@ hidden extern uint64_t _pthread_key_usage[(PTHREAD_KEYS_MAX + 63) / 64];
 hidden extern pthread_key_dtor _pthread_key_dtor[PTHREAD_KEYS_MAX];
 hidden extern _Thread_local void *_pthread_keys[PTHREAD_KEYS_MAX];
 
+void _pthread_atfork(int) hidden;
+void _pthread_funlock(pthread_mutex_t *, int) hidden;
 int _pthread_reschedule(struct PosixThread *) hidden;
 int _pthread_setschedparam_freebsd(int, int, const struct sched_param *) hidden;
 void _pthread_free(struct PosixThread *) hidden;
diff --git a/libc/thread/pthread_create.c b/libc/thread/pthread_create.c
@@ -19,6 +19,7 @@
 #include "libc/assert.h"
 #include "libc/calls/calls.h"
 #include "libc/calls/sched-sysv.internal.h"
+#include "libc/calls/state.internal.h"
 #include "libc/calls/struct/sigaltstack.h"
 #include "libc/calls/syscall-sysv.internal.h"
 #include "libc/dce.h"
@@ -35,6 +36,8 @@
 #include "libc/runtime/clone.internal.h"
 #include "libc/runtime/runtime.h"
 #include "libc/runtime/stack.h"
+#include "libc/stdio/fflush.internal.h"
+#include "libc/stdio/stdio.h"
 #include "libc/sysv/consts/clone.h"
 #include "libc/sysv/consts/map.h"
 #include "libc/sysv/consts/prot.h"
@@ -45,6 +48,7 @@
 #include "libc/thread/thread.h"
 #include "libc/thread/tls.h"
 #include "libc/thread/wait0.internal.h"
+#include "third_party/dlmalloc/dlmalloc.h"
 
 STATIC_YOINK("nsync_mu_lock");
 STATIC_YOINK("nsync_mu_unlock");
@@ -72,6 +76,30 @@ void _pthread_free(struct PosixThread *pt) {
   free(pt);
 }
 
+void _pthread_funlock(pthread_mutex_t *mu, int parent_tid) {
+  if (mu->_type == PTHREAD_MUTEX_NORMAL ||
+      (atomic_load_explicit(&mu->_lock, memory_order_relaxed) &&
+       mu->_owner != parent_tid)) {
+    atomic_store_explicit(&mu->_lock, 0, memory_order_relaxed);
+    mu->_nsync = 0;
+    mu->_depth = 0;
+    mu->_owner = 0;
+  }
+}
+
+void _pthread_atfork(int parent_tid) {
+  FILE *f;
+  if (_weaken(dlmalloc_atfork)) _weaken(dlmalloc_atfork)();
+  _pthread_funlock(&__fds_lock_obj, parent_tid);
+  _pthread_funlock(&__sig_lock_obj, parent_tid);
+  _pthread_funlock(&__fflush_lock_obj, parent_tid);
+  for (int i = 0; i < __fflush.handles.i; ++i) {
+    if ((f = __fflush.handles.p[i])) {
+      _pthread_funlock((pthread_mutex_t *)f->lock, parent_tid);
+    }
+  }
+}
+
 static int PosixThread(void *arg, int tid) {
   struct PosixThread *pt = arg;
   enum PosixThreadStatus status;
@@ -281,10 +309,12 @@ errno_t pthread_create(pthread_t *thread, const pthread_attr_t *attr,
   // set initial status
   switch (pt->attr.__detachstate) {
     case PTHREAD_CREATE_JOINABLE:
-      pt->status = kPosixThreadJoinable;
+      atomic_store_explicit(&pt->status, kPosixThreadJoinable,
+                            memory_order_relaxed);
       break;
     case PTHREAD_CREATE_DETACHED:
-      pt->status = kPosixThreadDetached;
+      atomic_store_explicit(&pt->status, kPosixThreadDetached,
+                            memory_order_relaxed);
       _pthread_zombies_add(pt);
       break;
     default:
diff --git a/net/turfwar/turfwar.c b/net/turfwar/turfwar.c
@@ -1770,11 +1770,11 @@ void CheckDatabase(void) {
 }
 
 int main(int argc, char *argv[]) {
-  ShowCrashReports();
+  // ShowCrashReports();
 
   if (IsLinux()) {
     Write(2, "Enabling TCP_FASTOPEN for server sockets...\n");
-    system("sudo sh -c 'echo 2 >/proc/sys/net/ipv4/tcp_fastopen'");
+    system("sudo sh -c 'echo 3 >/proc/sys/net/ipv4/tcp_fastopen'");
   }
 
   // we don't have proper futexes on these platforms
diff --git a/test/libc/stdio/popen_test.c b/test/libc/stdio/popen_test.c
@@ -20,10 +20,18 @@
 #include "libc/calls/struct/sigaction.h"
 #include "libc/dce.h"
 #include "libc/fmt/fmt.h"
+#include "libc/fmt/itoa.h"
+#include "libc/limits.h"
+#include "libc/mem/gc.h"
+#include "libc/mem/mem.h"
 #include "libc/runtime/runtime.h"
+#include "libc/stdio/lock.internal.h"
+#include "libc/stdio/rand.h"
 #include "libc/stdio/stdio.h"
+#include "libc/str/str.h"
 #include "libc/sysv/consts/sig.h"
 #include "libc/testlib/testlib.h"
+#include "libc/thread/thread.h"
 
 FILE *f;
 char buf[32];
@@ -93,3 +101,39 @@ TEST(popen, complicated) {
   ASSERT_EQ(1, gotsig);
   signal(SIGUSR1, SIG_DFL);
 }
+
+void *Worker(void *arg) {
+  FILE *f;
+  char buf[32];
+  char *arg1, *arg2, *cmd;
+  for (int i = 0; i < 8; ++i) {
+    cmd = malloc(64);
+    arg1 = malloc(13);
+    arg2 = malloc(13);
+    FormatInt32(arg1, _rand64());
+    FormatInt32(arg2, _rand64());
+    sprintf(cmd, "echo %s; ./echo.com %s", arg1, arg2);
+    strcat(arg1, "\n");
+    strcat(arg2, "\n");
+    ASSERT_NE(NULL, (f = popen(cmd, "r")));
+    ASSERT_STREQ(arg1, fgets(buf, sizeof(buf), f));
+    ASSERT_STREQ(arg2, fgets(buf, sizeof(buf), f));
+    ASSERT_EQ(0, pclose(f));
+    free(arg2);
+    free(arg1);
+    free(cmd);
+  }
+  return 0;
+}
+
+TEST(popen, torture) {
+  int i, n = 8;
+  pthread_t *t = _gc(malloc(sizeof(pthread_t) * n));
+  testlib_extract("/zip/echo.com", "echo.com", 0755);
+  for (i = 0; i < n; ++i) {
+    ASSERT_EQ(0, pthread_create(t + i, 0, Worker, 0));
+  }
+  for (i = 0; i < n; ++i) {
+    ASSERT_EQ(0, pthread_join(t[i], 0));
+  }
+}
diff --git a/third_party/dlmalloc/dlmalloc.c b/third_party/dlmalloc/dlmalloc.c
@@ -1315,6 +1315,13 @@ int dlposix_memalign(void** pp, size_t alignment, size_t bytes) {
   }
 }
 
+#if USE_LOCKS
+void dlmalloc_atfork(void) {
+  bzero(&gm->mutex, sizeof(gm->mutex));
+  bzero(&malloc_global_mutex, sizeof(malloc_global_mutex));
+}
+#endif
+
 void* dlvalloc(size_t bytes) {
   size_t pagesz;
   ensure_initialization();
diff --git a/third_party/dlmalloc/dlmalloc.h b/third_party/dlmalloc/dlmalloc.h
@@ -505,7 +505,8 @@ void mspace_inspect_all(mspace msp,
                         void (*handler)(void*, void*, size_t, void*),
                         void* arg);
 
-void dlmalloc_abort(void);
+void dlmalloc_atfork(void) hidden;
+void dlmalloc_abort(void) hidden;
 
 COSMOPOLITAN_C_END_
 #endif /* !(__ASSEMBLER__ + __LINKER__ + 0) */

Original file line number	Diff line number	Diff line change
`@@ -19,8 +19,6 @@`
`19`	`19`	`#include "libc/calls/state.internal.h"`
`20`	`20`	`#include "libc/thread/thread.h"`
`21`	`21`
`22`		`-static pthread_mutex_t __sig_lock_obj;`
`23`		`-`
`24`	`22`	`void(__sig_lock)(void) {`
`25`	`23`	`pthread_mutex_lock(&__sig_lock_obj);`
`26`	`24`	`}`