Fix an AppVerifier STOP in OOP server code. In the destructor of

the OOP server, we need to wait for any pending I/O to be done.



git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@308 4c0a9323-5329-0410-9bdc-e9ce6186880e

2 files changed, 22 insertions, 1 deletions

diff --git a/src/client/windows/crash_generation/crash_generation_server.cc b/src/client/windows/crash_generation/crash_generation_server.cc
index b13db9db..ac76e590 100644
--- a/src/client/windows/crash_generation/crash_generation_server.cc
+++ b/src/client/windows/crash_generation/crash_generation_server.cc
@@ -129,6 +129,19 @@ CrashGenerationServer::~CrashGenerationServer() {
   // Indicate to existing threads that server is shutting down.
   shutting_down_ = true;
 
+  // Even if there are no current worker threads running, it is possible that
+  // an I/O request is pending on the pipe right now but not yet done. In fact,
+  // it's very likely this is the case unless we are in an ERROR state. If we
+  // don't wait for the pending I/O to be done, then when the I/O completes,
+  // it may write to invalid memory. AppVerifier will flag this problem too.
+  // So we disconnect from the pipe and then wait for the server to get into
+  // error state so that the pending I/O will fail and get cleared.
+  DisconnectNamedPipe(pipe_);
+  int num_tries = 100;
+  while (num_tries-- && server_state_ != IPC_SERVER_STATE_ERROR) {
+    Sleep(10);
+  }
+
   // Unregister wait on the pipe.
   if (pipe_wait_handle_) {
     // Wait for already executing callbacks to finish.
@@ -629,6 +642,14 @@ bool CrashGenerationServer::RespondToClient(ClientInfo* client_info) {
 // implements the state machine described in ReadMe.txt along with the
 // helper methods HandleXXXState.
 void CrashGenerationServer::HandleConnectionRequest() {
+  // If we are shutting doen then get into ERROR state, reset the event so more
+  // workers don't run and return immediately.
+  if (shutting_down_) {
+    server_state_ = IPC_SERVER_STATE_ERROR;
+    ResetEvent(overlapped_.hEvent);
+    return;
+  }
+
   switch (server_state_) {
     case IPC_SERVER_STATE_ERROR:
       HandleErrorState();
diff --git a/src/client/windows/crash_generation/crash_generation_server.h b/src/client/windows/crash_generation/crash_generation_server.h
index b67b88de..cacb639a 100644
--- a/src/client/windows/crash_generation/crash_generation_server.h
+++ b/src/client/windows/crash_generation/crash_generation_server.h
@@ -241,7 +241,7 @@ class CrashGenerationServer {
   // Note that since we restrict the pipe to one instance, we
   // only need to keep one state of the server. Otherwise, server
   // would have one state per client it is talking to.
-  IPCServerState server_state_;
+  volatile IPCServerState server_state_;
 
   // Whether the server is shutting down.
   volatile bool shutting_down_;