diff --git a/toolkit/crashreporter/breakpad-client/linux/crash_generation/crash_generation_server.cc b/toolkit/crashreporter/breakpad-client/linux/crash_generation/crash_generation_server.cc index 26e696afe824..7d51e0ecb1a7 100644 --- a/toolkit/crashreporter/breakpad-client/linux/crash_generation/crash_generation_server.cc +++ b/toolkit/crashreporter/breakpad-client/linux/crash_generation/crash_generation_server.cc @@ -273,9 +273,8 @@ CrashGenerationServer::ClientEvent(short revents) return true; } + ClientInfo info(crashing_pid, this); if (dump_callback_) { - ClientInfo info(crashing_pid, this); - dump_callback_(dump_context_, info, minidump_filename); } @@ -283,6 +282,10 @@ CrashGenerationServer::ClientEvent(short revents) // (Closing this will make the child's sys_read unblock and return 0.) close(signal_fd); + if (exit_callback_) { + exit_callback_(exit_context_, info); + } + return true; } diff --git a/toolkit/crashreporter/breakpad-client/windows/crash_generation/crash_generation_server.cc b/toolkit/crashreporter/breakpad-client/windows/crash_generation/crash_generation_server.cc index 5beeb56003a0..56b85543c420 100644 --- a/toolkit/crashreporter/breakpad-client/windows/crash_generation/crash_generation_server.cc +++ b/toolkit/crashreporter/breakpad-client/windows/crash_generation/crash_generation_server.cc @@ -101,6 +101,7 @@ CrashGenerationServer::CrashGenerationServer( void* connect_context, OnClientDumpRequestCallback dump_callback, void* dump_context, + OnClientDumpWrittenCallback written_callback, OnClientExitedCallback exit_callback, void* exit_context, OnClientUploadRequestCallback upload_request_callback, @@ -116,6 +117,7 @@ CrashGenerationServer::CrashGenerationServer( connect_context_(connect_context), dump_callback_(dump_callback), dump_context_(dump_context), + written_callback_(written_callback), exit_callback_(exit_callback), exit_context_(exit_context), upload_request_callback_(upload_request_callback), @@ -892,11 +894,15 @@ void CrashGenerationServer::HandleDumpRequest(const ClientInfo& client_info) { } } - SetEvent(client_info.dump_generated_handle()); - if (dump_callback_ && execute_callback) { dump_callback_(dump_context_, client_info, dump_path); } + + SetEvent(client_info.dump_generated_handle()); + + if (written_callback_ && execute_callback) { + written_callback_(dump_context_, client_info); + } } void CrashGenerationServer::set_include_context_heap(bool enabled) { diff --git a/toolkit/crashreporter/breakpad-client/windows/crash_generation/crash_generation_server.h b/toolkit/crashreporter/breakpad-client/windows/crash_generation/crash_generation_server.h index a039244e9456..cc1912cf3c1a 100644 --- a/toolkit/crashreporter/breakpad-client/windows/crash_generation/crash_generation_server.h +++ b/toolkit/crashreporter/breakpad-client/windows/crash_generation/crash_generation_server.h @@ -57,6 +57,9 @@ class CrashGenerationServer { const ClientInfo& client_info, const std::wstring& file_path); + typedef void (*OnClientDumpWrittenCallback)(void* context, + const ClientInfo& client_info); + typedef void (*OnClientExitedCallback)(void* context, const ClientInfo& client_info); @@ -72,8 +75,9 @@ class CrashGenerationServer { // the Everyone group read access on the pipe. // Parameter connect_callback: Callback for a new client connection. // Parameter connect_context: Context for client connection callback. - // Parameter crash_callback: Callback for a client crash dump request. - // Parameter crash_context: Context for client crash dump request callback. + // Parameter dump_callback: Callback for a client crash dump request. + // Parameter dump_context: Context for client crash dump request callback. + // Parameter written_callback: Callback called after a crash dump was written. // Parameter exit_callback: Callback for client process exit. // Parameter exit_context: Context for client exit callback. // Parameter generate_dumps: Whether to automatically generate dumps. @@ -88,6 +92,7 @@ class CrashGenerationServer { void* connect_context, OnClientDumpRequestCallback dump_callback, void* dump_context, + OnClientDumpWrittenCallback written_callback, OnClientExitedCallback exit_callback, void* exit_context, OnClientUploadRequestCallback upload_request_callback, @@ -256,6 +261,9 @@ class CrashGenerationServer { // Context for client dump request callback. void* dump_context_; + // Callback for a client dump written. + OnClientDumpWrittenCallback written_callback_; + // Callback for client process exit. OnClientExitedCallback exit_callback_; diff --git a/toolkit/crashreporter/nsExceptionHandler.cpp b/toolkit/crashreporter/nsExceptionHandler.cpp index 572c18714ac6..d282a9449aa9 100644 --- a/toolkit/crashreporter/nsExceptionHandler.cpp +++ b/toolkit/crashreporter/nsExceptionHandler.cpp @@ -276,7 +276,8 @@ struct ChildProcessData : public nsUint32HashKey { explicit ChildProcessData(KeyTypePointer aKey) : nsUint32HashKey(aKey), sequence(0), - annotations(nullptr) + annotations(nullptr), + minidumpOnly(false) #ifdef MOZ_CRASHREPORTER_INJECTOR , callback(nullptr) @@ -289,6 +290,7 @@ struct ChildProcessData : public nsUint32HashKey { // indicate which process crashed first. uint32_t sequence; UniquePtr annotations; + bool minidumpOnly; // If true then no annotations are present #ifdef MOZ_CRASHREPORTER_INJECTOR InjectorCrashCallback* callback; #endif @@ -3184,11 +3186,9 @@ static void ReadExceptionTimeAnnotations(AnnotationTable& aAnnotations, } } -static void PopulateContentProcessAnnotations(AnnotationTable& aAnnotations, - uint32_t aPid) { +static void PopulateContentProcessAnnotations(AnnotationTable& aAnnotations) { MergeContentCrashAnnotations(aAnnotations); AddCommonAnnotations(aAnnotations); - ReadExceptionTimeAnnotations(aAnnotations, aPid); } // It really only makes sense to call this function when @@ -3255,13 +3255,13 @@ static void OnChildProcessDumpRequested(void* aContext, bool runCallback; #endif { - MutexAutoLock lock(*dumpMapLock); + dumpMapLock->Lock(); ChildProcessData* pd = pidToMinidump->PutEntry(pid); MOZ_ASSERT(!pd->minidump); pd->minidump = minidump; pd->sequence = ++crashSequence; pd->annotations = MakeUnique(); - PopulateContentProcessAnnotations(*(pd->annotations), pid); + PopulateContentProcessAnnotations(*(pd->annotations)); #ifdef MOZ_CRASHREPORTER_INJECTOR runCallback = nullptr != pd->callback; #endif @@ -3272,6 +3272,17 @@ static void OnChildProcessDumpRequested(void* aContext, } } +static void OnChildProcessDumpWritten(void* aContext, + const ClientInfo& aClientInfo) { + uint32_t pid = aClientInfo.pid(); + ChildProcessData* pd = pidToMinidump->GetEntry(pid); + MOZ_ASSERT(pd); + if (!pd->minidumpOnly) { + ReadExceptionTimeAnnotations(*(pd->annotations), pid); + } + dumpMapLock->Unlock(); +} + static bool OOPInitialized() { return pidToMinidump != nullptr; } void OOPInit() { @@ -3310,7 +3321,7 @@ void OOPInit() { std::wstring(NS_ConvertASCIItoUTF16(childCrashNotifyPipe).get()), nullptr, // default security attributes nullptr, nullptr, // we don't care about process connect here - OnChildProcessDumpRequested, nullptr, nullptr, + OnChildProcessDumpRequested, nullptr, OnChildProcessDumpWritten, nullptr, nullptr, // we don't care about process exit here nullptr, nullptr, // we don't care about upload request here true, // automatically generate dumps @@ -3328,9 +3339,8 @@ void OOPInit() { const std::string dumpPath = gExceptionHandler->minidump_descriptor().directory(); crashServer = new CrashGenerationServer( - serverSocketFd, OnChildProcessDumpRequested, nullptr, nullptr, - nullptr, // we don't care about process exit here - true, &dumpPath); + serverSocketFd, OnChildProcessDumpRequested, nullptr, + OnChildProcessDumpWritten, nullptr, true, &dumpPath); #elif defined(XP_MACOSX) childCrashNotifyPipe = mozilla::Smprintf("gecko-crash-server-pipe.%i", @@ -3338,11 +3348,11 @@ void OOPInit() { .release(); const std::string dumpPath = gExceptionHandler->dump_path(); - crashServer = new CrashGenerationServer(childCrashNotifyPipe, nullptr, - nullptr, OnChildProcessDumpRequested, - nullptr, nullptr, nullptr, - true, // automatically generate dumps - dumpPath); + crashServer = new CrashGenerationServer( + childCrashNotifyPipe, nullptr, nullptr, OnChildProcessDumpRequested, + nullptr, OnChildProcessDumpWritten, nullptr, + true, // automatically generate dumps + dumpPath); #endif if (!crashServer->Start()) MOZ_CRASH("can't start crash reporter server()"); @@ -3411,6 +3421,7 @@ void InjectCrashReporterIntoProcess(DWORD processID, ChildProcessData* pd = pidToMinidump->PutEntry(processID); MOZ_ASSERT(!pd->minidump && !pd->callback); pd->callback = cb; + pd->minidumpOnly = true; } nsCOMPtr r = new InjectCrashRunnable(processID); @@ -3563,7 +3574,7 @@ bool TakeMinidumpForChild(uint32_t childPid, nsIFile** dump, NS_IF_ADDREF(*dump = pd->minidump); // Only Flash process minidumps don't have annotations. Once we get rid of // the Flash processes this check will become redundant. - if (pd->annotations) { + if (!pd->minidumpOnly) { aAnnotations = *(pd->annotations); } if (aSequence) {