diff options
author | thakis@chromium.org <thakis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-08-15 19:24:29 +0000 |
---|---|---|
committer | thakis@chromium.org <thakis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-08-15 19:24:29 +0000 |
commit | 237529729bf9b6040a8dbde022e4b9c1f557cab2 (patch) | |
tree | 7bff6dc4848c34a57bc1da0ce30cf6a58b539848 /rlz/lib | |
parent | 9c621c92cb46f0d9bd56e0bca9102a8a46994fd8 (diff) | |
download | chromium_src-237529729bf9b6040a8dbde022e4b9c1f557cab2.zip chromium_src-237529729bf9b6040a8dbde022e4b9c1f557cab2.tar.gz chromium_src-237529729bf9b6040a8dbde022e4b9c1f557cab2.tar.bz2 |
rlz/mac: Make sure a crashing process doesn't leave a stale lockfile behind.
Don't use NSDistributedLock, which uses a lock that isn't cleaned up on
unexpected program termination, and which strongly recommends to not
call -breakLock (which makes this class fairly pointless).
Instead, use a flock(), which is cleaned up by the OS on process exit.
Suggested by shess@.
BUG=141108
Review URL: https://chromiumcodereview.appspot.com/10823329
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@151723 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'rlz/lib')
-rw-r--r-- | rlz/lib/rlz_lib_test.cc | 52 |
1 files changed, 51 insertions, 1 deletions
diff --git a/rlz/lib/rlz_lib_test.cc b/rlz/lib/rlz_lib_test.cc index 0f8cb4c..118f48c 100644 --- a/rlz/lib/rlz_lib_test.cc +++ b/rlz/lib/rlz_lib_test.cc @@ -13,6 +13,7 @@ // The "GGLA" brand is used to test the normal code flow of the code, and the // "TEST" brand is used to test the supplementary brand code code flow. +#include "base/eintr_wrapper.h" #include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "testing/gmock/include/gmock/gmock.h" @@ -795,7 +796,9 @@ class ReadonlyRlzDirectoryTest : public RlzLibTestNoMachineState { void ReadonlyRlzDirectoryTest::SetUp() { RlzLibTestNoMachineState::SetUp(); // Make the rlz directory non-writeable. - chmod(temp_dir_.path().value().c_str(), 0500); + int chmod_result = chmod(temp_dir_.path().value().c_str(), 0500); + ASSERT_EQ(0, chmod_result); + } TEST_F(ReadonlyRlzDirectoryTest, WriteFails) { @@ -821,4 +824,51 @@ TEST_F(ReadonlyRlzDirectoryTest, SupplementaryBrandingDoesNotCrash) { EXPECT_FALSE(rlz_lib::RecordProductEvent(rlz_lib::TOOLBAR_NOTIFIER, rlz_lib::IE_DEFAULT_SEARCH, rlz_lib::INSTALL)); } + +// Regression test for http://crbug.com/141108 +TEST_F(RlzLibTest, ConcurrentStoreAccessWithProcessExitsWhileLockHeld) { + // See the comment at the top of WriteFails. + if (!rlz_lib::SupplementaryBranding::GetBrand().empty()) + return; + + std::vector<pid_t> pids; + for (int i = 0; i < 10; ++i) { + pid_t pid = fork(); + ASSERT_NE(-1, pid); + if (pid == 0) { + // Child. + { + // SupplementaryBranding is a RAII object for the rlz lock. + rlz_lib::SupplementaryBranding branding("TEST"); + + // Simulate a crash while holding the lock in some of the children. + if (i > 0 && i % 3 == 0) + _exit(0); + + // Note: Since this is in a forked child, a failing expectation won't + // make the test fail. It does however cause lots of "check failed" + // error output. The parent process will then check the exit code + // below to make the test fail. + bool success = rlz_lib::RecordProductEvent(rlz_lib::TOOLBAR_NOTIFIER, + rlz_lib::IE_DEFAULT_SEARCH, rlz_lib::INSTALL); + EXPECT_TRUE(success); + _exit(success ? 0 : 1); + } + _exit(0); + } else { + // Parent. + pids.push_back(pid); + } + } + + int status; + for (size_t i = 0; i < pids.size(); ++i) { + if (HANDLE_EINTR(waitpid(pids[i], &status, 0)) != -1) + EXPECT_TRUE(WIFEXITED(status) && WEXITSTATUS(status) == 0); + } + + // No child should have the lock at this point, not even the crashed ones. + EXPECT_TRUE(rlz_lib::RecordProductEvent(rlz_lib::TOOLBAR_NOTIFIER, + rlz_lib::IE_DEFAULT_SEARCH, rlz_lib::INSTALL)); +} #endif |