Bug 1347358 - Add a Cleanup() function for profile locks. r?glandium draft
authorGian-Carlo Pascutto <gcp@mozilla.com>
Thu, 23 Mar 2017 18:02:10 +0100
changeset 503870 5f415fdfdbdc383f1975a34deabc6f692973bd5d
parent 503869 c309f93f5c33eaaff2294ef70ab18158d3fbb25d
child 550523 0baf55fc307bfa7b28ca4470f76c8dc91317b71c
push id50677
push usergpascutto@mozilla.com
push dateThu, 23 Mar 2017 17:16:23 +0000
reviewersglandium
bugs1347358
milestone55.0a1
Bug 1347358 - Add a Cleanup() function for profile locks. r?glandium MozReview-Commit-ID: GYQeUuzWPOV
toolkit/profile/gtest/TestProfileLock.cpp
toolkit/profile/gtest/TestProfileLockRetry.cpp
toolkit/profile/gtest/moz.build
toolkit/profile/nsProfileLock.cpp
toolkit/profile/nsProfileLock.h
toolkit/xre/nsAppRunner.cpp
--- a/toolkit/profile/gtest/TestProfileLock.cpp
+++ b/toolkit/profile/gtest/TestProfileLock.cpp
@@ -1,116 +1,36 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "gtest/gtest.h"
 
-#include <sys/eventfd.h>
-#include <sched.h>
-
 #include "nsCOMPtr.h"
 #include "nsIFile.h"
 #include "nsProfileLock.h"
 #include "nsString.h"
-
-static void CleanParentLock(const char *tmpdir)
-{
-  // nsProfileLock doesn't clean up all its files
-  char permanent_lockfile[] = "/.parentlock";
-
-  char * parentlock_name;
-  size_t parentlock_name_size = strlen(tmpdir) + strlen(permanent_lockfile) + 1;
-  parentlock_name = (char*)malloc(parentlock_name_size);
-
-  strcpy(parentlock_name, tmpdir);
-  strcat(parentlock_name, permanent_lockfile);
-
-  EXPECT_EQ(0, unlink(parentlock_name));
-  EXPECT_EQ(0, rmdir(tmpdir));
-  free(parentlock_name);
-}
+#include "nsDirectoryServiceDefs.h"
 
 TEST(ProfileLock, BasicLock)
 {
-  char templ[] = "/tmp/profilelocktest.XXXXXX";
-  char * tmpdir = mkdtemp(templ);
-  ASSERT_NE(tmpdir, nullptr);
+  char tmpExt[] = "profilebasiclocktest";
 
-  // This scope exits so the nsProfileLock destructor
-  // can clean up the files it leaves in tmpdir.
-  {
-    nsProfileLock myLock;
-    nsresult rv;
-    nsCOMPtr<nsIFile> dir(do_CreateInstance(NS_LOCAL_FILE_CONTRACTID, &rv));
-    ASSERT_EQ(NS_SUCCEEDED(rv), true);
-
-    rv = dir->InitWithNativePath(nsCString(tmpdir));
-    ASSERT_EQ(NS_SUCCEEDED(rv), true);
-
-    rv = myLock.Lock(dir, nullptr);
-    EXPECT_EQ(NS_SUCCEEDED(rv), true);
-  }
+  nsProfileLock myLock;
+  nsresult rv;
 
-  CleanParentLock(tmpdir);
-}
-
-TEST(ProfileLock, RetryLock)
-{
-  char templ[] = "/tmp/profilelocktest.XXXXXX";
-  char * tmpdir = mkdtemp(templ);
-  ASSERT_NE(tmpdir, nullptr);
-
-  {
-    nsProfileLock myLock;
-    nsProfileLock myLock2;
-    nsresult rv;
-    nsCOMPtr<nsIFile> dir(do_CreateInstance(NS_LOCAL_FILE_CONTRACTID, &rv));
-    ASSERT_EQ(NS_SUCCEEDED(rv), true);
-
-    rv = dir->InitWithNativePath(nsCString(tmpdir));
-    ASSERT_EQ(NS_SUCCEEDED(rv), true);
-
-    int eventfd_fd = eventfd(0, 0);
-    ASSERT_GE(eventfd_fd, 0);
+  nsCOMPtr<nsIFile> tmpDir;
+  rv = NS_GetSpecialDirectory(NS_OS_TEMP_DIR,
+                              getter_AddRefs(tmpDir));
+  ASSERT_EQ(NS_SUCCEEDED(rv), true);
 
-    // fcntl advisory locks are per process, so it's hard
-    // to avoid using fork here.
-    pid_t childpid = fork();
-
-    if (childpid) {
-      // parent
+  rv = tmpDir->AppendNative(nsCString(tmpExt));
+  ASSERT_EQ(NS_SUCCEEDED(rv), true);
 
-      // blocking read causing us to lose the race
-      eventfd_t value;
-      EXPECT_EQ(0, eventfd_read(eventfd_fd, &value));
-
-      rv = myLock2.Lock(dir, nullptr);
-      EXPECT_EQ(NS_ERROR_FILE_ACCESS_DENIED, rv);
-
-      // kill the child
-      EXPECT_EQ(0, kill(childpid, SIGTERM));
-
-      // reap zombie (required to collect the lock)
-      int status;
-      EXPECT_EQ(childpid, waitpid(childpid, &status, 0));
+  rv = tmpDir->CreateUnique(nsIFile::DIRECTORY_TYPE, 0700);
+  ASSERT_EQ(NS_SUCCEEDED(rv), true);
 
-      rv = myLock2.Lock(dir, nullptr);
-      EXPECT_EQ(NS_SUCCEEDED(rv), true);
-    } else {
-      // child
-      rv = myLock.Lock(dir, nullptr);
-      ASSERT_EQ(NS_SUCCEEDED(rv), true);
-
-      // unblock parent
-      EXPECT_EQ(0, eventfd_write(eventfd_fd, 1));
+  rv = myLock.Lock(tmpDir, nullptr);
+  EXPECT_EQ(NS_SUCCEEDED(rv), true);
 
-      // parent will kill us
-      for (;;)
-        sleep(1);
-    }
-
-    close(eventfd_fd);
-  }
-
-  CleanParentLock(tmpdir);
+  myLock.Cleanup();
 }
new file mode 100644
--- /dev/null
+++ b/toolkit/profile/gtest/TestProfileLockRetry.cpp
@@ -0,0 +1,74 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "gtest/gtest.h"
+
+#include <sys/eventfd.h>
+#include <sched.h>
+
+#include "nsCOMPtr.h"
+#include "nsIFile.h"
+#include "nsProfileLock.h"
+#include "nsString.h"
+
+TEST(ProfileLock, RetryLock)
+{
+  char templ[] = "/tmp/profilelocktest.XXXXXX";
+  char * tmpdir = mkdtemp(templ);
+  ASSERT_NE(tmpdir, nullptr);
+
+  nsProfileLock myLock;
+  nsProfileLock myLock2;
+  nsresult rv;
+  nsCOMPtr<nsIFile> dir(do_CreateInstance(NS_LOCAL_FILE_CONTRACTID, &rv));
+  ASSERT_EQ(NS_SUCCEEDED(rv), true);
+
+  rv = dir->InitWithNativePath(nsCString(tmpdir));
+  ASSERT_EQ(NS_SUCCEEDED(rv), true);
+
+  int eventfd_fd = eventfd(0, 0);
+  ASSERT_GE(eventfd_fd, 0);
+
+  // fcntl advisory locks are per process, so it's hard
+  // to avoid using fork here.
+  pid_t childpid = fork();
+
+  if (childpid) {
+    // parent
+
+    // blocking read causing us to lose the race
+    eventfd_t value;
+    EXPECT_EQ(0, eventfd_read(eventfd_fd, &value));
+
+    rv = myLock2.Lock(dir, nullptr);
+    EXPECT_EQ(NS_ERROR_FILE_ACCESS_DENIED, rv);
+
+    // kill the child
+    EXPECT_EQ(0, kill(childpid, SIGTERM));
+
+    // reap zombie (required to collect the lock)
+    int status;
+    EXPECT_EQ(childpid, waitpid(childpid, &status, 0));
+
+    rv = myLock2.Lock(dir, nullptr);
+    EXPECT_EQ(NS_SUCCEEDED(rv), true);
+  } else {
+    // child
+    rv = myLock.Lock(dir, nullptr);
+    ASSERT_EQ(NS_SUCCEEDED(rv), true);
+
+    // unblock parent
+    EXPECT_EQ(0, eventfd_write(eventfd_fd, 1));
+
+    // parent will kill us
+    for (;;)
+      sleep(1);
+  }
+
+  close(eventfd_fd);
+
+  myLock.Cleanup();
+  myLock2.Cleanup();
+}
--- a/toolkit/profile/gtest/moz.build
+++ b/toolkit/profile/gtest/moz.build
@@ -3,14 +3,18 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 LOCAL_INCLUDES += [
     '..',
 ]
 
+UNIFIED_SOURCES += [
+    'TestProfileLock.cpp',
+]
+
 if CONFIG['OS_ARCH'] == 'Linux':
     UNIFIED_SOURCES += [
-        'TestProfileLock.cpp',
+        'TestProfileLockRetry.cpp',
     ]
 
 FINAL_LIBRARY = 'xul-gtest'
--- a/toolkit/profile/nsProfileLock.cpp
+++ b/toolkit/profile/nsProfileLock.cpp
@@ -478,16 +478,21 @@ nsresult nsProfileLock::Lock(nsIFile* aP
     rv = aProfileDir->Clone(getter_AddRefs(lockFile));
     if (NS_FAILED(rv))
         return rv;
 
     rv = lockFile->Append(LOCKFILE_NAME);
     if (NS_FAILED(rv))
         return rv;
 
+    // Remember the name we're using so we can clean up
+    rv = lockFile->Clone(getter_AddRefs(mLockFile));
+    if (NS_FAILED(rv))
+        return rv;
+
 #if defined(XP_MACOSX)
     // First, try locking using fcntl. It is more reliable on
     // a local machine, but may not be supported by an NFS server.
 
     rv = LockWithFcntl(lockFile);
     if (NS_FAILED(rv) && (rv != NS_ERROR_FILE_ACCESS_DENIED))
     {
         // If that failed for any reason other than NS_ERROR_FILE_ACCESS_DENIED,
@@ -654,8 +659,17 @@ nsresult nsProfileLock::Unlock(bool aFat
         }
 #endif
 
         mHaveLock = false;
     }
 
     return rv;
 }
+
+nsresult nsProfileLock::Cleanup()
+{
+    if (mLockFile) {
+        return mLockFile->Remove(false);
+    }
+
+    return NS_OK;
+}
--- a/toolkit/profile/nsProfileLock.h
+++ b/toolkit/profile/nsProfileLock.h
@@ -46,23 +46,29 @@ public:
     /**
      * 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 true.
      */
     nsresult                Unlock(bool aFatalSignal = false);
 
     /**
+     * Clean up any left over files in the directory.
+     */
+    nsresult                Cleanup();
+
+    /**
      * Get the modification time of a replaced profile lock, otherwise 0.
      */
     nsresult                GetReplacedLockTime(PRTime* aResult);
 
 private:
     bool                    mHaveLock;
     PRTime                  mReplacedLockTime;
+    nsCOMPtr<nsIFile>       mLockFile;
 
 #if defined (XP_WIN)
     HANDLE                  mLockFileHandle;
 #elif defined (XP_UNIX)
 
     struct RemovePidLockFilesExiting {
         RemovePidLockFilesExiting() {}
         ~RemovePidLockFilesExiting() {
--- a/toolkit/xre/nsAppRunner.cpp
+++ b/toolkit/xre/nsAppRunner.cpp
@@ -4458,16 +4458,17 @@ XREMain::XRE_mainRun()
     // if we have X remote support, start listening for requests on the
     // proxy window.
     if (!mDisableRemote)
       mRemoteService = do_GetService("@mozilla.org/toolkit/remote-service;1");
     if (mRemoteService)
       mRemoteService->Startup(mAppData->remotingName, mProfileName.get());
     if (mRemoteLockDir) {
       mRemoteLock.Unlock();
+      mRemoteLock.Cleanup();
       mRemoteLockDir->Remove(false);
     }
 #endif /* MOZ_ENABLE_XREMOTE */
 
     mNativeApp->Enable();
   }
 
 #ifdef MOZ_INSTRUMENT_EVENT_LOOP