summaryrefslogtreecommitdiffstats
path: root/rlz/lib
diff options
context:
space:
mode:
authorthakis@chromium.org <thakis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-08-15 19:24:29 +0000
committerthakis@chromium.org <thakis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-08-15 19:24:29 +0000
commit237529729bf9b6040a8dbde022e4b9c1f557cab2 (patch)
tree7bff6dc4848c34a57bc1da0ce30cf6a58b539848 /rlz/lib
parent9c621c92cb46f0d9bd56e0bca9102a8a46994fd8 (diff)
downloadchromium_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.cc52
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