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).
 //
 // These tests require write access to HKLM and HKCU.
 //
 // 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"
 #include "testing/gtest/include/gtest/gtest.h"
 
 #include "rlz/lib/rlz_lib.h"
 #include "rlz/test/rlz_test_helpers.h"
 
 #if defined(OS_WIN)
 #include <Windows.h>
@@ skipping 762 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 (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));
+}

[Roger Tawa OOO till Jul 10th, 2012/08/15 15:10:23]

Should this test be wrapped in a #if defined(OS_MACOSX) or something?

[Mark Mentovai, 2012/08/15 16:10:00]

Roger Tawa wrote:
It already is.

 #endif