Increase the KillProcess timeout when running under Valgrind

base/process_util_posix.cc

 // Copyright (c) 2011 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 <dirent.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <signal.h>
 #include <stdlib.h>
 #include <sys/resource.h>
 #include <sys/time.h>
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <unistd.h>
 
 #include <limits>
 #include <set>
 
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/debug/stack_trace.h"
 #include "base/dir_reader_posix.h"
 #include "base/eintr_wrapper.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/process_util.h"
 #include "base/stringprintf.h"
 #include "base/synchronization/waitable_event.h"
+#include "base/third_party/dynamic_annotations/dynamic_annotations.h"

[darin (slow to review), 2011/06/17 20:54:42]

nit: it seems a bit unintuitive that RunningOnValgrind() would be buried in this
header file.  it might help if dynamic_annotations.h declared a namespace :)

[Timur Iskhodzhanov, 2011/06/17 21:02:47]

That's a good point.
However, currently dynamic_annotations are C headers (not C++) and maybe having
a namespace is an overkill (it mentions Valgrind after all)

On 2011/06/17 20:54:42, darin wrote:

 #include "base/threading/platform_thread.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time.h"
 
 #if defined(OS_MACOSX)
 #include <crt_externs.h>
 #include <sys/event.h>
 #define environ (*_NSGetEnviron())
 #else
 extern char** environ;
@@ skipping 176 matching lines @@
   DCHECK_GT(process_id, 1) << " tried to kill invalid process_id";
   if (process_id <= 1)
     return false;
   static unsigned kMaxSleepMs = 1000;
   unsigned sleep_ms = 4;
 
   bool result = kill(process_id, SIGTERM) == 0;
 
   if (result && wait) {
     int tries = 60;
+
+    if (RunningOnValgrind()) {

[darin (slow to review), 2011/06/17 20:54:42]

OK...

In my experience 2x might not even be enough :-)

[Timur Iskhodzhanov, 2011/06/17 21:02:47]

Totally agree - average Valgrind slowdown is ~20x.

But the idea right now is to increase the timout by just a bit and see how it
changes things.

I expect more leak reports to appear = good (because earlier the children
Valgrind processes were sometimes killed in the middle of the leak check)
but some tests may become slower or even flaky = bad.

That's why I don't want to increase the timeout by 20x immediately :)
After all, maybe 2x or 5x would be enough for 99% of tests.

+      // Wait for some extra time when running under Valgrind since the child
+      // processes may take some time doing leak checking.
+      tries *= 2;
+    }
+
     // The process may not end immediately due to pending I/O
     bool exited = false;
     while (tries-- > 0) {
       pid_t pid = HANDLE_EINTR(waitpid(process_id, NULL, WNOHANG));
       if (pid == process_id) {
         exited = true;
         break;
       }
       if (pid == -1) {
         if (errno == ECHILD) {
@@ skipping 757 matching lines @@
                       const ProcessFilter* filter) {
   bool exited_cleanly =
       WaitForProcessesToExit(executable_name, wait_milliseconds,
                              filter);
   if (!exited_cleanly)
     KillProcesses(executable_name, exit_code, filter);
   return exited_cleanly;
 }
 
 }  // namespace base