Expose a callback to allow crash server implementations to defer the uploading of crash dumps to a later time. The client can provide a crash_id when the dump is performed and then at a later time connect again and request that the crash id be uploaded triggering an implementation defined callback.

BUG=473
TEST=CrashGenerationServerTest.*
Review URL: https://breakpad.appspot.com/379001

git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@952 4c0a9323-5329-0410-9bdc-e9ce6186880e
This commit is contained in:
cdn@chromium.org 2012-04-13 22:20:30 +00:00
parent 789eac95fd
commit 9a3de4160b
10 changed files with 69 additions and 22 deletions

View File

@ -90,7 +90,8 @@ enum MessageTag {
MESSAGE_TAG_NONE = 0,
MESSAGE_TAG_REGISTRATION_REQUEST = 1,
MESSAGE_TAG_REGISTRATION_RESPONSE = 2,
MESSAGE_TAG_REGISTRATION_ACK = 3
MESSAGE_TAG_REGISTRATION_ACK = 3,
MESSAGE_TAG_UPLOAD_REQUEST = 4
};
struct CustomClientInfo {
@ -102,7 +103,7 @@ struct CustomClientInfo {
struct ProtocolMessage {
ProtocolMessage()
: tag(MESSAGE_TAG_NONE),
pid(0),
id(0),
dump_type(MiniDumpNormal),
thread_id(0),
exception_pointers(NULL),
@ -115,7 +116,7 @@ struct ProtocolMessage {
// Creates an instance with the given parameters.
ProtocolMessage(MessageTag arg_tag,
DWORD arg_pid,
DWORD arg_id,
MINIDUMP_TYPE arg_dump_type,
DWORD* arg_thread_id,
EXCEPTION_POINTERS** arg_exception_pointers,
@ -125,7 +126,7 @@ struct ProtocolMessage {
HANDLE arg_dump_generated_handle,
HANDLE arg_server_alive)
: tag(arg_tag),
pid(arg_pid),
id(arg_id),
dump_type(arg_dump_type),
thread_id(arg_thread_id),
exception_pointers(arg_exception_pointers),
@ -139,8 +140,9 @@ struct ProtocolMessage {
// Tag in the message.
MessageTag tag;
// Process id.
DWORD pid;
// The id for this message. This may be either a process id or a crash id
// depending on the type of message.
DWORD id;
// Dump type requested.
MINIDUMP_TYPE dump_type;

View File

@ -52,7 +52,8 @@ ClientInfo::ClientInfo(CrashGenerationServer* crash_server,
dump_requested_handle_(NULL),
dump_generated_handle_(NULL),
dump_request_wait_handle_(NULL),
process_exit_wait_handle_(NULL) {
process_exit_wait_handle_(NULL),
crash_id_(NULL) {
GetSystemTimeAsFileTime(&start_time_);
}
@ -62,6 +63,12 @@ bool ClientInfo::Initialize() {
return false;
}
// The crash_id will be the low order word of the process creation time.
FILETIME creation_time, exit_time, kernel_time, user_time;
if (GetProcessTimes(process_handle_, &creation_time, &exit_time,
&kernel_time, &user_time))
crash_id_ = creation_time.dwLowDateTime;
dump_requested_handle_ = CreateEvent(NULL, // Security attributes.
TRUE, // Manual reset.
FALSE, // Initial state.

View File

@ -65,6 +65,7 @@ class ClientInfo {
HANDLE process_handle() const { return process_handle_; }
HANDLE dump_requested_handle() const { return dump_requested_handle_; }
HANDLE dump_generated_handle() const { return dump_generated_handle_; }
DWORD crash_id() const { return crash_id_; }
HANDLE dump_request_wait_handle() const {
return dump_request_wait_handle_;
@ -160,6 +161,11 @@ class ClientInfo {
// for the client process when it signals a crash.
FILETIME start_time_;
// The crash id which can be used to request an upload. This will be the
// value of the low order dword of the process creation time for the process
// being dumped.
DWORD crash_id_;
// Disallow copy ctor and operator=.
ClientInfo(const ClientInfo& client_info);
ClientInfo& operator=(const ClientInfo& client_info);

View File

@ -223,7 +223,7 @@ bool CrashGenerationClient::RegisterClient(HANDLE pipe) {
crash_event_ = reply.dump_request_handle;
crash_generated_ = reply.dump_generated_handle;
server_alive_ = reply.server_alive_handle;
server_process_id_ = reply.pid;
server_process_id_ = reply.id;
return true;
}
@ -261,7 +261,7 @@ HANDLE CrashGenerationClient::ConnectToPipe(const wchar_t* pipe_name,
bool CrashGenerationClient::ValidateResponse(
const ProtocolMessage& msg) const {
return (msg.tag == MESSAGE_TAG_REGISTRATION_RESPONSE) &&
(msg.pid != 0) &&
(msg.id != 0) &&
(msg.dump_request_handle != NULL) &&
(msg.dump_generated_handle != NULL) &&
(msg.server_alive_handle != NULL);

View File

@ -84,11 +84,12 @@ static const int kShutdownDelayMs = 10000;
static const int kShutdownSleepIntervalMs = 5;
static bool IsClientRequestValid(const ProtocolMessage& msg) {
return msg.tag == MESSAGE_TAG_REGISTRATION_REQUEST &&
msg.pid != 0 &&
msg.thread_id != NULL &&
msg.exception_pointers != NULL &&
msg.assert_info != NULL;
return msg.tag == MESSAGE_TAG_UPLOAD_REQUEST ||
(msg.tag == MESSAGE_TAG_REGISTRATION_REQUEST &&
msg.id != 0 &&
msg.thread_id != NULL &&
msg.exception_pointers != NULL &&
msg.assert_info != NULL);
}
CrashGenerationServer::CrashGenerationServer(
@ -100,6 +101,8 @@ CrashGenerationServer::CrashGenerationServer(
void* dump_context,
OnClientExitedCallback exit_callback,
void* exit_context,
OnClientUploadRequestCallback upload_request_callback,
void* upload_context,
bool generate_dumps,
const std::wstring* dump_path)
: pipe_name_(pipe_name),
@ -113,6 +116,8 @@ CrashGenerationServer::CrashGenerationServer(
dump_context_(dump_context),
exit_callback_(exit_callback),
exit_context_(exit_context),
upload_request_callback_(upload_request_callback),
upload_context_(upload_context),
generate_dumps_(generate_dumps),
dump_generator_(NULL),
server_state_(IPC_SERVER_STATE_UNINITIALIZED),
@ -416,9 +421,15 @@ void CrashGenerationServer::HandleReadDoneState() {
return;
}
if (msg_.tag == MESSAGE_TAG_UPLOAD_REQUEST) {
if (upload_request_callback_)
upload_request_callback_(upload_context_, msg_.id);
return;
}
scoped_ptr<ClientInfo> client_info(
new ClientInfo(this,
msg_.pid,
msg_.id,
msg_.dump_type,
msg_.thread_id,
msg_.exception_pointers,
@ -582,7 +593,7 @@ void CrashGenerationServer::EnterStateImmediately(IPCServerState state) {
bool CrashGenerationServer::PrepareReply(const ClientInfo& client_info,
ProtocolMessage* reply) const {
reply->tag = MESSAGE_TAG_REGISTRATION_RESPONSE;
reply->pid = GetCurrentProcessId();
reply->id = GetCurrentProcessId();
if (CreateClientHandles(client_info, reply)) {
return true;

View File

@ -59,6 +59,9 @@ class CrashGenerationServer {
typedef void (*OnClientExitedCallback)(void* context,
const ClientInfo* client_info);
typedef void (*OnClientUploadRequestCallback)(void* context,
const DWORD crash_id);
// Creates an instance with the given parameters.
//
// Parameter pipe_name: Name of the Windows named pipe
@ -86,6 +89,8 @@ class CrashGenerationServer {
void* dump_context,
OnClientExitedCallback exit_callback,
void* exit_context,
OnClientUploadRequestCallback upload_request_callback,
void* upload_context,
bool generate_dumps,
const std::wstring* dump_path);
@ -250,6 +255,12 @@ class CrashGenerationServer {
// Context for client process exit callback.
void* exit_context_;
// Callback for upload request.
OnClientUploadRequestCallback upload_request_callback_;
// Context for upload request callback.
void* upload_context_;
// Whether to generate dumps.
bool generate_dumps_;

View File

@ -291,6 +291,8 @@ void CrashServerStart() {
NULL,
ShowClientExited,
NULL,
NULL,
NULL,
true,
&dump_path);

View File

@ -65,6 +65,7 @@ class CrashGenerationServerTest : public ::testing::Test {
CallOnClientConnected, &mock_callbacks_,
CallOnClientDumpRequested, &mock_callbacks_,
CallOnClientExited, &mock_callbacks_,
CallOnClientUploadRequested, &mock_callbacks_,
false,
NULL),
thread_id_(0),
@ -82,6 +83,8 @@ class CrashGenerationServerTest : public ::testing::Test {
const std::wstring* file_path));
MOCK_METHOD1(OnClientExited,
void(const google_breakpad::ClientInfo* client_info));
MOCK_METHOD1(OnClientUploadRequested,
void(const DWORD crash_id));
};
enum ClientFault {
@ -248,6 +251,11 @@ class CrashGenerationServerTest : public ::testing::Test {
OnClientExited(client_info);
}
static void CallOnClientUploadRequested(void* context, const DWORD crash_id) {
static_cast<MockCrashGenerationServerCallbacks*>(context)->
OnClientUploadRequested(crash_id);
}
DWORD thread_id_;
EXCEPTION_POINTERS* exception_pointers_;
MDRawAssertionInfo assert_info_;

View File

@ -161,8 +161,8 @@ TEST_F(ExceptionHandlerDeathTest, OutOfProcTest) {
ASSERT_TRUE(DoesPathExist(temp_path_));
std::wstring dump_path(temp_path_);
google_breakpad::CrashGenerationServer server(
kPipeName, NULL, NULL, NULL, &clientDumpCallback, NULL, NULL, NULL, true,
&dump_path);
kPipeName, NULL, NULL, NULL, &clientDumpCallback, NULL, NULL, NULL, NULL,
NULL, true, &dump_path);
// This HAS to be EXPECT_, because when this test case is executed in the
// child process, the server registration will fail due to the named pipe

View File

@ -220,8 +220,8 @@ TEST_F(ExceptionHandlerTest, InvalidParameterMiniDumpTest) {
ASSERT_TRUE(DoesPathExist(temp_path_));
wstring dump_path(temp_path_);
google_breakpad::CrashGenerationServer server(
kPipeName, NULL, NULL, NULL, ClientDumpCallback, NULL, NULL, NULL, true,
&dump_path);
kPipeName, NULL, NULL, NULL, ClientDumpCallback, NULL, NULL, NULL, NULL,
NULL, true, &dump_path);
ASSERT_TRUE(dump_file.empty() && full_dump_file.empty());
@ -291,8 +291,8 @@ TEST_F(ExceptionHandlerTest, PureVirtualCallMiniDumpTest) {
ASSERT_TRUE(DoesPathExist(temp_path_));
wstring dump_path(temp_path_);
google_breakpad::CrashGenerationServer server(
kPipeName, NULL, NULL, NULL, ClientDumpCallback, NULL, NULL, NULL, true,
&dump_path);
kPipeName, NULL, NULL, NULL, ClientDumpCallback, NULL, NULL, NULL, NULL,
NULL, true, &dump_path);
ASSERT_TRUE(dump_file.empty() && full_dump_file.empty());