Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Use vfork() improve performance when starting processes. #33289

Merged
merged 10 commits into from
Feb 15, 2019
1 change: 1 addition & 0 deletions src/Native/Unix/Common/pal_config.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#cmakedefine01 HAVE_GETIFADDRS
#cmakedefine01 HAVE_UTSNAME_DOMAINNAME
#cmakedefine01 HAVE_STAT64
#cmakedefine01 HAVE_VFORK
#cmakedefine01 HAVE_PIPE2
#cmakedefine01 HAVE_STAT_BIRTHTIME
#cmakedefine01 HAVE_STAT_TIMESPEC
Expand Down
100 changes: 87 additions & 13 deletions src/Native/Unix/System.Native/pal_process.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@
#if HAVE_PIPE2
#include <fcntl.h>
#endif
#if !HAVE_PIPE2
#include <pthread.h>
#endif

#if HAVE_SCHED_SETAFFINITY || HAVE_SCHED_GETAFFINITY
#include <sched.h>
Expand Down Expand Up @@ -165,8 +163,14 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename,
bool haveProcessCreateLock = false;
#endif
bool success = true;
int stdinFds[2] = {-1, -1}, stdoutFds[2] = {-1, -1}, stderrFds[2] = {-1, -1}, waitForChildToExecPipe[2] = {-1, -1};
int processId = -1;
int stdinFds[2] = {-1, -1}, stdoutFds[2] = {-1, -1}, stderrFds[2] = {-1, -1}, waitForChildToExecPipe[2] = {-1, 1};
pid_t processId = -1;
int thread_cancel_state;
sigset_t signal_set;
sigset_t old_signal_set;

// None of this code can be cancelled without leaking handles, so just don't allow it
pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &thread_cancel_state);

// Validate arguments
if (NULL == filename || NULL == argv || NULL == envp || NULL == stdinFd || NULL == stdoutFd ||
Expand Down Expand Up @@ -233,15 +237,73 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename,
SystemNative_Pipe(waitForChildToExecPipe, PAL_O_CLOEXEC);
#endif

// Fork the child process
if ((processId = fork()) == -1)
// The fork child must not be signalled until it calls exec(): our signal handlers do not
// handle being raised in the child process correctly
sigfillset(&signal_set);
Copy link
Member

Choose a reason for hiding this comment

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

A nit - use SIGALL_SET instead of creating your own set

Copy link
Contributor Author

Choose a reason for hiding this comment

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

joshua@novaϟ find /usr -name '*.h' -print0 | xargs -0 grep -i SIGALL_SET /dev/null
joshua@novaϟ

SIGALL_SET is not in my system header files.

pthread_sigmask(SIG_SETMASK, &signal_set, &old_signal_set);
Copy link
Member

Choose a reason for hiding this comment

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

What happens if a signal like SIGCHLD is received between here and the later sigmask call?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then it gets delivered on the return from pthread_sigmask, but the only way for that to happen is an explicit kill.

Copy link
Member

Choose a reason for hiding this comment

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

SIGCHLD will be handled by a different thread.


#if HAVE_VFORK
// This platform has vfork(). vfork() is either a synonym for fork or provides shared memory
// semantics. For a one gigabyte process/ the expected performance gain of using shared memory
// vfork() rather than fork() is 99.5% merely due to avoiding page faults as the kernel does not
// need to set all writable pages in the parent process to copy-on-write because the child process
// is allowed to write to the parent process memory pages.

// The thing to remember about shared memory vfork() is the documentation is way out of date.
// It does the following things:
// * creates a new process in the memory space of the calling process.
// * blocks the calling thread (not process!) in an uninterruptable sleep
// * sets up the process records so the following happen:
// + execve() replaces the memory space in the child and unblocks the parent
// + process exit by any means unblocks the parent
// + ptrace() makes a security demand against the parent process
// + accessing the terminal with read() or write() fail in system-dependent ways
// Due to lack of documentation, setting signal handlers in the vfork() child is a bad idea. We don't
// do this, but it's worth pointing out.

// All platforms that provide shared memory vfork() check the parent process's context when
// ptrace() is used on the child, thus making setuid() safe to use after vfork(). The fabled vfork()
// security hole is the other way around; if a multithreaded host were to execute setuid()
// on another thread while a vfork() child is still pending, bad things are possible; however we
// do not do that.
Copy link
Member

Choose a reason for hiding this comment

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

Is this important to mention? Can you phrase it as something we mustn't do?


if ((processId = vfork()) == 0) // processId == 0 if this is child process
#else
if ((processId = fork()) == 0) // processId == 0 if this is child process
#endif
{
success = false;
goto done;
}
// It turns out that child processes depend on their sigmask being set to something sane rather than mask all.
// On the other hand, we have to mask all to avoid our own signal handlers running in the child process, writing
// to the pipe, and waking up the handling thread in the parent process. This also avoids third-party code getting
// equally confused.
// Remove all signals, then restore signal mask.
// Since we are in a vfork() child, the only safe signal values are SIG_DFL and SIG_IGN. See man 3 libthr on BSD.
// "The implementation interposes the user-installed signal(3) handlers....to pospone signal delivery to threads
// which entered (libthr-internal) critical sections..." We want to pass SIG_DFL anyway.
sigset_t junk_signal_set;
struct sigaction sa_default;
struct sigaction sa_old;
memset(&sa_default, 0, sizeof(sa_default)); // On some architectures, sa_mask is a struct so assigning zero to it doesn't compile
sa_default.sa_handler = SIG_DFL;
for (int sig = 1; sig < NSIG; ++sig)
{
if (sig == SIGKILL || sig == SIGSTOP)
{
continue;
}
if (!sigaction(sig, NULL, &sa_old))
{
void (*oldhandler)(int) = (sa_old.sa_flags & SA_SIGINFO) ? (void (*)(int))sa_old.sa_sigaction : sa_old.sa_handler;
if (oldhandler != SIG_IGN && oldhandler != SIG_DFL)
{
// It has a custom handler, put the default handler back.
// We check first th preserve flags on default handlers.
sigaction(sig, &sa_default, NULL);
}
}
}
pthread_sigmask(SIG_SETMASK, &old_signal_set, &junk_signal_set); // Not all architectures allow NULL here

if (processId == 0) // processId == 0 if this is child process
{
// For any redirections that should happen, dup the pipe descriptors onto stdin/out/err.
// We don't need to explicitly close out the old pipe descriptors as they will be closed on the 'execve' call.
if ((redirectStdin && Dup2WithInterruptedRetry(stdinFds[READ_END_OF_PIPE], STDIN_FILENO) == -1) ||
Expand Down Expand Up @@ -275,6 +337,16 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename,
ExitChild(waitForChildToExecPipe[WRITE_END_OF_PIPE], errno); // execve failed
}

// Restore signal mask in the parent process immediately after fork() or vfork() call
pthread_sigmask(SIG_SETMASK, &old_signal_set, &signal_set);

if (processId < 0)
{
// failed
success = false;
goto done;
}

// This is the parent process. processId == pid of the child
*childPid = processId;
*stdinFd = stdinFds[WRITE_END_OF_PIPE];
Expand Down Expand Up @@ -336,10 +408,12 @@ done:;
*childPid = -1;

errno = priorErrno;
return -1;
}

return 0;
// Restore thread cancel state
pthread_setcancelstate(thread_cancel_state, &thread_cancel_state);

return success ? 0 : -1;
}

FILE* SystemNative_POpen(const char* command, const char* type)
Expand Down
5 changes: 5 additions & 0 deletions src/Native/Unix/configure.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,11 @@ check_symbol_exists(
sys/stat.h
HAVE_STAT64)

check_symbol_exists(
vfork
unistd.h
HAVE_VFORK)

check_symbol_exists(
pipe2
unistd.h
Expand Down