Skip to content

Commit 1d1b1a5

Browse files
sporksmithsporksmith
authored
ChildPidWatcher: remove dependence on pidfd_open (#1539)
pidfd_open was only added in Linux 5.3. There are still some platforms supported by Shadow that may not have a new enough kernel to use it. Fixes #1532
1 parent 9cea24a commit 1d1b1a5

3 files changed

Lines changed: 328 additions & 117 deletions

File tree

‎src/main/bindings/c/bindings.h‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,28 @@ struct ChildPidWatcher *childpidwatcher_new(void);
556556

557557
void childpidwatcher_free(struct ChildPidWatcher *watcher);
558558

559+
int32_t childpidwatcher_forkWatchable(const struct ChildPidWatcher *watcher,
560+
void (*child_fn)(void*),
561+
void *child_fn_data);
562+
563+
// Register interest in `pid`, and associate it with `read_fd`.
564+
//
565+
// `read_fd` should be the read end of a pipe, whose write end is owned
566+
// *solely* by `pid`, causing `read_fd` to become invalid when `pid` exits.
567+
// In a multi-threaded program care must be taken to prevent a concurrent
568+
// fork from leaking the write end of the pipe into other children. One way
569+
// to avoid this is to use O_CLOEXEC when creating the pipe, and then unset
570+
// O_CLOEXEC in the child before calling exec.
571+
//
572+
// Be sure to close the parent's write-end of the pipe.
573+
//
574+
// Takes ownership of `read_fd`, and will close it when appropriate.
575+
void childpidwatcher_registerPid(const struct ChildPidWatcher *watcher,
576+
int32_t pid,
577+
int32_t read_fd);
578+
579+
void childpidwatcher_unregisterPid(const struct ChildPidWatcher *watcher, int32_t pid);
580+
559581
// Call `callback` exactly once from another thread after the child `pid`
560582
// has exited, including if it has already exited. Does *not* reap the
561583
// child itself.

‎src/main/host/thread_preload.c‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,13 @@ static gchar** _add_shadow_pid_to_env(gchar** envp) {
9595

9696
static pid_t _threadpreload_fork_exec(ThreadPreload* thread, const char* file, char* const argv[],
9797
char* const envp[], const char* workingDir) {
98+
// For childpidwatcher. We must create them O_CLOEXEC to prevent them from
99+
// "leaking" into a concurrently forked child.
100+
int pipefd[2];
101+
if (pipe2(pipefd, O_CLOEXEC)) {
102+
utility_panic("pipe2: %s", g_strerror(errno));
103+
}
104+
98105
// vfork has superior performance to fork with large workloads.
99106
pid_t pid = vfork();
100107

@@ -111,6 +118,11 @@ static pid_t _threadpreload_fork_exec(ThreadPreload* thread, const char* file, c
111118
case 0: {
112119
// child
113120

121+
// *Don't* close the write end of the pipe on exec.
122+
if (fcntl(pipefd[1], F_SETFD, 0)) {
123+
die_after_vfork();
124+
}
125+
114126
// Set the working directory
115127
if (chdir(workingDir) < 0) {
116128
die_after_vfork();
@@ -124,10 +136,20 @@ static pid_t _threadpreload_fork_exec(ThreadPreload* thread, const char* file, c
124136
die_after_vfork();
125137
}
126138
default: // parent
127-
debug("started process %s with PID %d", file, pid);
128-
return pid;
129139
break;
130140
}
141+
142+
// *Must* close the write-end of the pipe, so that the child's copy is the
143+
// last remaining one, allowing the read-end to be notified when the child
144+
// exits.
145+
if (close(pipefd[1])) {
146+
utility_panic("close: %s", g_strerror(errno));
147+
}
148+
149+
childpidwatcher_registerPid(worker_getChildPidWatcher(), pid, pipefd[0]);
150+
151+
debug("started process %s with PID %d", file, pid);
152+
return pid;
131153
}
132154

133155
static void _threadpreload_cleanup(ThreadPreload* thread) {
@@ -314,6 +336,8 @@ void threadpreload_handleProcessExit(Thread* base) {
314336
ThreadPreload* thread = _threadToThreadPreload(base);
315337
// TODO [rwails]: come back and make this logic more solid
316338

339+
childpidwatcher_unregisterPid(worker_getChildPidWatcher(), base->nativePid);
340+
317341
/* make sure we cleanup circular refs */
318342
if (thread->base.sys) {
319343
syscallhandler_unref(thread->base.sys);

0 commit comments

Comments
 (0)