rlz/mac: Make sure a crashing process doesn't leave a stale lockfile behind.

rlz/lib/rlz_lib_test.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
 // A test application for the RLZ library.
 //
 // These tests should not be executed on the build server:
 // - They assert for the failed cases.
 // - They modify machine state (registry).
 //
@@ skipping 777 matching lines @@
 
 #if defined(OS_MACOSX)
 class ReadonlyRlzDirectoryTest : public RlzLibTestNoMachineState {
  protected:
   virtual void SetUp() OVERRIDE;
 };
 
 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) {
   // The rlz test runner runs every test twice: Once normally, and once with
   // a SupplementaryBranding on the stack. In the latter case, the rlz lock
   // has already been acquired before the rlz directory got changed to
   // read-only, which makes this test pointless. So run it only in the first
   // pass.
   if (!rlz_lib::SupplementaryBranding::GetBrand().empty())
     return;
 
   EXPECT_FALSE(rlz_lib::RecordProductEvent(rlz_lib::TOOLBAR_NOTIFIER,
       rlz_lib::IE_DEFAULT_SEARCH, rlz_lib::SET_TO_GOOGLE));
 }
 
 // Regression test for http://crbug.com/121255
 TEST_F(ReadonlyRlzDirectoryTest, SupplementaryBrandingDoesNotCrash) {
   // See the comment at the top of WriteFails.
   if (!rlz_lib::SupplementaryBranding::GetBrand().empty())
     return;
 
   rlz_lib::SupplementaryBranding branding("TEST");
   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 (waitpid(pids[i], &status, 0) != -1)

[Mark Mentovai, 2012/08/15 13:36:28]

HANDLE_EINTR

+      EXPECT_EQ(0, WEXITSTATUS(status));

[Mark Mentovai, 2012/08/15 13:36:28]

WEXITSTATUS is only valid if WIFEXITED is true. You should check WIFEXITED in
addition to WEXITSTATUS. If WEXITED is false, the test should fail anyway since
you’re expecting it to die by _exit, not by signal.

+  }
+
+  // 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