New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 695593 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Last visit > 30 days ago
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

mojo::ReportBadMessage crashes the browser if there's a return callback that's not invoked

Project Member Reported by dcheng@chromium.org, Feb 23 2017

Issue description

In my opinion, we shouldn't have to invoke the return callback on a bad message (since we'd have to make up bogus values to pass back). However, Mojo currently DCHECKs that the return callback is always invoked.

Crash stack in Mojo: https://paste.googleplex.com/5362279369408512?raw

Original CL that prompted this investigation: https://codereview.chromium.org/2711173002/
 

Comment 1 by yzshen@chromium.org, Feb 23 2017

Status: Started (was: Assigned)

Comment 2 by roc...@chromium.org, Feb 23 2017

Note that we don't DCHECK if the Binding responsible for a dropped callback
has been closed first. The correct thing to do when reporting a bad message
is to either invoke the callback anyway or Close the corresponding Binding.
I'm not sure any of this behavior is incorrect, particularly since
ReportBadMessage does not in all scenarios imply that you're going to get
shot or your pipe is going to be broken.

Comment 3 by dcheng@chromium.org, Feb 23 2017

If the right behavior here is to close the binding, I'm OK with WontFixing this.
Closing the binding sounds like a reasonable solution to me, thanks!

Comment 5 by yzshen@chromium.org, Feb 23 2017

Thanks Ken!

Another possible solution is to change the DCHECK to a DVLOG message. But that would be harder for the developers to find out why when they accidentally drop a callback (which lead to pipe disconnection).

So I lean towards keeping the current behavior and suggest the developers to adopt either of the solutions mentioned by Ken.

WDYT?

Comment 6 by yzshen@chromium.org, Feb 23 2017

Status: WontFix (was: Started)
(Oops, didn't see comment #3 and #4 before I replied.)

Sign in to add a comment