Joined threads now terminate, not park forever (and other TSAN-friendly fixes; 5 commits)

include/dmtcp.h

@@ -16,6 +16,8 @@
 #define DMTCP_H
 
 #include <netinet/ip.h>
+#include <assert.h>
+#include <pthread.h>
 #include <stddef.h>
 #include <stdio.h>
 #include <sys/socket.h>
@@ -43,8 +45,8 @@
 # define EXTERNC
 #endif // ifdef __cplusplus
 
-/* Define to the version of this package. */
-#define DMTCP_PLUGIN_API_VERSION "3"
+/* Bump when DmtcpPluginDescriptor_t changes ABI. */
+#define DMTCP_PLUGIN_API_VERSION "4"
 
 #ifdef __cplusplus
 namespace dmtcp {
@@ -145,6 +147,11 @@ typedef union _DmtcpEventData_t {
   struct {
     char *path;
   } realToVirtualPath, virtualToRealPath;
+
+  struct {
+    pthread_t pthread;
+    pid_t tid;
+  } pthreadInfo;
 } DmtcpEventData_t;
 
 typedef void (*HookFunctionPtr_t)(DmtcpEvent_t, DmtcpEventData_t *);
@@ -238,12 +245,78 @@ void dmtcp_initialize_plugin(void) __attribute((weak));
 typedef struct DmtcpUniqueProcessId {
   uint64_t _hostid;  // gethostid()
   uint64_t _time; // time()
-  pid_t _pid; // getpid()
+
+  union {
+    pid_t _pid; // getpid()
+    int32_t _;
+  };
+
   uint32_t _computation_generation; // computationGeneration()
 } DmtcpUniqueProcessId;
 
 int dmtcp_unique_pids_equal(DmtcpUniqueProcessId a, DmtcpUniqueProcessId b);
 
+typedef struct DmtcpInfo {
+  int argc;
+  const char **argv;
+} DmtcpInfo;
+
+enum ElfType {
+  Elf_32,
+  Elf_64
+};
+
+typedef struct {
+  uint64_t startAddr;
+  uint64_t endAddr;
+} MemRegion;
+
+typedef void (*PostRestartFnPtr_t)(double, int);
+#define DMTCP_CKPT_SIGNATURE "DMTCP_CHECKPOINT_IMAGE_v4.0\n"
+typedef struct {
+  char ckptSignature[32];
+
+  DmtcpUniqueProcessId upid;
+  DmtcpUniqueProcessId uppid;
+  DmtcpUniqueProcessId compGroup;
+
+  pid_t pid;
+  pid_t ppid;
+  pid_t sid;
+  pid_t gid;
+  pid_t fgid;
+  uint32_t isRootOfProcessTree;
+
+  uint32_t numPeers;
+  uint32_t elfType;
+
+  uint64_t clock_gettime_offset;
+  uint64_t getcpu_offset;
+  uint64_t gettimeofday_offset;
+  uint64_t time_offset;
+
+  // Reserve 3 * 30MB for restore buffer.
+#define RESTORE_BUF_TOTAL_SIZE (90 * 1024 * 1024)
+  MemRegion restoreBuf;
+
+  MemRegion vdso;
+  MemRegion vvar;
+  MemRegion vvarVClock;
+
+  uint64_t savedBrk;
+  uint64_t endOfStack;
+
+  uint64_t postRestartAddr;
+  //void (*post_restart)(double, int);
+
+  char procname[1024];
+  char procSelfExe[1024];
+
+  char padding[1792];
+} DmtcpCkptHeader;
+
+static_assert(sizeof(DmtcpCkptHeader) == 4096, "DmtcpCkptHeader must be 4096 bytes");
+
 // FIXME:
 // If a plugin is not compiled with defined(__PIC__) and we can verify
 // that we're using DMTCP (environment variables), and dmtcp_is_enabled
@@ -329,8 +402,6 @@ const char *dmtcp_get_ckpt_files_subdir(void);
 int dmtcp_should_ckpt_open_files(void);
 int dmtcp_allow_overwrite_with_ckpted_files(void);
 int dmtcp_skip_truncate_file_at_restart(const char* path);
-void dmtcp_set_restore_buf_addr(void *new_addr, uint64_t len);
-uint64_t dmtcp_restore_buf_len();
 
 int dmtcp_get_ckpt_signal(void);
 const char *dmtcp_get_uniquepid_str(void) __attribute__((weak));
@@ -428,13 +499,22 @@ int dmtcp_protected_environ_fd(void);
  *  discovers a pid without going through a system call (e.g., through
  *  the proc filesystem), use this to virtualize the pid.
  */
-pid_t dmtcp_real_to_virtual_pid(pid_t realPid) __attribute((weak));
-pid_t dmtcp_virtual_to_real_pid(pid_t virtualPid) __attribute((weak));
-
+pid_t dmtcp_pid_real_to_virtual(pid_t realPid) __attribute((weak));
+pid_t dmtcp_pid_virtual_to_real(pid_t virtualPid) __attribute((weak));
+
+// Returns 1 if virtual_tid names TSAN's own background thread, else 0.
+int dmtcp_is_tsan_background_thread(int virtual_tid) __attribute((weak));
+#define dmtcp_is_tsan_background_thread(virtual_tid) \
+  (dmtcp_is_tsan_background_thread ? \
+   dmtcp_is_tsan_background_thread(virtual_tid) : 0)
+
+// McMini helpers: translate pids only when running under DMTCP.
+// (DMTCP plugin API v4 renamed dmtcp_{real_to_virtual,virtual_to_real}_pid to
+// dmtcp_pid_{real_to_virtual,virtual_to_real}.)
 #define mcmini_virtual_pid(PID) \
-  (dmtcp_is_enabled() ? dmtcp_real_to_virtual_pid((PID)) : (PID))
+  (dmtcp_is_enabled() ? dmtcp_pid_real_to_virtual((PID)) : (PID))
 #define mcmini_real_pid(PID) \
-  (dmtcp_is_enabled() ? dmtcp_virtual_to_real_pid((PID)) : (PID))
+  (dmtcp_is_enabled() ? dmtcp_pid_virtual_to_real((PID)) : (PID))
 
 // bq_file -> "batch queue file"; used only by batch-queue plugin
 int dmtcp_is_bq_file(const char *path) __attribute((weak));
@@ -445,7 +525,7 @@ int dmtcp_bq_restore_file(const char *path,
                           int type) __attribute((weak));
 
 /*  These next two functions are defined in contrib/ckptfile/ckptfile.cpp
- *  But they are currently used only in src/plugin/ipc/file/fileconnection.cpp
+ *  But they are currently used only in src/plugin/file/fileconnection.cpp
  *    and in a trivial fashion.  These are intended for future extensions.
  */
 int dmtcp_must_ckpt_file(const char *path) __attribute((weak));

include/mcmini/spy/intercept/interception.h

@@ -29,6 +29,11 @@ int libdmtcp_pthread_create(pthread_t *thread, const pthread_attr_t *attr,
 int pthread_join(pthread_t thread, void**);
 int libpthread_pthread_join(pthread_t thread, void**);
 int libdmtcp_pthread_join(pthread_t thread, void**);
+// TSan-safe (libtsan-bypassing) handle for pthread_timedjoin_np, used by
+// mc_pthread_join's RECORD loop. Calling pthread_timedjoin_np directly resolves
+// to libtsan's interceptor, which trips a thread-registry CHECK. See
+// TSAN-McMini-DMTCP.txt.
+int libpthread_timedjoin_np(pthread_t thread, void**, const struct timespec*);
 
 int libpthread_mutex_init(pthread_mutex_t *, const pthread_mutexattr_t *);
 int libpthread_mutex_lock(pthread_mutex_t *);

src/common/multithreaded_fork.c

@@ -177,7 +177,7 @@ pid_t get_tid_from_pthread_descriptor(pthread_t pthread_descriptor) {
   int offset = pthreadDescriptorTidOffset();
   pid_t ctid = *(pid_t*)((char*)(pthread_descriptor) + offset);
 #ifdef DMTCP
-  pid_t virttid = dmtcp_real_to_virtual_pid(ctid);
+  pid_t virttid = dmtcp_pid_real_to_virtual(ctid);
   ctid = (virttid ? virttid : ctid);
 #endif
   return ctid;
@@ -209,7 +209,7 @@ int get_child_threads(int child_threads[]) {
       if (atoi(entry->d_name) != 0) {
         pid_t nexttid = atoi(entry->d_name);
 #ifdef DMTCP
-        pid_t virttid = dmtcp_real_to_virtual_pid(nexttid);
+        pid_t virttid = dmtcp_pid_real_to_virtual(nexttid);
         nexttid = (virttid ? virttid : nexttid);
 #endif
         child_threads[i++] = nexttid;

src/lib/dmtcp-callback.c

@@ -1,6 +1,5 @@
 #define _GNU_SOURCE
 #include <assert.h>
-#include <dirent.h>
 #include <errno.h>
 #include <fcntl.h>  // man 2 open
 #include <pthread.h>
@@ -42,8 +41,6 @@ struct threadinfo {
   // tlsAddr only used in __aarch64__ and __riscv
   // In fact, __riscv has the address in a normal register, restored w/ context.
   unsigned long int tlsAddr;
-  // The kernel has a process-wide sigmask, and also a per-thread sigmask.
-  sigset_t thread_sigmask;
   // glibc:pthread_create and pthread_self use this, but not the clone call:
   pthread_t pthread_descriptor;
 } threadInfos[1000];
@@ -125,23 +122,19 @@ static void saveThreadStateBeforeFork(struct threadinfo* threadInfo) {
   threadInfo->pthread_descriptor = pthread_self();
   getTLSPointer(threadInfo);
 
-  // FIXME:  Add func fo get/set signals in child thread of child process.
-  //         and restore thread sigmask sfter setcontext.
-  pthread_sigmask(SIG_BLOCK, NULL, &threadInfo->thread_sigmask);
-  sigset_t sigtest;
-  pthread_sigmask(SIG_BLOCK, NULL, &sigtest);
-  sigdelset(&sigtest, SIG_MULTITHREADED_FORK);
-  if (! sigisemptyset(&sigtest)) {
-    fprintf(stderr, "PID %d: multithreaded_fork() not yet implemented"
-                    " for non-empty thread signaks\n", getpid());
-    libc_abort();
-  }
+  // No manual signal-mask save/restore needed: getcontext()/setcontext()
+  // already save/restore the blocked-signal set via ucontext_t's uc_sigmask,
+  // even when setcontext() resumes on a brand-new clone()'d OS thread (see
+  // child_setcontext_fast() below).
 }
 
 static int child_setcontext_fast(void *arg) {
   struct threadinfo* threadInfo = arg;
   setTLSPointer(threadInfo);
   patchThreadDescriptor(threadInfo->pthread_descriptor);
+  // Does not return: jumps to the getcontext() call site in
+  // thread_handle_after_dmtcp_restart(), restoring uc_sigmask (see
+  // saveThreadStateBeforeFork() above) along with the rest of the context.
   setcontext(&(threadInfo->context));
   return 0; // not reached
 }
@@ -191,39 +184,8 @@ pid_t fast_multithreaded_fork(void) {
    *  ret = INLINE_SYSCALL_CALL (clone, flags, 0, NULL, ctid, 0);
    *}
    *********************************************************************/
-#if 1
   pid_t _Fork();
   int childpid = _Fork();
-#else
-  // NOT YET FULLY DEVELOPED:
-  int flags = CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD;
-  int childpid;
-// syscall(SYS_clone, ...);
-// stack must be NULL
-// https://stackoverflow.com/questions/2898579/clone-equivalent-of-fork
-//   But that says to use only SIGCHLD for flags, and glibc uses the above.
-//   But it's okay, since we're setting ctid and tls to NULL.
-// FIXME:  If we're going to set the last 3 args to NULL, who cares in what order they're found!
-# ifdef __x86_64__
-           long clone(unsigned long flags, void *stack,
-                      int *parent_tid, int *child_tid,
-                      unsigned long tls);
-# elif defined(__aarch64__)
-           long clone(unsigned long flags, void *stack,
-                     int *parent_tid, unsigned long tls,
-                     int *child_tid);
-# elif defined(__riscv)
-#  error Unimplemented CPU architecture
-https://github.com/bminor/glibc/blob/master/sysdeps/unix/sysv/linux/riscv/clone.S
-int clone(int (*fn)(void *arg), void *child_stack, int flags, void *arg,
-	     void *parent_tidptr, void *tls, void *child_tidptr) */
-	/* The syscall expects the args to be in different slots.  */
-	mv		a0,a2
-	mv		a2,a4
-	mv		a3,a5
-	mv		a4,a6
-# endif
-#endif
   if (childpid == 0) { // child process
     restart_child_threads_fast();
   }
@@ -321,23 +283,27 @@ static void *template_thread(void *unused) {
   // to ensure a stable recorded state is pointless: we're not going to read it
   // anyway! This is an only a potential optimization for later though.
 
+  // Counting via a live /proc/self/task scan is a TOCTOU race: DMTCP
+  // recreates checkpointed threads asynchronously via clone(), so a scan
+  // that runs before it has finished recreating all of them undercounts,
+  // and this barrier then releases before every thread has actually
+  // restarted (confirmed empirically: one thread's own restart-completion
+  // signal can arrive after this barrier already declared a "consistent
+  // state").
+  //
+  // head_record_mode instead gives an exact, race-free count: it only ever
+  // gets THREAD entries for genuine target threads (the template thread and
+  // the checkpoint thread never go through libmcmini's wrapped
+  // pthread_create(), so neither is ever recorded here), and -- since a
+  // DMTCP checkpoint is a full memory snapshot -- this list is preserved
+  // exactly as it was at record time across every restart, with no
+  // dependence on restart-time scheduling.
   int thread_count = 0;
-  struct dirent *entry;
-  DIR *dp = opendir("/proc/self/task");
-  if (dp == NULL) {
-    perror("opendir");
-    mc_exit(EXIT_FAILURE);
-  }
-
-  while ((entry = readdir(dp)))
-    if (strcmp(entry->d_name, ".") != 0 && strcmp(entry->d_name, "..") != 0)
+  for (rec_list *entry = head_record_mode; entry != NULL; entry = entry->next) {
+    if (entry->vo.type == THREAD && entry->vo.thrd_state.status == ALIVE) {
       thread_count++;
-
-  // We don't want to count the template thread nor
-  // the checkpoint thread, but these will appear in
-  // `/proc/self/tasks`
-  thread_count -= 2;
-  closedir(dp);
+    }
+  }
   log_debug(
       "There are %d threads... waiting for them to get into a consistent "
       "state...\n",

src/lib/interception.c

@@ -16,6 +16,7 @@ typeof(&pthread_create) libpthread_pthread_create_ptr;
 typeof(&pthread_create) libdmtcp_pthread_create_ptr;
 typeof(&pthread_join) libpthread_pthread_join_ptr;
 typeof(&pthread_join) libdmtcp_pthread_join_ptr;
+typeof(&pthread_timedjoin_np) libpthread_timedjoin_np_ptr;
 typeof(&pthread_mutex_init) pthread_mutex_init_ptr;
 typeof(&pthread_mutex_lock) pthread_mutex_lock_ptr;
 typeof(&pthread_mutex_trylock) pthread_mutex_trylock_ptr;
@@ -69,6 +70,7 @@ void mc_load_intercepted_pthread_functions(void) {
 
   libpthread_pthread_create_ptr = dlsym(libpthread_handle, "pthread_create");
   libpthread_pthread_join_ptr = dlsym(libpthread_handle, "pthread_join");
+  libpthread_timedjoin_np_ptr = dlsym(libpthread_handle, "pthread_timedjoin_np");
   pthread_mutex_init_ptr = dlsym(libpthread_handle, "pthread_mutex_init");
   pthread_mutex_lock_ptr = dlsym(libpthread_handle, "pthread_mutex_lock");
   pthread_mutex_trylock_ptr = dlsym(libpthread_handle, "pthread_mutex_trylock");
@@ -228,6 +230,11 @@ int libpthread_pthread_join(pthread_t thread, void **rv) {
   libmcmini_init();
   return (*libpthread_pthread_join_ptr)(thread, rv);
 }
+int libpthread_timedjoin_np(pthread_t thread, void **rv,
+                            const struct timespec *abstime) {
+  libmcmini_init();
+  return (*libpthread_timedjoin_np_ptr)(thread, rv, abstime);
+}
 int libdmtcp_pthread_join(pthread_t thread, void **rv) {
   libmcmini_init();
   return (*libdmtcp_pthread_join_ptr)(thread, rv);