New issue
Advanced search Search tips

Issue 746543 link

Starred by 2 users
Status: Fixed
Owner:
mek@chromium.org
Closed: Jul 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug
Proj-Servicification

Blocking:
issue 611935



Sign in to add a comment

[mojo-blobs] Fix passing of blink::BlobDataHandle across threads.

Project Member Reported by mek@chromium.org, Jul 19 2017 
Passing BlobDataHandle across thread was never really safe (because of String members), although due to liberal use of IsolatedCopy and (accidentally) creating new BlobDataHandles when passing them across threads this never really was an issue in practice. With mojo blobs however this will be an issue as passing a mojom::BlobPtr across threads isn't going to work correctly.

We should probably fix this by making BlobDataHandle (non-threadsafe) RefCounted, and adding an explicit CopyData method to extract the data in a form that can be passed across threads.
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 25 2017 

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

commit ea14eb6306dd3986ffbe5a9ddbc9cdb28e16d8f2
Author: Marijn Kruisselbrink <mek@chromium.org>
Date: Tue Jul 25 05:31:43 2017

Use BlobPtrInfo with a Mutex to make BlobDataHandle threadsafe again.

BlobDataHandle can be used from multiple threads concurrently, but mojo
InterfacePtrs are not thread safe. To work around this change it to
use InterfacePtrInfo the BlobDataHandle instead, only temporarily binding
it to a InterfacePtr (protected by a mutex) whenever we need to do
something with the mojo interface.

Bug:  746543 
Change-Id: I0778c02c710de6c02af9c738f65618f13633785b
Reviewed-on: https://chromium-review.googlesource.com/580150
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489238}
[modify] https://crrev.com/ea14eb6306dd3986ffbe5a9ddbc9cdb28e16d8f2/third_party/WebKit/Source/platform/blob/BlobData.cpp
[modify] https://crrev.com/ea14eb6306dd3986ffbe5a9ddbc9cdb28e16d8f2/third_party/WebKit/Source/platform/blob/BlobData.h

Comment 2 by mek@chromium.org, Jul 27 2017

Status: Fixed (was: Started)

Comment 3 by laforge@google.com, Nov 7 2017

Components: Internals>Network>Service

Comment 4 by laforge@google.com, Nov 7 2017

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.

Comment 5 by laforge@google.com, Nov 8 2017

Components: Internals>Services>Storage
Setting Internals>Services>Storage to all children of issue 611935
►

Sign in to add a comment