Fix CrashGenerationServer to recover from protocol errors and a test for same.

R=siggi at http://breakpad.appspot.com/196001/show


git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@695 4c0a9323-5329-0410-9bdc-e9ce6186880e
This commit is contained in:
erikwright@chromium.org 2010-09-20 21:35:24 +00:00
parent 378e28e301
commit b6ee7dcb22
6 changed files with 442 additions and 77 deletions

View file

@ -34,6 +34,8 @@
#include "client/windows/common/auto_critical_section.h"
#include "processor/scoped_ptr.h"
#include "client/windows/crash_generation/client_info.h"
namespace google_breakpad {
// Output buffer size.
@ -113,7 +115,7 @@ CrashGenerationServer::CrashGenerationServer(
exit_context_(exit_context),
generate_dumps_(generate_dumps),
dump_generator_(NULL),
server_state_(IPC_SERVER_STATE_INITIAL),
server_state_(IPC_SERVER_STATE_UNINITIALIZED),
shutting_down_(false),
overlapped_(),
client_info_(NULL),
@ -197,10 +199,18 @@ CrashGenerationServer::~CrashGenerationServer() {
CloseHandle(server_alive_handle_);
}
if (overlapped_.hEvent) {
CloseHandle(overlapped_.hEvent);
}
DeleteCriticalSection(&clients_sync_);
}
bool CrashGenerationServer::Start() {
if (server_state_ != IPC_SERVER_STATE_UNINITIALIZED) {
return false;
}
server_state_ = IPC_SERVER_STATE_INITIAL;
server_alive_handle_ = CreateMutex(NULL, TRUE, NULL);
@ -239,9 +249,12 @@ bool CrashGenerationServer::Start() {
return false;
}
// Signal the event to start a separate thread to handle
// client connections.
return SetEvent(overlapped_.hEvent) != FALSE;
// Kick-start the state machine. This will initiate an asynchronous wait
// for client connections.
HandleInitialState();
// If we are in error state, it's because we failed to start listening.
return server_state_ != IPC_SERVER_STATE_ERROR;
}
// If the server thread serving clients ever gets into the
@ -283,33 +296,29 @@ void CrashGenerationServer::HandleInitialState() {
assert(server_state_ == IPC_SERVER_STATE_INITIAL);
if (!ResetEvent(overlapped_.hEvent)) {
server_state_ = IPC_SERVER_STATE_ERROR;
EnterErrorState();
return;
}
bool success = ConnectNamedPipe(pipe_, &overlapped_) != FALSE;
DWORD error_code = success ? ERROR_SUCCESS : GetLastError();
// From MSDN, it is not clear that when ConnectNamedPipe is used
// in an overlapped mode, will it ever return non-zero value, and
// if so, in what cases.
assert(!success);
DWORD error_code = GetLastError();
switch (error_code) {
case ERROR_IO_PENDING:
server_state_ = IPC_SERVER_STATE_CONNECTING;
EnterStateWhenSignaled(IPC_SERVER_STATE_CONNECTING);
break;
case ERROR_PIPE_CONNECTED:
if (SetEvent(overlapped_.hEvent)) {
server_state_ = IPC_SERVER_STATE_CONNECTED;
} else {
server_state_ = IPC_SERVER_STATE_ERROR;
}
EnterStateImmediately(IPC_SERVER_STATE_CONNECTED);
break;
default:
server_state_ = IPC_SERVER_STATE_ERROR;
EnterErrorState();
break;
}
}
@ -328,14 +337,14 @@ void CrashGenerationServer::HandleConnectingState() {
&overlapped_,
&bytes_count,
FALSE) != FALSE;
DWORD error_code = success ? ERROR_SUCCESS : GetLastError();
if (success) {
server_state_ = IPC_SERVER_STATE_CONNECTED;
return;
}
if (GetLastError() != ERROR_IO_INCOMPLETE) {
server_state_ = IPC_SERVER_STATE_DISCONNECTING;
EnterStateImmediately(IPC_SERVER_STATE_CONNECTED);
} else if (error_code != ERROR_IO_INCOMPLETE) {
EnterStateImmediately(IPC_SERVER_STATE_DISCONNECTING);
} else {
// remain in CONNECTING state
}
}
@ -353,16 +362,17 @@ void CrashGenerationServer::HandleConnectedState() {
sizeof(msg_),
&bytes_count,
&overlapped_) != FALSE;
DWORD error_code = success ? ERROR_SUCCESS : GetLastError();
// Note that the asynchronous read issued above can finish before the
// code below executes. But, it is okay to change state after issuing
// the asynchronous read. This is because even if the asynchronous read
// is done, the callback for it would not be executed until the current
// thread finishes its execution.
if (success || GetLastError() == ERROR_IO_PENDING) {
server_state_ = IPC_SERVER_STATE_READING;
if (success || error_code == ERROR_IO_PENDING) {
EnterStateWhenSignaled(IPC_SERVER_STATE_READING);
} else {
server_state_ = IPC_SERVER_STATE_DISCONNECTING;
EnterStateImmediately(IPC_SERVER_STATE_DISCONNECTING);
}
}
@ -378,21 +388,18 @@ void CrashGenerationServer::HandleReadingState() {
&overlapped_,
&bytes_count,
FALSE) != FALSE;
DWORD error_code = success ? ERROR_SUCCESS : GetLastError();
if (success && bytes_count == sizeof(ProtocolMessage)) {
server_state_ = IPC_SERVER_STATE_READ_DONE;
return;
EnterStateImmediately(IPC_SERVER_STATE_READ_DONE);
} else {
// We should never get an I/O incomplete since we should not execute this
// unless the Read has finished and the overlapped event is signaled. If
// we do get INCOMPLETE, we have a bug in our code.
assert(error_code != ERROR_IO_INCOMPLETE);
EnterStateImmediately(IPC_SERVER_STATE_DISCONNECTING);
}
DWORD error_code;
error_code = GetLastError();
// We should never get an I/O incomplete since we should not execute this
// unless the Read has finished and the overlapped event is signaled. If
// we do get INCOMPLETE, we have a bug in our code.
assert(error_code != ERROR_IO_INCOMPLETE);
server_state_ = IPC_SERVER_STATE_DISCONNECTING;
}
// When the server thread serving the client is in the READ_DONE state,
@ -405,7 +412,7 @@ void CrashGenerationServer::HandleReadDoneState() {
assert(server_state_ == IPC_SERVER_STATE_READ_DONE);
if (!IsClientRequestValid(msg_)) {
server_state_ = IPC_SERVER_STATE_DISCONNECTING;
EnterStateImmediately(IPC_SERVER_STATE_DISCONNECTING);
return;
}
@ -419,22 +426,26 @@ void CrashGenerationServer::HandleReadDoneState() {
msg_.custom_client_info));
if (!client_info->Initialize()) {
server_state_ = IPC_SERVER_STATE_DISCONNECTING;
EnterStateImmediately(IPC_SERVER_STATE_DISCONNECTING);
return;
}
// Issues an asynchronous WriteFile call if successful.
// Iff successful, assigns ownership of the client_info pointer to the server
// instance, in which case we must be sure not to free it in this function.
if (!RespondToClient(client_info.get())) {
server_state_ = IPC_SERVER_STATE_DISCONNECTING;
EnterStateImmediately(IPC_SERVER_STATE_DISCONNECTING);
return;
}
client_info_ = client_info.release();
// Note that the asynchronous write issued by RespondToClient function
// can finish before the code below executes. But it is okay to change
// state after issuing the asynchronous write. This is because even if
// the asynchronous write is done, the callback for it would not be
// executed until the current thread finishes its execution.
server_state_ = IPC_SERVER_STATE_WRITING;
client_info_ = client_info.release();
EnterStateWhenSignaled(IPC_SERVER_STATE_WRITING);
}
// When the server thread serving the clients is in the WRITING state,
@ -449,21 +460,19 @@ void CrashGenerationServer::HandleWritingState() {
&overlapped_,
&bytes_count,
FALSE) != FALSE;
DWORD error_code = success ? ERROR_SUCCESS : GetLastError();
if (success) {
server_state_ = IPC_SERVER_STATE_WRITE_DONE;
EnterStateImmediately(IPC_SERVER_STATE_WRITE_DONE);
return;
}
DWORD error_code;
error_code = GetLastError();
// We should never get an I/O incomplete since we should not execute this
// unless the Write has finished and the overlapped event is signaled. If
// we do get INCOMPLETE, we have a bug in our code.
assert(error_code != ERROR_IO_INCOMPLETE);
server_state_ = IPC_SERVER_STATE_DISCONNECTING;
EnterStateImmediately(IPC_SERVER_STATE_DISCONNECTING);
}
// When the server thread serving the clients is in the WRITE_DONE state,
@ -473,23 +482,20 @@ void CrashGenerationServer::HandleWritingState() {
void CrashGenerationServer::HandleWriteDoneState() {
assert(server_state_ == IPC_SERVER_STATE_WRITE_DONE);
server_state_ = IPC_SERVER_STATE_READING_ACK;
DWORD bytes_count = 0;
bool success = ReadFile(pipe_,
&msg_,
sizeof(msg_),
&bytes_count,
&overlapped_) != FALSE;
&msg_,
sizeof(msg_),
&bytes_count,
&overlapped_) != FALSE;
DWORD error_code = success ? ERROR_SUCCESS : GetLastError();
if (success) {
return;
}
DWORD error_code = GetLastError();
if (error_code != ERROR_IO_PENDING) {
server_state_ = IPC_SERVER_STATE_DISCONNECTING;
EnterStateImmediately(IPC_SERVER_STATE_READING_ACK);
} else if (error_code == ERROR_IO_PENDING) {
EnterStateWhenSignaled(IPC_SERVER_STATE_READING_ACK);
} else {
EnterStateImmediately(IPC_SERVER_STATE_DISCONNECTING);
}
}
@ -503,6 +509,7 @@ void CrashGenerationServer::HandleReadingAckState() {
&overlapped_,
&bytes_count,
FALSE) != FALSE;
DWORD error_code = success ? ERROR_SUCCESS : GetLastError();
if (success) {
// The connection handshake with the client is now complete; perform
@ -511,15 +518,13 @@ void CrashGenerationServer::HandleReadingAckState() {
connect_callback_(connect_context_, client_info_);
}
} else {
DWORD error_code = GetLastError();
// We should never get an I/O incomplete since we should not execute this
// unless the Read has finished and the overlapped event is signaled. If
// we do get INCOMPLETE, we have a bug in our code.
assert(error_code != ERROR_IO_INCOMPLETE);
}
server_state_ = IPC_SERVER_STATE_DISCONNECTING;
EnterStateImmediately(IPC_SERVER_STATE_DISCONNECTING);
}
// When the server thread serving the client is in the DISCONNECTING state,
@ -539,12 +544,12 @@ void CrashGenerationServer::HandleDisconnectingState() {
overlapped_.Pointer = NULL;
if (!ResetEvent(overlapped_.hEvent)) {
server_state_ = IPC_SERVER_STATE_ERROR;
EnterErrorState();
return;
}
if (!DisconnectNamedPipe(pipe_)) {
server_state_ = IPC_SERVER_STATE_ERROR;
EnterErrorState();
return;
}
@ -554,7 +559,21 @@ void CrashGenerationServer::HandleDisconnectingState() {
return;
}
server_state_ = IPC_SERVER_STATE_INITIAL;
EnterStateImmediately(IPC_SERVER_STATE_INITIAL);
}
void CrashGenerationServer::EnterErrorState() {
SetEvent(overlapped_.hEvent);
server_state_ = IPC_SERVER_STATE_ERROR;
}
void CrashGenerationServer::EnterStateWhenSignaled(IPCServerState state) {
server_state_ = state;
}
void CrashGenerationServer::EnterStateImmediately(IPCServerState state) {
server_state_ = state;
if (!SetEvent(overlapped_.hEvent)) {
server_state_ = IPC_SERVER_STATE_ERROR;
}
@ -626,18 +645,25 @@ bool CrashGenerationServer::RespondToClient(ClientInfo* client_info) {
return false;
}
DWORD bytes_count = 0;
bool success = WriteFile(pipe_,
&reply,
sizeof(reply),
&bytes_count,
&overlapped_) != FALSE;
DWORD error_code = success ? ERROR_SUCCESS : GetLastError();
if (!success && error_code != ERROR_IO_PENDING) {
return false;
}
// Takes over ownership of client_info. We MUST return true if AddClient
// succeeds.
if (!AddClient(client_info)) {
return false;
}
DWORD bytes_count = 0;
bool success = WriteFile(pipe_,
&reply,
sizeof(reply),
&bytes_count,
&overlapped_) != FALSE;
return success || GetLastError() == ERROR_IO_PENDING;
return true;
}
// The server thread servicing the clients runs this method. The method
@ -739,7 +765,7 @@ bool CrashGenerationServer::AddClient(ClientInfo* client_info) {
// static
void CALLBACK CrashGenerationServer::OnPipeConnected(void* context, BOOLEAN) {
assert (context);
assert(context);
CrashGenerationServer* obj =
reinterpret_cast<CrashGenerationServer*>(context);