Non-SFI NaCl: Stop running sandbox tests in components_unittests

sandbox/linux/tests/unit_tests.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.
 
 #include <fcntl.h>
 #include <poll.h>
 #include <signal.h>
 #include <stdio.h>
 #include <sys/resource.h>
 #include <sys/time.h>
@@ skipping 24 matching lines @@
   if (task_d != 0 || task_stat.st_nlink < 3)
     return -1;
   const int num_threads = task_stat.st_nlink - 2;
   return num_threads;
 }
 
 }  // namespace
 
 namespace sandbox {
 
-extern bool kAllowForkWithThreads;
-
 bool IsAndroid() {
 #if defined(OS_ANDROID)
   return true;
 #else
   return false;
 #endif
 }
 
 bool IsArchitectureArm() {
 #if defined(ARCH_CPU_ARM_FAMILY)
@@ skipping 59 matching lines @@
   const int kNumExpectedThreads = 1;
 #else
   // Under TSAN, there is a special helper thread. It should be completely
   // invisible to our testing, so we ignore it. It should be ok to fork()
   // with this thread. It's currently buggy, but it's the best we can do until
   // there is a way to delay the start of the thread
   // (https://code.google.com/p/thread-sanitizer/issues/detail?id=19).
   const int kNumExpectedThreads = 2;
 #endif
 
   // The kernel is at liberty to wake a thread id futex before updating /proc.

[hamaji, 2014/05/07 06:52:51]

I guess we want to keep this?

[jln (very slow on Chromium), 2014/05/08 00:36:37]

Ohh yeah absolutely. If a test starts and stops a thread, we need to wait until
it disappears.

   // If another test running in the same process has stopped a thread, it may
   // appear as still running in /proc.
   // We poll /proc, with an exponential back-off. At most, we'll sleep around
   // 2^iterations nanoseconds in nanosleep().
-  if (!kAllowForkWithThreads) {
-    for (unsigned int iteration = 0; iteration < 30; iteration++) {
-      struct timespec ts = {0, 1L << iteration /* nanoseconds */};
-      PCHECK(0 == HANDLE_EINTR(nanosleep(&ts, &ts)));
-      num_threads = CountThreads();
-      if (kNumExpectedThreads == num_threads)
-        break;
-    }
+  for (unsigned int iteration = 0; iteration < 30; iteration++) {
+    struct timespec ts = {0, 1L << iteration /* nanoseconds */};
+    PCHECK(0 == HANDLE_EINTR(nanosleep(&ts, &ts)));
+    num_threads = CountThreads();
+    if (kNumExpectedThreads == num_threads)
+      break;
   }
 
-  const std::string multiple_threads_error =
-      "Running sandbox tests with multiple threads "
-      "is not supported and will make the tests flaky.";
-  if (!kAllowForkWithThreads) {
-    ASSERT_EQ(kNumExpectedThreads, num_threads) << multiple_threads_error;
-  } else {
-    if (kNumExpectedThreads != num_threads)
-      LOG(ERROR) << multiple_threads_error;
-  }
-
+  ASSERT_EQ(kNumExpectedThreads, num_threads)
+      << "Running sandbox tests with multiple threads "
+      << "is not supported and will make the tests "
+      << "flaky.\n";

[Mark Seaborn, 2014/05/08 15:29:39]

Nit: Isn't '\n' unnecessary for ASSERT_EQ?  Also fits on previous line?

[jln (DO NOT USE THIS), 2014/05/08 16:30:02]

Ohh good catch!

[hamaji, 2014/05/10 08:06:24]

Done. After I reverted https://codereview.chromium.org/242383003, I didn't look
at the code carefully :(

   int fds[2];
   ASSERT_EQ(0, pipe(fds));
   // Check that our pipe is not on one of the standard file descriptor.
   SANDBOX_ASSERT(fds[0] > 2 && fds[1] > 2);
 
   pid_t pid;
   ASSERT_LE(0, (pid = fork()));
   if (!pid) {
     // In child process
     // Redirect stderr to our pipe. This way, we can capture all error
@@ skipping 130 matching lines @@
   fflush(stderr);
   _exit(kExitWithAssertionFailure);
 }
 
 void UnitTests::IgnoreThisTest() {
   fflush(stderr);
   _exit(kIgnoreThisTest);
 }
 
 }  // namespace