From 4f3a2496ae5cee4572c800dee04e6c20ea51a5fb Mon Sep 17 00:00:00 2001 From: Ted Mielczarek Date: Mon, 14 Dec 2009 06:44:27 -0500 Subject: [PATCH] bug 514188 - fix nsProfileLock to use SA_SIGINFO style signal handler, so it can chain to Breakpad's signal handler properly. r=bsmedberg --HG-- extra : rebase_source : 4684665d9c32a76ad7fffdf9e305b5b617fca58c --- .../dirserviceprovider/src/nsProfileLock.cpp | 11 +++++--- .../dirserviceprovider/src/nsProfileLock.h | 4 ++- toolkit/crashreporter/test/nsITestCrasher.idl | 10 +++++++ toolkit/crashreporter/test/nsTestCrasher.cpp | 8 ++++++ .../test_crashreporter_crash_profile_lock.js | 27 +++++++++++++++++++ 5 files changed, 56 insertions(+), 4 deletions(-) create mode 100644 toolkit/crashreporter/test/unit/test_crashreporter_crash_profile_lock.js diff --git a/profile/dirserviceprovider/src/nsProfileLock.cpp b/profile/dirserviceprovider/src/nsProfileLock.cpp index e29aee395b9..4eeb25656e9 100644 --- a/profile/dirserviceprovider/src/nsProfileLock.cpp +++ b/profile/dirserviceprovider/src/nsProfileLock.cpp @@ -159,7 +159,8 @@ static struct sigaction SIGABRT_oldact; static struct sigaction SIGSEGV_oldact; static struct sigaction SIGTERM_oldact; -void nsProfileLock::FatalSignalHandler(int signo) +void nsProfileLock::FatalSignalHandler(int signo, siginfo_t *info, + void *context) { // Remove any locks still held. RemovePidLockFiles(); @@ -211,6 +212,10 @@ void nsProfileLock::FatalSignalHandler(int signo) raise(signo); } + else if (oldact->sa_sigaction && + (oldact->sa_flags & SA_SIGINFO) == SA_SIGINFO) { + oldact->sa_sigaction(signo, info, context); + } else if (oldact->sa_handler && oldact->sa_handler != SIG_IGN) { oldact->sa_handler(signo); @@ -387,8 +392,8 @@ nsresult nsProfileLock::LockWithSymlink(const nsACString& lockFilePath, PRBool a // because mozilla is run via nohup. if (!sDisableSignalHandling) { struct sigaction act, oldact; - act.sa_handler = FatalSignalHandler; - act.sa_flags = 0; + act.sa_sigaction = FatalSignalHandler; + act.sa_flags = SA_SIGINFO; sigfillset(&act.sa_mask); #define CATCH_SIGNAL(signame) \ diff --git a/profile/dirserviceprovider/src/nsProfileLock.h b/profile/dirserviceprovider/src/nsProfileLock.h index ec9cd967433..78471d1aaef 100644 --- a/profile/dirserviceprovider/src/nsProfileLock.h +++ b/profile/dirserviceprovider/src/nsProfileLock.h @@ -55,6 +55,7 @@ class nsIProfileUnlocker; #endif #if defined (XP_UNIX) +#include #include "prclist.h" #endif @@ -92,7 +93,8 @@ private: LHANDLE mLockFileHandle; #elif defined (XP_UNIX) static void RemovePidLockFiles(); - static void FatalSignalHandler(int signo); + static void FatalSignalHandler(int signo, siginfo_t *info, + void *context); static PRCList mPidLockList; nsresult LockWithFcntl(const nsACString& lockFilePath); diff --git a/toolkit/crashreporter/test/nsITestCrasher.idl b/toolkit/crashreporter/test/nsITestCrasher.idl index 5f93507d9e3..77ece9787eb 100644 --- a/toolkit/crashreporter/test/nsITestCrasher.idl +++ b/toolkit/crashreporter/test/nsITestCrasher.idl @@ -1,7 +1,17 @@ #include "nsISupports.idl" +interface nsILocalFile; + [scriptable, uuid(95464a04-6949-46cb-b621-d167790704a0)] interface nsITestCrasher : nsISupports { void crash(); + + /** + * Lock a directory using XRE_LockProfileDirectory. + * + * @param directory The directory to lock + * @return An opaque lock object. + */ + nsISupports lockDir(in nsILocalFile directory); }; diff --git a/toolkit/crashreporter/test/nsTestCrasher.cpp b/toolkit/crashreporter/test/nsTestCrasher.cpp index 7e17b7255d4..61f61965f7b 100644 --- a/toolkit/crashreporter/test/nsTestCrasher.cpp +++ b/toolkit/crashreporter/test/nsTestCrasher.cpp @@ -2,6 +2,7 @@ #include "nsIComponentManager.h" #include "nsIGenericFactory.h" #include "nsITestCrasher.h" +#include "nsXULAppAPI.h" class nsTestCrasher : public nsITestCrasher { @@ -27,6 +28,13 @@ NS_IMETHODIMP nsTestCrasher::Crash() return NS_OK; } +/* nsISupports LockDir (in nsILocalFile directory); */ +NS_IMETHODIMP nsTestCrasher::LockDir(nsILocalFile *directory, + nsISupports **_retval NS_OUTPARAM) +{ + return XRE_LockProfileDirectory(directory, _retval); +} + // 54afce51-38d7-4df0-9750-2f90f9ffbca2 #define NS_TESTCRASHER_CID \ { 0x54afce51, 0x38d7, 0x4df0, {0x97, 0x50, 0x2f, 0x90, 0xf9, 0xff, 0xbc, 0xa2} } diff --git a/toolkit/crashreporter/test/unit/test_crashreporter_crash_profile_lock.js b/toolkit/crashreporter/test/unit/test_crashreporter_crash_profile_lock.js new file mode 100644 index 00000000000..877c6b0485a --- /dev/null +++ b/toolkit/crashreporter/test/unit/test_crashreporter_crash_profile_lock.js @@ -0,0 +1,27 @@ +function run_test() +{ + if (!("@mozilla.org/toolkit/crash-reporter;1" in Components.classes)) { + dump("INFO | test_crashreporter.js | Can't test crashreporter in a non-libxul build.\n"); + return; + } + + // lock a profile directory, crash, and ensure that + // the profile lock signal handler doesn't interfere with + // writing a minidump + do_crash(function() { + let env = Components.classes["@mozilla.org/process/environment;1"] + .getService(Components.interfaces.nsIEnvironment); + // the python harness sets this in the environment for us + let profd = env.get("XPCSHELL_TEST_PROFILE_DIR"); + let dir = Components.classes["@mozilla.org/file/local;1"] + .createInstance(Components.interfaces.nsILocalFile); + dir.initWithPath(profd); + let mycrasher = Components.classes["@mozilla.org/testcrasher;1"].createInstance(Components.interfaces.nsITestCrasher); + let lock = mycrasher.lockDir(dir); + // when we crash, the lock file should be cleaned up + }, + function(mdump, extra) { + // if we got here, we have a minidump, so that's all we wanted + do_check_true(true); + }); +}