From 2285dea6a317bb4aad8ea13c21b22666e13885fb Mon Sep 17 00:00:00 2001 From: Gian-Carlo Pascutto Date: Thu, 23 Mar 2017 18:02:10 +0100 Subject: [PATCH] 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 | 114 +++--------------- .../profile/gtest/TestProfileLockRetry.cpp | 74 ++++++++++++ toolkit/profile/gtest/moz.build | 6 +- toolkit/profile/nsProfileLock.cpp | 14 +++ toolkit/profile/nsProfileLock.h | 6 + toolkit/xre/nsAppRunner.cpp | 1 + 6 files changed, 117 insertions(+), 98 deletions(-) create mode 100644 toolkit/profile/gtest/TestProfileLockRetry.cpp diff --git a/toolkit/profile/gtest/TestProfileLock.cpp b/toolkit/profile/gtest/TestProfileLock.cpp index ac5117d74f7c..f0477e2d2b20 100644 --- a/toolkit/profile/gtest/TestProfileLock.cpp +++ b/toolkit/profile/gtest/TestProfileLock.cpp @@ -5,112 +5,32 @@ #include "gtest/gtest.h" -#include -#include - #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 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 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 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(); } diff --git a/toolkit/profile/gtest/TestProfileLockRetry.cpp b/toolkit/profile/gtest/TestProfileLockRetry.cpp new file mode 100644 index 000000000000..dc06275e82c0 --- /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 +#include + +#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 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(); +} diff --git a/toolkit/profile/gtest/moz.build b/toolkit/profile/gtest/moz.build index 51b420e19d05..f84d5fa71375 100644 --- a/toolkit/profile/gtest/moz.build +++ b/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' diff --git a/toolkit/profile/nsProfileLock.cpp b/toolkit/profile/nsProfileLock.cpp index 0ce39202d29e..6971f21c9039 100644 --- a/toolkit/profile/nsProfileLock.cpp +++ b/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; +} diff --git a/toolkit/profile/nsProfileLock.h b/toolkit/profile/nsProfileLock.h index e78a3577e67c..b92798fce28d 100644 --- a/toolkit/profile/nsProfileLock.h +++ b/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 mLockFile; #if defined (XP_WIN) HANDLE mLockFileHandle; diff --git a/toolkit/xre/nsAppRunner.cpp b/toolkit/xre/nsAppRunner.cpp index b3a1357ef7fc..6180df5c3b2f 100644 --- a/toolkit/xre/nsAppRunner.cpp +++ b/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 */