Fix shutdown UAF in MessagePipeDispatcher in new Mojo EDK.

Fix shutdown UAF in MessagePipeDispatcher in new Mojo EDK.

The problem was that the PostTask to the IO thread might not run if the IO thread has shutdown. In that case the MessagePipeDispatcher would get destructed, and RawChannel (called by IO thread destruction) can call its OnError method. Fix this by manually refcounting so that in this situation we just leak the MessagePipeDispatcher instead.

BUG=561803
Committed: https://crrev.com/d0a0ab7efa8f9e7eb2124468070e79c2786113a8
Cr-Commit-Position: refs/heads/master@{#365881}

[jam, 2015-12-17 18:38:14]

jam@chromium.org changed reviewers:
+ msw@chromium.org

[msw, 2015-12-17 18:49:59]

lgtm with a nit and a q

https://codereview.chromium.org/1539573002/diff/40001/mojo/edk/system/message...
File mojo/edk/system/message_pipe_dispatcher.cc (right):

https://codereview.chromium.org/1539573002/diff/40001/mojo/edk/system/message...
mojo/edk/system/message_pipe_dispatcher.cc:440: // a chance to execute. If that
happens, the RawChannel's will still call our
nit: "The RawChannel will" or "The RawChannel's ____ will"

https://codereview.chromium.org/1539573002/diff/40001/mojo/edk/system/message...
mojo/edk/system/message_pipe_dispatcher.cc:441: // OnError method because it
always runs (since it watches thread
q: Should this try to release in OnError?

[jam, 2015-12-17 18:54:36]

https://codereview.chromium.org/1539573002/diff/40001/mojo/edk/system/message...
File mojo/edk/system/message_pipe_dispatcher.cc (right):

https://codereview.chromium.org/1539573002/diff/40001/mojo/edk/system/message...
mojo/edk/system/message_pipe_dispatcher.cc:440: // a chance to execute. If that
happens, the RawChannel's will still call our
On 2015/12/17 18:49:59, msw wrote:
> nit: "The RawChannel will" or "The RawChannel's ____ will"

will fix in a followup, thanks

https://codereview.chromium.org/1539573002/diff/40001/mojo/edk/system/message...
mojo/edk/system/message_pipe_dispatcher.cc:441: // OnError method because it
always runs (since it watches thread
On 2015/12/17 18:49:59, msw wrote:
> q: Should this try to release in OnError?

(per in-person discussion)
We could add extra logic, although we'd have to handle the case when CloseOnIO
runs, and when OnError is called before either of these. I'm hesitant to add
even more state & complexity to avoid a shutdown leak, since in general shutdown
leaks are ok.

[jam, 2015-12-17 19:14:27]

The CQ bit was checked by jam@chromium.org

[commit-bot: I haz the power, 2015-12-17 19:17:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1539573002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1539573002/40001

[commit-bot: I haz the power, 2015-12-17 20:55:22]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-12-17 20:57:20]

Description was changed from

==========
Fix shutdown UAF in MessagePipeDispatcher in new Mojo EDK.

The problem was that the PostTask to the IO thread might not run if the IO
thread has shutdown. In that case the MessagePipeDispatcher would get
destructed, and RawChannel (called by IO thread destruction) can call its
OnError method. Fix this by manually refcounting so that in this situation we
just leak the MessagePipeDispatcher instead.

BUG=561803
==========

to

==========
Fix shutdown UAF in MessagePipeDispatcher in new Mojo EDK.

The problem was that the PostTask to the IO thread might not run if the IO
thread has shutdown. In that case the MessagePipeDispatcher would get
destructed, and RawChannel (called by IO thread destruction) can call its
OnError method. Fix this by manually refcounting so that in this situation we
just leak the MessagePipeDispatcher instead.

BUG=561803

Committed: https://crrev.com/d0a0ab7efa8f9e7eb2124468070e79c2786113a8
Cr-Commit-Position: refs/heads/master@{#365881}
==========

[commit-bot: I haz the power, 2015-12-17 20:57:21]

Patchset 3 (id:??) landed as
https://crrev.com/d0a0ab7efa8f9e7eb2124468070e79c2786113a8
Cr-Commit-Position: refs/heads/master@{#365881}