Add sandbox support for associating peer processes

Add sandbox support for associating peer processes 

TEST=HandlePolicyTest.DuplicatePeerHandle

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=131940
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=132112

[cpu_(ooo_6.6-7.5), 2012-04-09 23:17:40]

http://codereview.chromium.org/9960045/diff/3002/sandbox/src/broker_services.cc
File sandbox/src/broker_services.cc (right):

http://codereview.chromium.org/9960045/diff/3002/sandbox/src/broker_services....
sandbox/src/broker_services.cc:334: FALSE, 0)) {
UnregisterWaitEx with some magic params.

http://codereview.chromium.org/9960045/diff/3002/sandbox/src/broker_services....
sandbox/src/broker_services.cc:338: HANDLE timer;
wait_object

http://codereview.chromium.org/9960045/diff/3002/sandbox/src/broker_services....
sandbox/src/broker_services.cc:346: ::CancelWaitableTimer(timer);
UnregisterWait

[jschuh, 2012-04-10 00:10:11]

http://codereview.chromium.org/9960045/diff/3002/sandbox/src/broker_services.cc
File sandbox/src/broker_services.cc (right):

http://codereview.chromium.org/9960045/diff/3002/sandbox/src/broker_services....
sandbox/src/broker_services.cc:334: FALSE, 0)) {
On 2012/04/09 23:17:40, cpu wrote:
> UnregisterWaitEx with some magic params.

Done.

http://codereview.chromium.org/9960045/diff/3002/sandbox/src/broker_services....
sandbox/src/broker_services.cc:338: HANDLE timer;
On 2012/04/09 23:17:40, cpu wrote:
> wait_object

Done.

http://codereview.chromium.org/9960045/diff/3002/sandbox/src/broker_services....
sandbox/src/broker_services.cc:346: ::CancelWaitableTimer(timer);
On 2012/04/09 23:17:40, cpu wrote:
> UnregisterWait

Done.

[jschuh, 2012-04-10 17:43:08]

I realized I had a shutdown race so I had to modify the code to account for it.
I'm now tracking the peers in a map indexed by the process ID, and clean up
outstanding waits at shutdown.

[cpu_(ooo_6.6-7.5), 2012-04-10 22:00:14]

lgtm

http://codereview.chromium.org/9960045/diff/17/sandbox/src/broker_services.h
File sandbox/src/broker_services.h (right):

http://codereview.chromium.org/9960045/diff/17/sandbox/src/broker_services.h#...
sandbox/src/broker_services.h:70: typedef struct PeerTracker {
please add a fwd decl here are move PeerTracker definition to the cc file.

[commit-bot: I haz the power, 2012-04-10 22:14:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jschuh@chromium.org/9960045/19001

[commit-bot: I haz the power, 2012-04-12 01:48:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jschuh@chromium.org/9960045/50001

[cpu_(ooo_6.6-7.5), 2012-04-12 02:17:52]

http://codereview.chromium.org/9960045/diff/50001/sandbox/src/broker_services.cc
File sandbox/src/broker_services.cc (right):

http://codereview.chromium.org/9960045/diff/50001/sandbox/src/broker_services...
sandbox/src/broker_services.cc:114: }
sounds like you want peer_map.clear() after 113 so the callback does not find a
wait object that is was already unregistered.

[jschuh, 2012-04-12 03:04:22]

http://codereview.chromium.org/9960045/diff/50001/sandbox/src/broker_services.cc
File sandbox/src/broker_services.cc (right):

http://codereview.chromium.org/9960045/diff/50001/sandbox/src/broker_services...
sandbox/src/broker_services.cc:114: }
On 2012/04/12 02:17:52, cpu wrote:
> sounds like you want peer_map.clear() after 113 so the callback does not find
a
> wait object that is was already unregistered.

Oops. Looks like I accidentally dropped the "erase" on the last upload. Thanks
for catching it.

[commit-bot: I haz the power, 2012-04-12 03:13:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jschuh@chromium.org/9960045/50004

[commit-bot: I haz the power, 2012-04-12 06:01:02]

Change committed as 131940

[jschuh, 2012-04-12 17:19:44]

After going through this, I don't think there's any safe way to use
RegisterWaitForSingleObject here. At shutdown I'll either get deadlocks (from a
blocked handler) or crashes from an invalid access due to a handler that
launched too late. So, I just created a separate wait thread, which I can kill
at shutdown, thus avoiding the whole headache.

[cpu_(ooo_6.6-7.5), 2012-04-12 19:59:09]

http://codereview.chromium.org/9960045/diff/52001/sandbox/src/broker_services.cc
File sandbox/src/broker_services.cc (right):

http://codereview.chromium.org/9960045/diff/52001/sandbox/src/broker_services...
sandbox/src/broker_services.cc:413: if (!::PulseEvent(peer_event_))
nooooo....

[jschuh, 2012-04-12 22:10:54]

Okay, moved all the peer map handling onto worker thread, where it's serialized
and terminated before we destroy the broker. So, there should no longer be any
risk of deadlock or use after free.

[cpu_(ooo_6.6-7.5), 2012-04-13 00:13:03]

lgtm

http://codereview.chromium.org/9960045/diff/55001/sandbox/src/broker_services.cc
File sandbox/src/broker_services.cc (right):

http://codereview.chromium.org/9960045/diff/55001/sandbox/src/broker_services...
sandbox/src/broker_services.cc:45: THREAD_CTRL_REMOVE_PEER,
please leave ctrl_quit last.

http://codereview.chromium.org/9960045/diff/55001/sandbox/src/broker_services...
sandbox/src/broker_services.cc:63: DWORD id_;
remove the trailing underscores, it is a struct. Then watch out with self
assignment in the ctor.

http://codereview.chromium.org/9960045/diff/55001/sandbox/src/broker_services...
sandbox/src/broker_services.cc:248: // The broker object is being destroyed so
the thread needs to exit.
please move the THREAD_CTRL_QUIT to the end.

[jschuh, 2012-04-13 00:25:43]

http://codereview.chromium.org/9960045/diff/55001/sandbox/src/broker_services.cc
File sandbox/src/broker_services.cc (right):

http://codereview.chromium.org/9960045/diff/55001/sandbox/src/broker_services...
sandbox/src/broker_services.cc:45: THREAD_CTRL_REMOVE_PEER,
On 2012/04/13 00:13:03, cpu wrote:
> please leave ctrl_quit last.

Done.

http://codereview.chromium.org/9960045/diff/55001/sandbox/src/broker_services...
sandbox/src/broker_services.cc:63: DWORD id_;
On 2012/04/13 00:13:03, cpu wrote:
> remove the trailing underscores, it is a struct. Then watch out with self
> assignment in the ctor.

Done.

http://codereview.chromium.org/9960045/diff/55001/sandbox/src/broker_services...
sandbox/src/broker_services.cc:248: // The broker object is being destroyed so
the thread needs to exit.
On 2012/04/13 00:13:03, cpu wrote:
> please move the THREAD_CTRL_QUIT to the end.

Done.

[commit-bot: I haz the power, 2012-04-13 00:26:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jschuh@chromium.org/9960045/50040