Restricting lifetime of python sync server on Windows via a JobObject.

If a sync integration test runs for 30 seconds or more, OutOfProcTestRunner
forcibly kills the test case executable. This leaves an orphaned python
server instance in memory. When a subsequent test case is run, it
attempts to kick off a new python server and immediately checks for the
existence of a running server, following which it pumps messages to the
server instance. If the server detected by the test case happens to be
the old orphaned instance (that goes on to die when a new server
instance is started), test cases can fail.

This change list restricts the lifetime of the python.exe process started by
a test case such that if the test case is killed by OutOfProcTestRunner,
the python server instance it created dies with it.

In order to restrict the lifetime of a test server, it needs to be
started as a job. When a test server is spawned as a child process by a
test case that is running under a debugger, the child process needs to
be created using the CREATE_BREAKAWAY_FROM_JOB flag to first
disassociate it from the JobObject created by the debugger.

TestServerLauncher::Start() used to invoke base::LaunchApp() in order to
create the child process. This changelist implements a new method in
class TestServerLauncher called LaunchTestServerAsJob(). The call to
base::LaunchApp() in TestServerLauncher::Start() is replaced with a call
to TestServerLauncher::LaunchTestServerAsJob().

BUG=43777
TEST=sync_integration_tests

Review URL: http://codereview.chromium.org/2344001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@48762 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 81 insertions, 4 deletions

diff --git a/net/socket/ssl_test_util.cc b/net/socket/ssl_test_util.cc
index 81f5022..7ef1a83 100644
--- a/net/socket/ssl_test_util.cc
+++ b/net/socket/ssl_test_util.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -184,7 +184,10 @@ bool TestServerLauncher::Start(Protocol protocol,
   // Deliberately do not pass the --forking flag. It breaks the tests
   // on Windows.
 
-  if (!base::LaunchApp(command_line, false, true, &process_handle_)) {
+  if (!LaunchTestServerAsJob(command_line,
+                             true,
+                             &process_handle_,
+                             &job_handle_)) {
     LOG(ERROR) << "Failed to launch " << command_line;
     return false;
   }
@@ -353,4 +356,59 @@ bool TestServerLauncher::CheckCATrusted() {
   return true;
 }
 
+#if defined(OS_WIN)
+bool LaunchTestServerAsJob(const std::wstring& cmdline,
+                           bool start_hidden,
+                           base::ProcessHandle* process_handle,
+                           ScopedHandle* job_handle) {
+  // Launch test server process.
+  STARTUPINFO startup_info = {0};
+  startup_info.cb = sizeof(startup_info);
+  startup_info.dwFlags = STARTF_USESHOWWINDOW;
+  startup_info.wShowWindow = start_hidden ? SW_HIDE : SW_SHOW;
+  PROCESS_INFORMATION process_info;
+
+  // If this code is run under a debugger, the test server process is
+  // automatically associated with a job object created by the debugger. 
+  // The CREATE_BREAKAWAY_FROM_JOB flag is used to prevent this.
+  if (!CreateProcess(NULL,
+                     const_cast<wchar_t*>(cmdline.c_str()), NULL, NULL,
+                     FALSE, CREATE_BREAKAWAY_FROM_JOB, NULL, NULL,
+                     &startup_info, &process_info)) {
+    LOG(ERROR) << "Could not create process.";
+    return false;
+  }
+  CloseHandle(process_info.hThread);
+
+  // If the caller wants the process handle, we won't close it.
+  if (process_handle) {
+    *process_handle = process_info.hProcess;
+  } else {
+    CloseHandle(process_info.hProcess);
+  }
+
+  // Create a JobObject and associate the test server process with it.
+  job_handle->Set(CreateJobObject(NULL, NULL));
+  if (!job_handle->IsValid()) {
+    LOG(ERROR) << "Could not create JobObject.";
+    return false;
+  } else {
+    JOBOBJECT_EXTENDED_LIMIT_INFORMATION limit_info = {0};
+    limit_info.BasicLimitInformation.LimitFlags =
+        JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE;
+    if (0 == SetInformationJobObject(job_handle->Get(),
+      JobObjectExtendedLimitInformation, &limit_info, sizeof(limit_info))) {
+      LOG(ERROR) << "Could not SetInformationJobObject.";
+      return false;
+    }
+    if (0 == AssignProcessToJobObject(job_handle->Get(),
+                                      process_info.hProcess)) {
+      LOG(ERROR) << "Could not AssignProcessToObject.";
+      return false;
+    }
+  }
+  return true;
+}
+#endif
+
 }  // namespace net
diff --git a/net/socket/ssl_test_util.h b/net/socket/ssl_test_util.h
index 5a987ae..cc3e2ce 100644
--- a/net/socket/ssl_test_util.h
+++ b/net/socket/ssl_test_util.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -11,6 +11,11 @@
 
 #include "base/file_path.h"
 #include "base/process_util.h"
+
+#if defined(OS_WIN)
+#include "base/scoped_handle_win.h"
+#endif
+
 #if defined(USE_NSS)
 #include "base/ref_counted.h"
 #include "net/base/x509_certificate.h"
@@ -111,6 +116,11 @@ class TestServerLauncher {
 
   base::ProcessHandle process_handle_;
 
+#if defined(OS_WIN)
+  // JobObject used to clean up orphaned child processes.
+  ScopedHandle job_handle_;
+#endif
+
   // True if the server should handle each request in a separate process.
   bool forking_;
 
@@ -125,6 +135,15 @@ class TestServerLauncher {
   DISALLOW_COPY_AND_ASSIGN(TestServerLauncher);
 };
 
-}
+#if defined(OS_WIN)
+// Launch test server as a job so that it is not orphaned if the test case is
+// abnormally terminated.
+bool LaunchTestServerAsJob(const std::wstring& cmdline,
+                           bool start_hidden,
+                           base::ProcessHandle* process_handle,
+                           ScopedHandle* job_handle);
+#endif
+
+}  // namespace net
 
 #endif  // NET_SOCKET_SSL_TEST_UTIL_H_