Bug 1347358 - Add a Cleanup() function for profile locks. r=glandium

MozReview-Commit-ID: GYQeUuzWPOV

--HG--
extra : rebase_source : 1c031180cb7e31cbeff205de583bd37a3c13945c

toolkit/profile/gtest/TestProfileLock.cpp

@@ -5,112 +5,32 @@
 
 #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);
+  nsProfileLock myLock;
+  nsresult rv;
 
-    rv = dir->InitWithNativePath(nsCString(tmpdir));
-    ASSERT_EQ(NS_SUCCEEDED(rv), true);
+  nsCOMPtr<nsIFile> tmpDir;
+  rv = NS_GetSpecialDirectory(NS_OS_TEMP_DIR,
+                              getter_AddRefs(tmpDir));
+  ASSERT_EQ(NS_SUCCEEDED(rv), true);
 
-    rv = myLock.Lock(dir, nullptr);
-    EXPECT_EQ(NS_SUCCEEDED(rv), true);
-  }
+  rv = tmpDir->AppendNative(nsCString(tmpExt));
+  ASSERT_EQ(NS_SUCCEEDED(rv), true);
 
-  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);
-
-    // 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);
-  }
-
-  CleanParentLock(tmpdir);
+  rv = tmpDir->CreateUnique(nsIFile::DIRECTORY_TYPE, 0700);
+  ASSERT_EQ(NS_SUCCEEDED(rv), true);
+
+  rv = myLock.Lock(tmpDir, nullptr);
+  EXPECT_EQ(NS_SUCCEEDED(rv), true);
+
+  myLock.Cleanup();
 }

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();
+}

toolkit/profile/gtest/moz.build

@@ -8,9 +8,13 @@ LOCAL_INCLUDES += [
     '..',
 ]
 
+UNIFIED_SOURCES += [
+    'TestProfileLock.cpp',
+]
+
 if CONFIG['OS_ARCH'] == 'Linux':
     UNIFIED_SOURCES += [
-        'TestProfileLock.cpp',
+        'TestProfileLockRetry.cpp',
     ]
 
 FINAL_LIBRARY = 'xul-gtest'

toolkit/profile/nsProfileLock.cpp

@@ -483,6 +483,11 @@ nsresult nsProfileLock::Lock(nsIFile* aProfileDir,
     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.
@@ -659,3 +664,12 @@ nsresult nsProfileLock::Unlock(bool aFatalSignal)
 
     return rv;
 }
+
+nsresult nsProfileLock::Cleanup()
+{
+    if (mLockFile) {
+        return mLockFile->Remove(false);
+    }
+
+    return NS_OK;
+}

toolkit/profile/nsProfileLock.h

@@ -50,6 +50,11 @@ public:
      */
     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.
      */
@@ -58,6 +63,7 @@ public:
 private:
     bool                    mHaveLock;
     PRTime                  mReplacedLockTime;
+    nsCOMPtr<nsIFile>       mLockFile;
 
 #if defined (XP_WIN)
     HANDLE                  mLockFileHandle;

toolkit/xre/nsAppRunner.cpp

@@ -4463,6 +4463,7 @@ XREMain::XRE_mainRun()
       mRemoteService->Startup(mAppData->remotingName, mProfileName.get());
     if (mRemoteLockDir) {
       mRemoteLock.Unlock();
+      mRemoteLock.Cleanup();
       mRemoteLockDir->Remove(false);
     }
 #endif /* MOZ_ENABLE_XREMOTE */