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

Issue 864731 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
please use my google.com address
Closed: Sep 19
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Mojo: Associated interfaces can synchronously invoke error handlers

Project Member Reported by roc...@chromium.org, Jul 17

Issue description

When their local master pipe endpoint is closed, associated interface endpoints are notified synchronously of a connection error.

This is problematic because it's extremely subtle. As with bug 854993, it can easily lead to an error handler being invoked while its bound object is partially destructed. The only way to prevent this would be to explicitly close the associated bindings before closing the master binding.

Because this is so subtle, and because the behavior as-is does not seem to be necessary for any practical reasons, we should change it: if a master endpoint is closed, its local associated endpoints can receive a connection error asynchronously (or not at all of course, if they happen to be destroyed before their async handler can run).
 
Description: Show this description
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dc4eddf6787185047877ddecf2577fe4af0a4d0d

commit dc4eddf6787185047877ddecf2577fe4af0a4d0d
Author: Ken Rockot <rockot@chromium.org>
Date: Thu Jul 19 00:56:32 2018

[mojo-public] Async dispatch for associated errors

Associated endpoints have been getting error handler invocations
synchronously whenever their corresponding master interface endpoint is
closed. This is bad for reasons explained in the linked bug. Fix!

Bug:  864731 
Change-Id: If4a7d205ab42be5ee43926be619e77ad0ab15d68
Reviewed-on: https://chromium-review.googlesource.com/1141078
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576288}
[modify] https://crrev.com/dc4eddf6787185047877ddecf2577fe4af0a4d0d/content/browser/dom_storage/local_storage_context_mojo_unittest.cc
[modify] https://crrev.com/dc4eddf6787185047877ddecf2577fe4af0a4d0d/content/renderer/dom_storage/local_storage_cached_areas_unittest.cc
[modify] https://crrev.com/dc4eddf6787185047877ddecf2577fe4af0a4d0d/mojo/public/cpp/bindings/lib/multiplex_router.cc
[modify] https://crrev.com/dc4eddf6787185047877ddecf2577fe4af0a4d0d/mojo/public/cpp/bindings/lib/multiplex_router.h
[modify] https://crrev.com/dc4eddf6787185047877ddecf2577fe4af0a4d0d/mojo/public/cpp/bindings/tests/associated_interface_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment