From 104e4e01146aca48dfe3e639a33f96c5e1477151 Mon Sep 17 00:00:00 2001 From: doshimun Date: Fri, 16 Jan 2009 22:37:48 +0000 Subject: 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 --- .../crash_generation/crash_generation_server.cc | 21 +++++++++++++++++++++ .../crash_generation/crash_generation_server.h | 2 +- 2 files changed, 22 insertions(+), 1 deletion(-) (limited to 'src/client/windows/crash_generation') 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_; -- cgit v1.2.1