Skip to content

Remove needless signal-mask check in threaded fork #10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
gc00 wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Remove needless signal-mask check in threaded fork #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions BUG.README
View file
Open in desktop

This file was deleted.

20 changes: 7 additions & 13 deletions src/lib/dmtcp-callback.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,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];
Expand Down Expand Up @@ -125,23 +123,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).
}
Comment on lines +126 to 130

@coderabbitai coderabbitai Bot Jul 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Locate getcontext call site, child resume path, and any SIG_MULTITHREADED_FORK (un)masking.
fd -t f 'dmtcp-callback.c' --exec rg -nP -C4 \
  'getcontext|setcontext|SIG_MULTITHREADED_FORK|sigdelset|pthread_sigmask|sigprocmask|child_setcontext_fast|thread_handle_after_dmtcp_restart' {}

# Cross-check the shared fork implementation for where the fork signal is unblocked.
fd -t f 'multithreaded_fork.c' --exec rg -nP -C3 \
  'SIG_MULTITHREADED_FORK|sigdelset|sigaddset|pthread_sigmask|sigprocmask|uc_sigmask' {}

Repository: mcminickpt/deep-debug

Length of output: 6625

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show the relevant sections in both files with line numbers.
sed -n '225,255p' src/common/multithreaded_fork.c
printf '\n---\n'
sed -n '400,450p' src/common/multithreaded_fork.c
printf '\n---\n'
sed -n '110,145p' src/lib/dmtcp-callback.c

Repository: mcminickpt/deep-debug

Length of output: 4628

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for any explicit unblock/removal of SIG_MULTITHREADED_FORK on the child resume path.
rg -n "SIG_MULTITHREADED_FORK|pthread_sigmask|sigprocmask|sigdelset|sigaddset" src/lib/dmtcp-callback.c src/common/multithreaded_fork.c

Repository: mcminickpt/deep-debug

Length of output: 1435

Restore the SIG_MULTITHREADED_FORK mask handling

getcontext() runs inside the SIG_MULTITHREADED_FORK handler, so the saved uc_sigmask includes that signal blocked. child_setcontext_fast() restores that mask on the new thread, and there’s no explicit unblock on the child resume path. That can leave this thread unable to participate in a later multithreaded fork and hang the process.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/dmtcp-callback.c` around lines 126 - 130, The signal-mask handling
around getcontext()/setcontext() in dmtcp callback setup drops the required
SIG_MULTITHREADED_FORK unblock on the child resume path. Update the logic in
child_setcontext_fast() and the related context-saving path so the new thread
restores the saved ucontext_t state without leaving SIG_MULTITHREADED_FORK
permanently blocked, preserving multithreaded fork participation.


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
}
Expand Down