Skip to content

Commit

Permalink
Serialize createdump core dump generation (dotnet#90130)
Browse files Browse the repository at this point in the history
* Serialize createdump core dump generation

Only allow one thread at a time to generate a core dump.

Issue: dotnet#82989

* Code review feedback. Move serializing code into PROCCreateCrashDump

* Code review feedback - put while (true) around poll()'s
  • Loading branch information
mikem8361 authored Aug 8, 2023
1 parent 39f4921 commit 05c5144
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 13 deletions.
11 changes: 9 additions & 2 deletions src/coreclr/pal/src/exception/signal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -388,8 +388,15 @@ static void invoke_previous_action(struct sigaction* action, int code, siginfo_t
{
if (signalRestarts)
{
// Shutdown and create the core dump before we restore the signal to the default handler.
PROCNotifyProcessShutdown(IsRunningOnAlternateStack(context));

PROCCreateCrashDumpIfEnabled(code, siginfo, true);

// Restore the original and restart h/w exception.
restore_signal(code, action);

return;
}
else
{
Expand All @@ -413,7 +420,7 @@ static void invoke_previous_action(struct sigaction* action, int code, siginfo_t

PROCNotifyProcessShutdown(IsRunningOnAlternateStack(context));

PROCCreateCrashDumpIfEnabled(code, siginfo);
PROCCreateCrashDumpIfEnabled(code, siginfo, true);
}

/*++
Expand Down Expand Up @@ -746,7 +753,7 @@ static void sigterm_handler(int code, siginfo_t *siginfo, void *context)
DWORD val = 0;
if (enableDumpOnSigTerm.IsSet() && enableDumpOnSigTerm.TryAsInteger(10, val) && val == 1)
{
PROCCreateCrashDumpIfEnabled(code, siginfo);
PROCCreateCrashDumpIfEnabled(code, siginfo, false);
}
// g_pSynchronizationManager shouldn't be null if PAL is initialized.
_ASSERTE(g_pSynchronizationManager != nullptr);
Expand Down
4 changes: 3 additions & 1 deletion src/coreclr/pal/src/include/pal/process.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,10 +178,12 @@ VOID PROCNotifyProcessShutdown(bool isExecutingOnAltStack = false);
Parameters:
signal - POSIX signal number
siginfo - POSIX signal info or nullptr
serialize - allow only one thread to generate core dump
(no return value)
--*/
VOID PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo);
VOID PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo, bool serialize);

/*++
Function:
Expand Down
51 changes: 41 additions & 10 deletions src/coreclr/pal/src/thread/process.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,9 @@ LPWSTR g_lpwstrAppDir = NULL;
// Thread ID of thread that has started the ExitProcess process
Volatile<LONG> terminator = 0;

// Id of thread generating a core dump
Volatile<LONG> g_crashingThreadId = 0;

// Process and session ID of this process.
DWORD gPID = (DWORD) -1;
DWORD gSID = (DWORD) -1;
Expand Down Expand Up @@ -1194,7 +1197,10 @@ ExitProcess(
Update: [TODO] PROCSuspendOtherThreads has been removed. Can this
code be changed? */
WARN("termination already started from another thread; blocking.\n");
poll(NULL, 0, INFTIM);
while (true)
{
poll(NULL, 0, INFTIM);
}
}

/* ExitProcess may be called even if PAL is not initialized.
Expand Down Expand Up @@ -2175,20 +2181,40 @@ PROCBuildCreateDumpCommandLine(
Function:
PROCCreateCrashDump
Creates crash dump of the process. Can be called from the
unhandled native exception handler.
Creates crash dump of the process. Can be called from the unhandled
native exception handler. Allows only one thread to generate the core
dump if serialize is true.
(no return value)
Return:
TRUE - succeeds, FALSE - fails
--*/
BOOL
PROCCreateCrashDump(
std::vector<const char*>& argv,
LPSTR errorMessageBuffer,
INT cbErrorMessageBuffer)
INT cbErrorMessageBuffer,
bool serialize)
{
_ASSERTE(argv.size() > 0);
_ASSERTE(errorMessageBuffer == nullptr || cbErrorMessageBuffer > 0);

if (serialize)
{
size_t currentThreadId = THREADSilentGetCurrentThreadId();
size_t previousThreadId = InterlockedCompareExchange(&g_crashingThreadId, currentThreadId, 0);
if (previousThreadId != 0)
{
// Should never reenter or recurse
_ASSERTE(previousThreadId != currentThreadId);

// The first thread generates the crash info and any other threads are blocked
while (true)
{
poll(NULL, 0, INFTIM);
}
}
}

int pipe_descs[2];
if (pipe(pipe_descs) == -1)
{
Expand Down Expand Up @@ -2413,7 +2439,7 @@ PAL_GenerateCoreDump(
BOOL result = PROCBuildCreateDumpCommandLine(argvCreateDump, &program, &pidarg, dumpName, nullptr, dumpType, flags);
if (result)
{
result = PROCCreateCrashDump(argvCreateDump, errorMessageBuffer, cbErrorMessageBuffer);
result = PROCCreateCrashDump(argvCreateDump, errorMessageBuffer, cbErrorMessageBuffer, false);
}
free(program);
free(pidarg);
Expand All @@ -2429,11 +2455,13 @@ PAL_GenerateCoreDump(
Parameters:
signal - POSIX signal number
siginfo - POSIX signal info or nullptr
serialize - allow only one thread to generate core dump
(no return value)
--*/
VOID
PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo)
PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo, bool serialize)
{
// If enabled, launch the create minidump utility and wait until it completes
if (!g_argvCreateDump.empty())
Expand Down Expand Up @@ -2491,7 +2519,7 @@ PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo)
argv.push_back(nullptr);
}

PROCCreateCrashDump(argv, nullptr, 0);
PROCCreateCrashDump(argv, nullptr, 0, serialize);

free(signalArg);
free(crashThreadArg);
Expand Down Expand Up @@ -2520,7 +2548,7 @@ PROCAbort(int signal, siginfo_t* siginfo)
// Do any shutdown cleanup before aborting or creating a core dump
PROCNotifyProcessShutdown();

PROCCreateCrashDumpIfEnabled(signal, siginfo);
PROCCreateCrashDumpIfEnabled(signal, siginfo, true);

// Restore all signals; the SIGABORT handler to prevent recursion and
// the others to prevent multiple core dumps from being generated.
Expand Down Expand Up @@ -3222,7 +3250,10 @@ CorUnix::TerminateCurrentProcessNoExit(BOOL bTerminateUnconditionally)
TerminateProcess won't call DllMain, so there's no danger to get
caught in an infinite loop */
WARN("termination already started from another thread; blocking.\n");
poll(NULL, 0, INFTIM);
while (true)
{
poll(NULL, 0, INFTIM);
}
}

/* Try to lock the initialization count to prevent multiple threads from
Expand Down

0 comments on commit 05c5144

Please sign in to comment.