diff --git a/dom/plugins/PluginModuleParent.cpp b/dom/plugins/PluginModuleParent.cpp --- a/dom/plugins/PluginModuleParent.cpp +++ b/dom/plugins/PluginModuleParent.cpp @@ -90,115 +90,99 @@ PluginModuleParent::LoadModule(const cha PluginModuleParent::PluginModuleParent(const char* aFilePath) : mSubprocess(new PluginProcessParent(aFilePath)) , mShutdown(false) , mNPNIface(NULL) , mPlugin(NULL) , mProcessStartTime(time(NULL)) - , mPluginCrashedTask(NULL) + , mTaskFactory(this) { NS_ASSERTION(mSubprocess, "Out of memory!"); if (!mValidIdentifiers.Init()) { NS_ERROR("Out of memory"); } nsContentUtils::RegisterPrefCallback(kTimeoutPref, TimeoutChanged, this); } PluginModuleParent::~PluginModuleParent() { NS_ASSERTION(OkToCleanup(), "unsafe destruction"); - if (mPluginCrashedTask) { - mPluginCrashedTask->Cancel(); - mPluginCrashedTask = 0; - } - if (!mShutdown) { NS_WARNING("Plugin host deleted the module without shutting down."); NPError err; NP_Shutdown(&err); } NS_ASSERTION(mShutdown, "NP_Shutdown didn't"); if (mSubprocess) { mSubprocess->Delete(); mSubprocess = nsnull; } nsContentUtils::UnregisterPrefCallback(kTimeoutPref, TimeoutChanged, this); } void -PluginModuleParent::WriteExtraDataEntry(nsIFileOutputStream* stream, - const char* key, - const char* value) +PluginModuleParent::WritePluginExtraDataForMinidump(const nsAString& id) { - PRUint32 written; - stream->Write(key, strlen(key), &written); - stream->Write("=", 1, &written); - stream->Write(value, strlen(value), &written); - stream->Write("\n", 1, &written); -} + typedef nsDependentCString CS; -void -PluginModuleParent::WriteExtraDataForMinidump(nsIFile* dumpFile) -{ - // get a reference to the extra file, and add some more entries - nsCOMPtr extraFile; - nsresult rv = dumpFile->Clone(getter_AddRefs(extraFile)); - if (NS_FAILED(rv)) + CrashReporter::AnnotationTable notes; + if (!notes.Init(32)) return; - nsAutoString leafName; - rv = extraFile->GetLeafName(leafName); - if (NS_FAILED(rv)) - return; + notes.Put(CS("ProcessType"), CS("plugin")); - leafName.Replace(leafName.Length() - 3, 3, - NS_LITERAL_STRING("extra")); - rv = extraFile->SetLeafName(leafName); - if (NS_FAILED(rv)) - return; - - nsCOMPtr stream = - do_CreateInstance("@mozilla.org/network/file-output-stream;1"); - // PR_WRONLY | PR_APPEND - rv = stream->Init(extraFile, 0x12, 0600, 0); - if (NS_FAILED(rv)) - return; - WriteExtraDataEntry(stream, "ProcessType", "plugin"); char startTime[32]; sprintf(startTime, "%lld", static_cast(mProcessStartTime)); - WriteExtraDataEntry(stream, "StartupTime", startTime); + notes.Put(CS("StartupTime"), CS(startTime)); // Get the plugin filename, try to get just the file leafname const std::string& pluginFile = mSubprocess->GetPluginFilePath(); size_t filePos = pluginFile.rfind(FILE_PATH_SEPARATOR); if (filePos == std::string::npos) filePos = 0; else filePos++; - WriteExtraDataEntry(stream, "PluginFilename", - pluginFile.substr(filePos).c_str()); + notes.Put(CS("PluginFilename"), CS(pluginFile.substr(filePos).c_str())); + //TODO: add plugin name and version: bug 539841 // (as PluginName, PluginVersion) - WriteExtraDataEntry(stream, "PluginName", ""); - WriteExtraDataEntry(stream, "PluginVersion", ""); + notes.Put(CS("PluginName"), CS("")); + notes.Put(CS("PluginVersion"), CS("")); - if (!mCrashNotes.IsEmpty()) { - WriteExtraDataEntry(stream, "Notes", mCrashNotes.get()); - } + if (!mCrashNotes.IsEmpty()) + notes.Put(CS("Notes"), CS(mCrashNotes.get())); - stream->Close(); + if (!mHangID.IsEmpty()) + notes.Put(CS("HangID"), NS_ConvertUTF16toUTF8(mHangID)); + + if (!CrashReporter::AppendExtraData(id, notes)) + NS_WARNING("problem appending plugin data to .extra"); } +void +PluginModuleParent::WriteExtraDataForHang() +{ + // this writes HangID + WritePluginExtraDataForMinidump(mPluginDumpID); + + CrashReporter::AnnotationTable notes; + if (!notes.Init(4)) + return; + + notes.Put(nsDependentCString("HangID"), NS_ConvertUTF16toUTF8(mHangID)); + if (!CrashReporter::AppendExtraData(mBrowserDumpID, notes)) + NS_WARNING("problem appending browser data to .extra"); +} bool PluginModuleParent::RecvAppendNotesToCrashReport(const nsCString& aNotes) { mCrashNotes.Append(aNotes); return true; } @@ -222,88 +206,94 @@ PluginModuleParent::CleanupFromTimeout() { if (!mShutdown) Close(); } bool PluginModuleParent::ShouldContinueFromReplyTimeout() { - // FIXME/bug 544095: pop up a dialog asking the user what to do - bool waitMoar = false; + nsCOMPtr pluginDump; + nsCOMPtr browserDump; + if (CrashReporter::CreatePairedMinidumps(OtherProcess(), + &mHangID, + getter_AddRefs(pluginDump), + getter_AddRefs(browserDump)) && + CrashReporter::GetIDFromMinidump(pluginDump, mPluginDumpID) && + CrashReporter::GetIDFromMinidump(browserDump, mBrowserDumpID)) { - if (!waitMoar) { - // We can't depend on the IO thread notifying us of a channel - // error, because there's an inherent race between killing the - // subprocess and shutting down the socket. It would be nice - // to call Close() here and do all the IPDL cleanup - // immediately, but we might have arbitrary junk below us on - // the stack. So, a compromise: enqueue an event now that - // will Close(), *before* killing the child process. This - // guarantees that the Close() event will be processed before - // the IO error event, if it's delivered. - MessageLoop::current()->PostTask( - FROM_HERE, - NewRunnableMethod(this, &PluginModuleParent::CleanupFromTimeout)); + PLUGIN_LOG_DEBUG( + ("generated paired browser/plugin minidumps: %s/%s (ID=%s)", + NS_ConvertUTF16toUTF8(mBrowserDumpID).get(), + NS_ConvertUTF16toUTF8(mPluginDumpID).get(), + NS_ConvertUTF16toUTF8(mHangID).get())); + } + else { + NS_WARNING("failed to capture paired minidumps from hang"); + } - // FIXME/bug 544095: kill the subprocess in a way that - // triggers breakpad, and also capture a minidump for this - // process - KillProcess(ChildProcessHandle(), 1, false); - } - + // this must run before the error notification from the channel, + // or not at all + MessageLoop::current()->PostTask( + FROM_HERE, + mTaskFactory.NewRunnableMethod( + &PluginModuleParent::CleanupFromTimeout)); - return waitMoar; + if (!KillProcess(OtherProcess(), 1, false)) + NS_WARNING("failed to kill subprocess!"); + + return false; } void PluginModuleParent::ActorDestroy(ActorDestroyReason why) { switch (why) { case AbnormalShutdown: { - nsCOMPtr dump; + nsCOMPtr pluginDump; if (CrashReporter::TakeMinidumpForChild(ChildProcessHandle(), - getter_AddRefs(dump))) { - WriteExtraDataForMinidump(dump); - if (NS_SUCCEEDED(dump->GetLeafName(mDumpID))) { - mDumpID.Replace(mDumpID.Length() - 4, 4, - NS_LITERAL_STRING("")); - } + getter_AddRefs(pluginDump)) && + CrashReporter::GetIDFromMinidump(pluginDump, mPluginDumpID)) { + PLUGIN_LOG_DEBUG(("got child minidump: %s", + NS_ConvertUTF16toUTF8(mPluginDumpID).get())); + WritePluginExtraDataForMinidump(mPluginDumpID); + } + else if (!mPluginDumpID.IsEmpty() && !mBrowserDumpID.IsEmpty()) { + WriteExtraDataForHang(); } else { NS_WARNING("[PluginModuleParent::ActorDestroy] abnormal shutdown without minidump!"); } mShutdown = true; // Defer the PluginCrashed method so that we don't re-enter // and potentially modify the actor child list while enumerating it. - if (mPlugin) { - mPluginCrashedTask = NewRunnableMethod( - this, &PluginModuleParent::NotifyPluginCrashed); - MessageLoop::current()->PostTask(FROM_HERE, mPluginCrashedTask); - } + if (mPlugin) + MessageLoop::current()->PostTask( + FROM_HERE, + mTaskFactory.NewRunnableMethod( + &PluginModuleParent::NotifyPluginCrashed)); break; } case NormalShutdown: mShutdown = true; break; default: NS_ERROR("Unexpected shutdown reason for toplevel actor."); } } void PluginModuleParent::NotifyPluginCrashed() { - // MessageLoop owns this - mPluginCrashedTask = NULL; - + // FIXME/bug 544936: propagate mBrowserDumpID out to nsNPAPIPlugin + // and beyond if (mPlugin) - mPlugin->PluginCrashed(mDumpID); + mPlugin->PluginCrashed(mPluginDumpID); } PPluginInstanceParent* PluginModuleParent::AllocPPluginInstance(const nsCString& aMimeType, const uint16_t& aMode, const nsTArray& aNames, const nsTArray& aValues, NPError* rv) diff --git a/dom/plugins/PluginModuleParent.h b/dom/plugins/PluginModuleParent.h --- a/dom/plugins/PluginModuleParent.h +++ b/dom/plugins/PluginModuleParent.h @@ -228,31 +228,31 @@ private: #if defined(XP_WIN) || defined(XP_MACOSX) || defined(XP_OS2) virtual nsresult NP_GetEntryPoints(NPPluginFuncs* pFuncs, NPError* error); #endif virtual nsresult NPP_New(NPMIMEType pluginType, NPP instance, uint16_t mode, int16_t argc, char* argn[], char* argv[], NPSavedData* saved, NPError* error); private: - void WriteExtraDataForMinidump(nsIFile* dumpFile); - void WriteExtraDataEntry(nsIFileOutputStream* stream, - const char* key, - const char* value); + void WritePluginExtraDataForMinidump(const nsAString& id); + void WriteExtraDataForHang(); void CleanupFromTimeout(); static int TimeoutChanged(const char* aPref, void* aModule); void NotifyPluginCrashed(); nsCString mCrashNotes; PluginProcessParent* mSubprocess; bool mShutdown; const NPNetscapeFuncs* mNPNIface; nsTHashtable mValidIdentifiers; nsNPAPIPlugin* mPlugin; time_t mProcessStartTime; - CancelableTask* mPluginCrashedTask; - nsString mDumpID; + ScopedRunnableMethodFactory mTaskFactory; + nsString mPluginDumpID; + nsString mBrowserDumpID; + nsString mHangID; }; } // namespace plugins } // namespace mozilla #endif // ifndef dom_plugins_PluginModuleParent_h