Bug 522332 - Make nsProfileLock::FatalSignalHandler async-signal-safe…

…. r=bsmedberg a=blocking2.0
EtlerEdelgard · Jul 16, 2010 · a458ee3 · a458ee3
1 parent 78f7ba2
commit a458ee3
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 8 deletions.
diff --git a/profile/dirserviceprovider/src/nsProfileLock.cpp b/profile/dirserviceprovider/src/nsProfileLock.cpp
@@ -142,12 +142,12 @@ static int setupPidLockCleanup;
 PRCList nsProfileLock::mPidLockList =
     PR_INIT_STATIC_CLIST(&nsProfileLock::mPidLockList);
 
-void nsProfileLock::RemovePidLockFiles()
+void nsProfileLock::RemovePidLockFiles(PRBool aFatalSignal)
 {
     while (!PR_CLIST_IS_EMPTY(&mPidLockList))
     {
         nsProfileLock *lock = static_cast<nsProfileLock*>(mPidLockList.next);
-        lock->Unlock();
+        lock->Unlock(aFatalSignal);
     }
 }
 
@@ -163,7 +163,7 @@ void nsProfileLock::FatalSignalHandler(int signo, siginfo_t *info,
                                        void *context)
 {
     // Remove any locks still held.
-    RemovePidLockFiles();
+    RemovePidLockFiles(PR_TRUE);
 
     // Chain to the old handler, which may exit.
     struct sigaction *oldact = nsnull;
@@ -385,7 +385,7 @@ nsresult nsProfileLock::LockWithSymlink(const nsACString& lockFilePath, PRBool a
             if (!setupPidLockCleanup++)
             {
                 // Clean up on normal termination.
-                atexit(RemovePidLockFiles);
+                atexit(RemovePidLockFilesExiting);
 
                 // Clean up on abnormal termination, using POSIX sigaction.
                 // Don't arm a handler if the signal is being ignored, e.g.,
@@ -652,7 +652,7 @@ nsresult nsProfileLock::Lock(nsILocalFile* aProfileDir,
 }
 
 
-nsresult nsProfileLock::Unlock()
+nsresult nsProfileLock::Unlock(PRBool aFatalSignal)
 {
     nsresult rv = NS_OK;
 
@@ -675,7 +675,14 @@ nsresult nsProfileLock::Unlock()
         {
             PR_REMOVE_LINK(this);
             (void) unlink(mPidLockFileName);
-            free(mPidLockFileName);
+
+            // Only free mPidLockFileName if we're not in the fatal signal
+            // handler.  The problem is that a call to free() might be the
+            // cause of this fatal signal.  If so, calling free() might cause
+            // us to wait on the malloc implementation's lock.  We're already
+            // holding this lock, so we'll deadlock. See bug 522332.
+            if (!aFatalSignal)
+                free(mPidLockFileName);
             mPidLockFileName = nsnull;
         }
         else if (mLockFileDesc != -1)

diff --git a/profile/dirserviceprovider/src/nsProfileLock.h b/profile/dirserviceprovider/src/nsProfileLock.h
@@ -82,7 +82,13 @@ class nsProfileLock
      * @throws NS_ERROR_FILE_ACCESS_DENIED if the profile is locked.
      */
     nsresult                Lock(nsILocalFile* aProfileDir, nsIProfileUnlocker* *aUnlocker);
-    nsresult                Unlock();
+
+    /**
+     * Unlock a profile directory.  If you're unlocking the directory because
+     * the application is in the process of shutting down because of a fatal
+     * signal, set aFatalSignal to PR_TRUE.
+     */
+    nsresult                Unlock(PRBool aFatalSignal = PR_FALSE);
 
 private:
     PRPackedBool            mHaveLock;
@@ -92,7 +98,17 @@ class nsProfileLock
 #elif defined (XP_OS2)
     LHANDLE                 mLockFileHandle;
 #elif defined (XP_UNIX)
-    static void             RemovePidLockFiles();
+
+    static void             RemovePidLockFilesExiting()
+    {
+      // We can't implement this function with a default parameter on
+      // RemovePidLockFiles(aFatalSignal) since we register
+      //    atexit(RemovePidLockFilesExiting).
+
+      RemovePidLockFiles(PR_FALSE);
+    }
+
+    static void             RemovePidLockFiles(PRBool aFatalSignal);
     static void             FatalSignalHandler(int signo, siginfo_t *info,
                                                void *context);
     static PRCList          mPidLockList;