CrashGenerationServer's state machine in can be invoked from both the application thread and thread…

CrashGenerationServer's state machine can be invoked from both the application
thread and thread pool threads. This CL serializes access to the FSM state.
Handling of crash dump and client shutdown requests is still done
asynchronously.

BUG=132164
TEST=remoting_unittests.BreakpadWinDeathTest.*

[alexeypa]

Please take a look.

[Mark Mentovai]

https://breakpad.appspot.com/396002/diff/1/client/windows/crash_generation/cr...
File client/windows/crash_generation/crash_generation_server.cc (right):

https://breakpad.appspot.com/396002/diff/1/client/windows/crash_generation/cr...
client/windows/crash_generation/crash_generation_server.cc:131:
EnterCriticalSection(&sync_);
This is a little scary without a scoper.

https://breakpad.appspot.com/396002/diff/1/client/windows/crash_generation/cr...
client/windows/crash_generation/crash_generation_server.cc:146:
LeaveCriticalSection(&sync_);
but I understand why you didn’t use a scoper.

Could the scoper be extended to handle this pattern?

[alexeypa]

FYI.

https://breakpad.appspot.com/396002/diff/1/client/windows/crash_generation/cr...
File client/windows/crash_generation/crash_generation_server.cc (right):

https://breakpad.appspot.com/396002/diff/1/client/windows/crash_generation/cr...
client/windows/crash_generation/crash_generation_server.cc:146:
LeaveCriticalSection(&sync_);
On 2012/06/12 16:42:40, Mark Mentovai wrote:
> Could the scoper be extended to handle this pattern?

I've added Acquire() and Release() methods to the scoper along with a couple of
asserts to catch invalid call patterns.

https://breakpad.appspot.com/396002/diff/1/client/windows/crash_generation/cr...
client/windows/crash_generation/crash_generation_server.cc:185:
AutoCriticalSection lock(&clients_sync_);
I'm not sure how I was able to compile it before. I deleted all build files and
rebuilt it from scratch this time to make sure it works. I also verified in the
debugger that the code I'm running is actually compiled from the new source.

[Mark Mentovai]

https://breakpad.appspot.com/396002/diff/11001/client/windows/crash_generatio...
File client/windows/crash_generation/crash_generation_server.cc (right):

https://breakpad.appspot.com/396002/diff/11001/client/windows/crash_generatio...
client/windows/crash_generation/crash_generation_server.cc:139: // an I/O
request is pending on the pipe right now but not yet done. In fact,
80 columns

(When you reindent code because you change the scoping level, you need to watch
out for this.)

https://breakpad.appspot.com/396002/diff/11001/client/windows/crash_generatio...
client/windows/crash_generation/crash_generation_server.cc:148: lock.Release();
This is better.

[alexeypa]

https://breakpad.appspot.com/396002/diff/11001/client/windows/crash_generatio...
File client/windows/crash_generation/crash_generation_server.cc (right):

https://breakpad.appspot.com/396002/diff/11001/client/windows/crash_generatio...
client/windows/crash_generation/crash_generation_server.cc:139: // an I/O
request is pending on the pipe right now but not yet done. In fact,
On 2012/06/12 19:33:50, Mark Mentovai wrote:
> 80 columns

Done.

[Mark Mentovai]

LGTM

[alexeypa]

On 2012/06/12 20:57:29, Mark Mentovai wrote:
> LGTM

Thanks. I'm not a Breakpad committer though. What is the best way to check this
in?

[Mark Mentovai]

I committed it for you at r970.