Bug 1818714 - Remove cachePurge lock file after running BackgroundTask_removeDirectory r=necko-reviewers,saschanaz,jesup

Differential Revision: https://phabricator.services.mozilla.com/D176548

netwerk/cache2/CachePurgeLock.cpp

@@ -7,18 +7,16 @@
 #include "nsIFile.h"
 #include "nsAppRunner.h"
 #include "mozilla/MultiInstanceLock.h"
+#include "nsLocalFile.h"
 
 namespace mozilla::net {
 
 NS_IMPL_ISUPPORTS(CachePurgeLock, nsICachePurgeLock)
 
-NS_IMETHODIMP
-CachePurgeLock::Lock(const nsACString& profileName) {
+static nsresult PrepareLockArguments(const nsACString& profileName,
+                                     nsCString& lockName,
+                                     nsString& appDirPath) {
   nsresult rv;
-  if (mLock != MULTI_INSTANCE_LOCK_HANDLE_ERROR) {
-    // Lock is already open.
-    return NS_OK;
-  }
 
   nsCOMPtr<nsIFile> appFile = mozilla::GetNormalizedAppFile(nullptr);
   if (!appFile) {
@@ -29,13 +27,54 @@ CachePurgeLock::Lock(const nsACString& profileName) {
   rv = appFile->GetParent(getter_AddRefs(appDirFile));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  nsAutoString appDirPath;
   rv = appDirFile->GetPath(appDirPath);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  nsAutoCString lockName(profileName);
+  lockName = profileName;
   lockName.Append("-cachePurge");
 
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+CachePurgeLock::GetLockFile(const nsACString& profileName, nsIFile** aResult) {
+  nsresult rv;
+  nsCString lockName;
+  nsString appDirPath;
+  rv = PrepareLockArguments(profileName, lockName, appDirPath);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
+  nsCString filePath;
+  if (!GetMultiInstanceLockFileName(lockName.get(), appDirPath.get(),
+                                    filePath)) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+
+  nsCOMPtr<nsIFile> lockFile = new nsLocalFile();
+  rv = lockFile->InitWithNativePath(filePath);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  lockFile.forget(aResult);
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+CachePurgeLock::Lock(const nsACString& profileName) {
+  nsresult rv;
+  if (mLock != MULTI_INSTANCE_LOCK_HANDLE_ERROR) {
+    // Lock is already open.
+    return NS_OK;
+  }
+
+  nsCString lockName;
+  nsString appDirPath;
+  rv = PrepareLockArguments(profileName, lockName, appDirPath);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
   mLock = mozilla::OpenMultiInstanceLock(lockName.get(), appDirPath.get());
   if (mLock == MULTI_INSTANCE_LOCK_HANDLE_ERROR) {
     return NS_ERROR_FAILURE;

netwerk/cache2/nsICachePurgeLock.idl

@@ -3,6 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsISupports.idl"
+interface nsIFile;
 
 /**
  * This object is a wrapper of MultiInstanceLock.
@@ -30,4 +31,10 @@ interface nsICachePurgeLock : nsISupports {
      * after unlocking.
      */
     void unlock();
+
+    /**
+     * Returns the file used to guarantee single access to a resource.
+     * This method is used to remove the lock file when no longer necessary.
+     */
+    nsIFile getLockFile(in AUTF8String profileName);
 };

netwerk/test/marionette/test_purge_http_cache_at_shutdown.py

@@ -2,6 +2,7 @@
 # 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/.
 
+import os
 from pathlib import Path
 
 from marionette_driver import Wait
@@ -34,8 +35,28 @@ class PurgeHTTPCacheAtShutdownTestCase(MarionetteTestCase):
             child.name.endswith(".purge.bg_rm") for child in self.profile_path.iterdir()
         )
 
-    def test_ensure_cache_purge_after_in_app_quit(self):
-        self.assertTrue(self.cacheDirExists(), "Cache directory must exist")
+    def initLockDir(self):
+        self.lock_dir = None
+        with self.marionette.using_context("chrome"):
+            path = self.marionette.execute_script(
+                """
+              return Services.dirsvc.get("UpdRootD", Ci.nsIFile).parent.parent.path;
+            """
+            )
+            self.lock_dir = Path(path)
+
+    def assertNoLocks(self):
+        locks = [
+            x
+            for x in self.lock_dir.iterdir()
+            if x.is_file() and "-cachePurge" in x.name
+        ]
+        self.assertEqual(locks, [], "All locks should have been removed")
+
+    # Tests are run in lexicographical order, so test_a* is run first to
+    # cleanup locks that may be there from previous runs.
+    def test_asetup(self):
+        self.initLockDir()
 
         self.marionette.quit()
 
@@ -44,8 +65,33 @@ class PurgeHTTPCacheAtShutdownTestCase(MarionetteTestCase):
             message="Cache directory must be removed after orderly shutdown",
         )
 
+        # delete locks from previous runs
+        locks = [
+            x
+            for x in self.lock_dir.iterdir()
+            if x.is_file() and "-cachePurge" in x.name
+        ]
+        for lock in locks:
+            os.remove(lock)
+        # all locks should have been removed successfully.
+        self.assertNoLocks()
+
+    def test_ensure_cache_purge_after_in_app_quit(self):
+        self.assertTrue(self.cacheDirExists(), "Cache directory must exist")
+        self.initLockDir()
+
+        self.marionette.quit()
+
+        Wait(self.marionette, timeout=60).until(
+            lambda _: not self.cacheDirExists() and not self.renamedDirExists(),
+            message="Cache directory must be removed after orderly shutdown",
+        )
+
+        self.assertNoLocks()
+
     def test_longstanding_cache_purge_after_in_app_quit(self):
         self.assertTrue(self.cacheDirExists(), "Cache directory must exist")
+        self.initLockDir()
 
         self.marionette.set_pref(
             "toolkit.background_tasks.remove_directory.testing.sleep_ms", 5000
@@ -58,12 +104,15 @@ class PurgeHTTPCacheAtShutdownTestCase(MarionetteTestCase):
             message="Cache directory must be removed after orderly shutdown",
         )
 
+        self.assertNoLocks()
+
     def test_ensure_cache_purge_after_forced_restart(self):
         """
         Doing forced restart here to prevent the shutdown phase purging and only allow startup
         phase one, via `CacheFileIOManager::OnDelayedStartupFinished`.
         """
         self.profile_path.joinpath("foo.purge.bg_rm").mkdir()
+        self.initLockDir()
 
         self.marionette.restart(in_app=False)
 
@@ -72,3 +121,5 @@ class PurgeHTTPCacheAtShutdownTestCase(MarionetteTestCase):
             message="Directories with .purge.bg_rm postfix must be removed at startup after"
             "disorderly shutdown",
         )
+
+        self.assertNoLocks()

toolkit/components/backgroundtasks/BackgroundTask_removeDirectory.sys.mjs

@@ -88,6 +88,14 @@ function tryRemoveDir(aFile, countObj) {
 
 const FILE_CHECK_ITERATION_TIMEOUT_MS = 1000;
 
+function cleanupDirLockFile(aLock, aProfileName) {
+  let lockFile = aLock.getLockFile(aProfileName);
+  try {
+    // Try to clean up the lock file
+    lockFile.remove(false);
+  } catch (ex) {}
+}
+
 async function deleteChildDirectory(
   parentDirPath,
   childDirName,
@@ -178,6 +186,7 @@ async function deleteChildDirectory(
     if (locked) {
       dirLock.unlock();
       locked = false;
+      cleanupDirLockFile(dirLock, childDirName);
     }
   }
 }
@@ -243,6 +252,7 @@ async function cleanupOtherDirectories(
       `Deletion of folder ${entry.leafName} - success=${removedDir}`
     );
     dirLock.unlock();
+    cleanupDirLockFile(dirLock, entry.leafName);
   }
 }
 

toolkit/xre/MultiInstanceLock.cpp

@@ -31,8 +31,9 @@
 
 namespace mozilla {
 
-static bool GetLockFileName(const char* nameToken, const char16_t* installPath,
-                            nsCString& filePath) {
+bool GetMultiInstanceLockFileName(const char* nameToken,
+                                  const char16_t* installPath,
+                                  nsCString& filePath) {
 #ifdef XP_WIN
   // On Windows, the lock file is placed at the path
   // [updateDirectory]\[nameToken]-[pathHash], so first we need to get the
@@ -108,7 +109,7 @@ static bool GetLockFileName(const char* nameToken, const char16_t* installPath,
 MultiInstLockHandle OpenMultiInstanceLock(const char* nameToken,
                                           const char16_t* installPath) {
   nsCString filePath;
-  if (!GetLockFileName(nameToken, installPath, filePath)) {
+  if (!GetMultiInstanceLockFileName(nameToken, installPath, filePath)) {
     return MULTI_INSTANCE_LOCK_HANDLE_ERROR;
   }
 

toolkit/xre/MultiInstanceLock.h

@@ -88,6 +88,12 @@ bool IsOtherInstanceRunning(MultiInstLockHandle lock, bool* aResult);
 // this function ensures the file path is properly normalized.
 already_AddRefed<nsIFile> GetNormalizedAppFile(nsIFile* aAppFile);
 
+// Computes the file path of multi instance lock
+// Returns true when successful - false otherwise
+bool GetMultiInstanceLockFileName(const char* nameToken,
+                                  const char16_t* installPath,
+                                  nsCString& filePath);
+
 };  // namespace mozilla
 
 #endif  // MULTIINSTANCELOCK_H