Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 07fbff4

Browse files
tmdsstephentoub
tmds
authored and
stephentoub
committed
Use SIGCHLD to trigger Process waitpid check (#26291)
* Separate signal handling from console implementation * Pass SIGCHLD to SignalHandlerLoop * Move lock to native code * Add sigchld callback * Reap children on SIGCHLD * Reap child on unsuccesful ForkAndExecProcess * ResumeSigChld: improve comment and fix build * Handle iterator becoming invalid while reaping children * Fix comments * pal_signal.cpp -> pal_signal.c * Throw Win32Exception when InitializeSignalHandling fails * Fix alpine build: missing C void arguments * Remove ResumeSigChld * Call InitializeSignalHandling from InitializeConsoleInitializeConsole and RegisterForSigChldRegisterForSigChld * Use ReaderWriterLock to allow multiple Processes to start concurrently * throw Win32Exception when InitializeConsole fails * PR feedback * Implement SystemNative_WaitPid using waitid to check OS support * Fix WaitPid waitid implementation * Replace WaitPid with WaitIdExitedNoHang * Optimize child reaping by asking OS what children terminated instead of iterating * Implement WaitForExit for children using ManualResetEvent * Remove SystemNative_W{ExitStatus,IfExited,IfSignaled,TermSig} * Don't spin up wait loop for children * Don't create ManualResetEvent when the child has already exited * Add TestChildProcessCleanup test * TestChildProcessCleanup: 'uname' is not at '/usr/bin' on Debian systems * Fix multiple assignments of _waitStateHolder * Add TestProcessWaitStateReferenceCount * FailFast when waitid gives an unexpected return * ProcessWaitHandle: let ProcessWaitState dispose the handle * TestProcessWaitStateReferenceCount: fix test, need to WaitForExit otherwise Dispose cancels Exited event * Add TestChildProcessCleanupAfterDispose * TestProcessWaitStateReferenceCount: add retry+sleep
1 parent 3fca26c commit 07fbff4

20 files changed

Lines changed: 984 additions & 503 deletions

‎src/Common/src/Interop/Unix/System.Native/Interop.RegisterForSigChld.cs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System.Runtime.InteropServices;
6+
7+
internal partial class Interop
8+
{
9+
internal partial class Sys
10+
{
11+
internal delegate void SigChldCallback(bool reapAll);
12+
13+
[DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_RegisterForSigChld")]
14+
internal static extern bool RegisterForSigChld(SigChldCallback handler);
15+
}
16+
}

‎src/Common/src/Interop/Unix/System.Native/Interop.WaitId.cs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System;
6+
using System.Runtime.InteropServices;
7+
8+
internal static partial class Interop
9+
{
10+
internal static partial class Sys
11+
{
12+
/// <summary>
13+
/// Waits for terminated child processes.
14+
/// </summary>
15+
/// <param name="pid">The PID of a child process. -1 for any child.</param>
16+
/// <param name="status">The output exit status of the process</param>
17+
/// <param name="keepWaitable">Tells the OS to leave the child waitable or reap it.</param>
18+
/// <returns>
19+
/// 1) returns the process id of a terminated child process
20+
/// 2) if no children are waiting, 0 is returned
21+
/// 3) on error, -1 is returned
22+
/// </returns>
23+
[DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_WaitIdExitedNoHang", SetLastError = true)]
24+
internal static extern int WaitIdExitedNoHang(int pid, out int exitCode, bool keepWaitable);
25+
}
26+
}

‎src/Common/src/Interop/Unix/System.Native/Interop.WaitPid.cs‎

Lines changed: 0 additions & 49 deletions
This file was deleted.

‎src/Native/Unix/System.Native/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ set(NATIVE_SOURCES
1414
pal_random.cpp
1515
pal_runtimeinformation.cpp
1616
pal_runtimeextensions.cpp
17+
pal_signal.c
1718
pal_string.cpp
1819
pal_tcpstate.c
1920
pal_time.cpp

‎src/Native/Unix/System.Native/pal_console.cpp‎

Lines changed: 13 additions & 196 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,12 @@
44

55
#include "pal_config.h"
66
#include "pal_console.h"
7-
#include "pal_io.h"
87
#include "pal_utilities.h"
8+
#include "pal_signal.h"
99

1010
#include <assert.h>
1111
#include <errno.h>
1212
#include <fcntl.h>
13-
#include <pthread.h>
14-
#include <signal.h>
1513
#include <stdio.h>
1614
#include <stdlib.h>
1715
#include <string.h>
@@ -80,8 +78,11 @@ static struct termios g_initTermios = {}; // the initial attributes captured
8078
static struct termios g_preReadTermios = {}; // the original attributes captured before a read; valid if g_readInProgress is true
8179
static struct termios g_currTermios = {}; // the current attributes set during a read; valid if g_readInProgress is true
8280

83-
static void UninitializeConsole()
81+
void UninitializeConsole()
8482
{
83+
// pal_signal.cpp calls this on SIGQUIT/SIGINT.
84+
// This can happen when SystemNative_InitializeConsole was not called.
85+
8586
// Put the attributes back to what they were when the console was initially initialized.
8687
// We only do so, however, if we have explicitly modified the termios; doing so always
8788
// can result in problems if the app is in the background, as then attempting to call
@@ -300,36 +301,11 @@ extern "C" int32_t SystemNative_SetSignalForBreak(int32_t signalForBreak)
300301
return 0;
301302
}
302303

303-
static struct sigaction g_origSigIntHandler, g_origSigQuitHandler; // saved signal handlers for ctrl handling
304-
static struct sigaction g_origSigContHandler, g_origSigChldHandler; // saved signal handlers for reinitialization
305-
static volatile CtrlCallback g_ctrlCallback = nullptr; // Callback invoked for SIGINT/SIGQUIT
306-
static int g_signalPipe[2] = {-1, -1}; // Pipe used between signal handler and worker
307-
308-
// Signal handler for signals where we want our background thread to do the real processing.
309-
// It simply writes the signal code to a pipe that's read by the thread.
310-
static void TransferSignalToHandlerLoop(int sig, siginfo_t* siginfo, void* context)
311-
{
312-
(void)siginfo; // unused
313-
(void)context; // unused
314-
315-
// Write the signal code to the pipe
316-
uint8_t signalCodeByte = static_cast<uint8_t>(sig);
317-
ssize_t writtenBytes;
318-
while (CheckInterrupted(writtenBytes = write(g_signalPipe[1], &signalCodeByte, 1)));
319-
320-
if (writtenBytes != 1)
321-
{
322-
abort(); // fatal error
323-
}
324-
}
325-
326-
static void HandleSignalForReinitialize(int sig, siginfo_t* siginfo, void* context)
304+
void ReinitializeConsole()
327305
{
328-
// SIGCONT will be sent when we're resumed after suspension, at which point
329-
// we need to set the terminal back up. Similarly, SIGCHLD will be sent after
330-
// a child process completes, and that child could have left things in a bad state,
331-
// so we similarly need to reinitialize.
332-
assert(sig == SIGCONT || sig == SIGCHLD);
306+
// pal_signal.cpp calls this on SIGCONT/SIGCHLD.
307+
// This can happen when SystemNative_InitializeConsole was not called.
308+
// This gets called on a signal handler, we may only use async-signal-safe functions.
333309

334310
// If the process was suspended while reading, we need to
335311
// re-initialize the console for the read, as the attributes
@@ -342,172 +318,15 @@ static void HandleSignalForReinitialize(int sig, siginfo_t* siginfo, void* conte
342318

343319
// "Application mode" will also have been reset and needs to be redone.
344320
WriteKeypadXmit();
345-
346-
// Delegate to any saved handler we may have
347-
struct sigaction origHandler = sig == SIGCONT ? g_origSigContHandler : g_origSigChldHandler;
348-
if (origHandler.sa_sigaction != nullptr &&
349-
reinterpret_cast<void*>(origHandler.sa_sigaction) != reinterpret_cast<void*>(SIG_DFL) &&
350-
reinterpret_cast<void*>(origHandler.sa_sigaction) != reinterpret_cast<void*>(SIG_IGN))
351-
{
352-
origHandler.sa_sigaction(sig, siginfo, context);
353-
}
354321
}
355322

356-
// Entrypoint for the thread that handles signals where our handling
357-
// isn't signal-safe. Those signal handlers write the signal to a pipe,
358-
// which this loop reads and processes.
359-
void* SignalHandlerLoop(void* arg)
360-
{
361-
// Passed in argument is a ptr to the file descriptor
362-
// for the read end of the pipe.
363-
assert(arg != nullptr);
364-
int pipeFd = *reinterpret_cast<int*>(arg);
365-
free(arg);
366-
assert(pipeFd >= 0);
367-
368-
// Continually read a signal code from the signal pipe and process it,
369-
// until the pipe is closed.
370-
while (true)
371-
{
372-
// Read the next signal, trying again if we were interrupted
373-
uint8_t signalCode;
374-
ssize_t bytesRead;
375-
while (CheckInterrupted(bytesRead = read(pipeFd, &signalCode, 1)));
376-
377-
if (bytesRead <= 0)
378-
{
379-
// Write end of pipe was closed or another error occurred.
380-
// Regardless, no more data is available, so we close the read
381-
// end of the pipe and exit.
382-
close(pipeFd);
383-
return nullptr;
384-
}
385-
386-
assert_msg(signalCode == SIGQUIT || signalCode == SIGINT, "invalid signalCode", static_cast<int>(signalCode));
387-
388-
// We're now handling SIGQUIT and SIGINT. Invoke the callback, if we have one.
389-
CtrlCallback callback = g_ctrlCallback;
390-
int rv = callback != nullptr ? callback(signalCode == SIGQUIT ? Break : Interrupt) : 0;
391-
if (rv == 0) // callback removed or was invoked and didn't handle the signal
392-
{
393-
// In general, we now want to remove our handler and reissue the signal to
394-
// be picked up by the previously registered handler. In the most common case,
395-
// this will be the default handler, causing the process to be torn down.
396-
// It could also be a custom handle registered by other code before us.
397-
398-
if (signalCode == SIGINT)
399-
{
400-
UninitializeConsole();
401-
sigaction(SIGINT, &g_origSigIntHandler, NULL);
402-
kill(getpid(), SIGINT);
403-
}
404-
else if (signalCode == SIGQUIT)
405-
{
406-
UninitializeConsole();
407-
sigaction(SIGQUIT, &g_origSigQuitHandler, NULL);
408-
kill(getpid(), SIGQUIT);
409-
}
410-
411-
}
412-
}
413-
}
414-
415-
static void CloseSignalHandlingPipe()
416-
{
417-
assert(g_signalPipe[0] >= 0);
418-
assert(g_signalPipe[1] >= 0);
419-
close(g_signalPipe[0]);
420-
close(g_signalPipe[1]);
421-
g_signalPipe[0] = -1;
422-
g_signalPipe[1] = -1;
423-
}
424-
425-
static bool InitializeSignalHandling()
323+
extern "C" int32_t SystemNative_InitializeConsole()
426324
{
427-
// Create a pipe we'll use to communicate with our worker
428-
// thread. We can't do anything interesting in the signal handler,
429-
// so we instead send a message to another thread that'll do
430-
// the handling work.
431-
if (SystemNative_Pipe(g_signalPipe, PAL_O_CLOEXEC) != 0)
432-
{
433-
return false;
434-
}
435-
assert(g_signalPipe[0] >= 0);
436-
assert(g_signalPipe[1] >= 0);
437-
438-
// Create a small object to pass the read end of the pipe to the worker.
439-
int* readFdPtr = reinterpret_cast<int*>(malloc(sizeof(int)));
440-
if (readFdPtr == nullptr)
441-
{
442-
CloseSignalHandlingPipe();
443-
errno = ENOMEM;
444-
return false;
445-
}
446-
*readFdPtr = g_signalPipe[0];
447-
448-
// The pipe is created. Create the worker thread.
449-
pthread_t handlerThread;
450-
if (pthread_create(&handlerThread, nullptr, SignalHandlerLoop, readFdPtr) != 0)
325+
if (!InitializeSignalHandling())
451326
{
452-
int err = errno;
453-
free(readFdPtr);
454-
CloseSignalHandlingPipe();
455-
errno = err;
456-
return false;
327+
return 0;
457328
}
458329

459-
// Finally, register our signal handlers
460-
struct sigaction newAction;
461-
memset(&newAction, 0, sizeof(struct sigaction));
462-
newAction.sa_flags = SA_RESTART | SA_SIGINFO;
463-
464-
sigemptyset(&newAction.sa_mask);
465-
int rv;
466-
467-
// Hook up signal handlers for use with ctrl-C / ctrl-Break handling
468-
// We don't handle ignored signals. If we'd setup a handler, our child processes
469-
// would reset to the default on exec causing them to terminate on these signals.
470-
newAction.sa_sigaction = &TransferSignalToHandlerLoop;
471-
rv = sigaction(SIGINT, NULL, &g_origSigIntHandler);
472-
assert(rv == 0);
473-
if (reinterpret_cast<void*>(g_origSigIntHandler.sa_sigaction) != reinterpret_cast<void*>(SIG_IGN))
474-
{
475-
rv = sigaction(SIGINT, &newAction, NULL);
476-
assert(rv == 0);
477-
}
478-
rv = sigaction(SIGQUIT, NULL, &g_origSigQuitHandler);
479-
assert(rv == 0);
480-
if (reinterpret_cast<void*>(g_origSigQuitHandler.sa_sigaction) != reinterpret_cast<void*>(SIG_IGN))
481-
{
482-
rv = sigaction(SIGQUIT, &newAction, NULL);
483-
assert(rv == 0);
484-
}
485-
486-
// Hook up signal handlers for use with signals that require us to reinitialize the terminal
487-
newAction.sa_sigaction = &HandleSignalForReinitialize;
488-
rv = sigaction(SIGCONT, &newAction, &g_origSigContHandler);
489-
assert(rv == 0);
490-
rv = sigaction(SIGCHLD, &newAction, &g_origSigChldHandler);
491-
assert(rv == 0);
492-
493-
return true;
494-
}
495-
496-
extern "C" void SystemNative_RegisterForCtrl(CtrlCallback callback)
497-
{
498-
assert(callback != nullptr);
499-
assert(g_ctrlCallback == nullptr);
500-
g_ctrlCallback = callback;
501-
}
502-
503-
extern "C" void SystemNative_UnregisterForCtrl()
504-
{
505-
assert(g_ctrlCallback != nullptr);
506-
g_ctrlCallback = nullptr;
507-
}
508-
509-
extern "C" int32_t SystemNative_InitializeConsole()
510-
{
511330
if (tcgetattr(STDIN_FILENO, &g_initTermios) >= 0)
512331
{
513332
g_haveInitTermios = true;
@@ -520,7 +339,5 @@ extern "C" int32_t SystemNative_InitializeConsole()
520339
}
521340
atexit(UninitializeConsole);
522341

523-
// Do all initialization needed for the console. Right now that's just
524-
// initializing the signal handling thread.
525-
return InitializeSignalHandling() ? 1 : 0;
342+
return 1;
526343
}

0 commit comments

Comments
 (0)