Implement a Windows crash client registrar.

Implement a Windows crash client registrar.

BUG=

[erikwright (departed), 2015-06-29 14:29:48]

erikwright@chromium.org changed reviewers:
+ scottmg@chromium.org

[erikwright (departed), 2015-06-29 14:29:49]

Scott, PTAL.

[erikwright (departed), 2015-06-29 14:32:44]

On 2015/06/29 14:29:49, erikwright wrote:
> Scott, PTAL.

Something worth considering is what happens if the reporter process crashes.

A client that signals the reporter process, then blocks on the "report complete"
event, will hang forever.

I think it might be appropriate to extend the event pair to include a handle to
the reporter process with SYNCHRONIZE privilege.

The client would then wait on both the reporter process and the "dump complete"
event. If either is signaled, the request is considered complete (success or
failure).

I would not include that in this CL.

[scottmg, 2015-06-29 20:29:05]

This is great. I'm just a little concerned about the complexity of the
threadpool usage.

https://codereview.chromium.org/1213723004/diff/40001/handler/win/registrar.cc
File handler/win/registrar.cc (right):

https://codereview.chromium.org/1213723004/diff/40001/handler/win/registrar.c...
handler/win/registrar.cc:100: // Helps clean up a registered wait handle in case
of failure. Use Receive() to store a
80 col

https://codereview.chromium.org/1213723004/diff/40001/handler/win/registrar.c...
handler/win/registrar.cc:157: bool LoggedCreateEvent(ScopedKernelHANDLE*
destination) {
Logged -> Logging

https://codereview.chromium.org/1213723004/diff/40001/handler/win/registrar.c...
handler/win/registrar.cc:233: // The ReportRequestCallback, on the other hand,
may still trigger a report now
80 col

https://codereview.chromium.org/1213723004/diff/40001/handler/win/registrar.c...
handler/win/registrar.cc:270: // If the process has already exited the "process
exit" callback may very well
The number of cases here worries me. Is it possible to do a more plain acquire,
then insert, then register? And similarly have the destruction/exit go through
single path that takes a (perhaps) larger lock and then updates the state
atomically in one place?

https://codereview.chromium.org/1213723004/diff/40001/handler/win/registrar.h
File handler/win/registrar.h (right):

https://codereview.chromium.org/1213723004/diff/40001/handler/win/registrar.h...
handler/win/registrar.h:57: //! \param client_process The client to register.
I think it needs a \n before \param line here.

https://codereview.chromium.org/1213723004/diff/40001/handler/win/registrar.h...
handler/win/registrar.h:57: //! \param client_process The client to register.
\param lines are one of \param[in], \param[out], or \param[inout] in Crashpad, I
don't know if that's mandatory for doxygen or not.

https://codereview.chromium.org/1213723004/diff/40001/handler/win/registrar_t...
File handler/win/registrar_test.cc (right):

https://codereview.chromium.org/1213723004/diff/40001/handler/win/registrar_t...
handler/win/registrar_test.cc:36: DWORD bytes_read = 0;
Replace 36-38 with CheckReadFile() from util/file/file_io.h.

https://codereview.chromium.org/1213723004/diff/40001/handler/win/registrar_t...
handler/win/registrar_test.cc:44: DWORD bytes_written = 0;
And CheckedWriteFile here.