Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Workers: Allow ChromeWorkers access to XPCOM objects

RESOLVED FIXED

Status

()

Core
DOM
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Ben Turner (not reading bugmail, use the needinfo flag!), Assigned: Ben Turner (not reading bugmail, use the needinfo flag!))

Tracking

({dev-doc-complete})

Trunk
dev-doc-complete
Points:
---

Firefox Tracking Flags

(blocking2.0 beta9+)

Details

(Whiteboard: [hardblocker])

Attachments

(1 attachment, 1 obsolete attachment)

Created attachment 496975 [details] [diff] [review]
Patch, v1

From an email thread, here's our plan for allowing ChromeWorkers more access to XPCOM:

1. Allow postMessage to pass XPCWrappedNatives to ChromeWorkers only, provided that the underlying object is marked with nsIClassInfo.THREADSAFE.

2. Expose a new "XPCOM" global object to ChromeWorkers only. This object will have "createInstance" and "getService" functions, taking a single contract id string argument. When these functions are called we will create/get the object on the worker thread. We will then QI to nsIClassInfo. If the QI fails or if the object is not marked with nsIClassInfo.THREADSAFE we will abort. Otherwise we will wrap the object and define all interfaces indicated by nsIClassInfo::GetInterfaces before returning it. We will not expose "Components" or "Components.interfaces" etc., so QI will not be available.

Patch attached that implements this.

This originally sprung up in response to bug 608142.
Attachment #496975 - Flags: review?(mrbkap)
Attachment #496975 - Flags: review?(jorendorff)
I think we want this to block.
blocking2.0: --- → ?
Yup, this is effectively what will restore the ability to create XPCOM objects in JS off the main thread, which was possible (though unsafe) in 3.6.
blocking2.0: ? → beta9+
Comment on attachment 496975 [details] [diff] [review]
Patch, v1

r=me on the security manager changes.
Attachment #496975 - Flags: review?(mrbkap) → review+
Comment on attachment 496975 [details] [diff] [review]
Patch, v1

Johnny, can you review the dom/src/threads changes?
Attachment #496975 - Flags: review?(jst)
Comment on attachment 496975 [details] [diff] [review]
Patch, v1

Just as an API thing, I'd rather see a single extra argument everywhere of type JSStructuredCloneCallbacks *, rather than asking for the appropriate minimal subsets of the callbacks, in two or three arguments.

r=me for the js/src bits, with or without that change.
Attachment #496975 - Flags: review?(jorendorff) → review+
Created attachment 498544 [details] [diff] [review]
Patch, v2

Ok, this patch fixes two things.

First, we no longer abort on trying to create non-threadsafe components. Johnny had the brilliant idea that we could just sync-wait for the main thread to make and test the instance for threadsafety before actually doing it on the worker. We keep a cache of threadsafe contract IDs too.

Second, I've changed the structured clone API a little more so that we can pass a void* to the callbacks. This lets us avoid leaks that could happen if we're serializing wrapped natives and AddRef'ing as we go. If something failed later, or if the buffer gets deleted without deserializing, then we'd leak all the wrapped natives. This way we can grab a list of all the wrapped natives and only AddRef if everything succeeds. Jorendorff, you ok with these changes?
Attachment #496975 - Attachment is obsolete: true
Attachment #498544 - Flags: review?(jst)
Attachment #498544 - Flags: review?(jorendorff)
Attachment #496975 - Flags: review?(jst)
Comment on attachment 498544 [details] [diff] [review]
Patch, v2

- In WriteStructuredClone():

+      if (NS_SUCCEEDED(classInfo->GetFlags(&flags)) &&
+          (flags & nsIClassInfo::THREADSAFE)) {
+        // Write the raw pointer into the stream, and add it to the list we're

Looks like that comment ends mid-sentence.

r=jst
Attachment #498544 - Flags: review?(jst) → review+

Updated

7 years ago
Whiteboard: hardblocker
Comment on attachment 498544 [details] [diff] [review]
Patch, v2

>+JS_WriteStructuredClone(JSContext *cx, jsval v, uint64 **bufp, size_t *nbytesp,
>+                        const JSStructuredCloneCallbacks *optionalCallbacks,
>+                        void *closure)

Well she ain't gonna win any beauty contests, that's for sure. ;)

r=me on the new js/src bits.
Attachment #498544 - Flags: review?(jorendorff) → review+

Updated

7 years ago
Whiteboard: hardblocker → [hardblocker]
http://hg.mozilla.org/mozilla-central/rev/5e2045dddd67
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Summary: Allow ChromeWorkers access to XPCOM objects → Workers: Allow ChromeWorkers access to XPCOM objects
Keywords: dev-doc-needed
These revisions to the ChromeWorker documentation have been made. Please take a look and let me know if I've missed anything.

https://developer.mozilla.org/en/DOM/ChromeWorker
Keywords: dev-doc-needed → dev-doc-complete

Comment 11

7 years ago
Creating a thread is a bad example for something to send to a worker imo, since there really isn't anything useful you can do with it. Can we use something else in the example?
Yeah, a really good example would be doing disk I/O of a large file, so that it doesn't block the UI.
I'll see if I can put together a more useful example. I used the tests as a basis for the one that's currently there. If anyone has some code I can use, awesome. Otherwise this'll take a bit of time. :)
I'll try to collaborate with bz today to get some file/stream example code hashed out.
bent: if you do that, you two will be my heroes. Thanks!
This works fine with threadsafe xpcom objects (or objects marked as such to make it work).

What is still missing is a way to hand off an xpcom object to a different thread. It is a common task to prepare an object on a thread and then hand it off to where it came from. Any chance this will be possible in the 2.0 timeframe?
(In reply to comment #16)
> What is still missing is a way to hand off an xpcom object to a different
> thread. It is a common task to prepare an object on a thread and then hand it
> off to where it came from. Any chance this will be possible in the 2.0
> timeframe?

This is possible now (provided that the XPCOM object is marked threadsafe).
(In reply to comment #17)
> This is possible now (provided that the XPCOM object is marked threadsafe).
Thats exactly the point :-) The object I am working with is not marked threadsafe, and since its very complex it seems it cannot be marked threadsafe without some sort of modification (createInstance fails).

It would be good if I could create the non-threadsafe components in a worker so I can fill its info. Then I'd like to hand over the object and everything it accesses back to the main thread and end the worker.

Comment 19

7 years ago
What object is this?  In general, if an object can't be marked threadsafe, then accessing it on multiple threads is rather unsafe, no?
(In reply to comment #19)
> What object is this?  In general, if an object can't be marked threadsafe, then
> accessing it on multiple threads is rather unsafe, no?

Its our implementation of calIEvent. I may be mistaking, but threadsafe means it can be accessed by multiple threads at the same time, right? I only need access from one thread at a time and after the object is initialized and sent to the main thread, I don't want to access it from the worker again.

Maybe a different scenario, but what if a developer wants to make use of core xpcom components that are not threadsafe, to initialize a different component that is marked threadsafe?
Some objects are fine with at-most-one-thread-at-a-time; other code depends on being on the same thread, for TLS or JSThread or similar reasons.

The developer mooted in your second paragraph should initialize on the main thread and pass to the worker once it's done.

Comment 22

7 years ago
> but threadsafe means it can be accessed by multiple threads at the same time,
> right?

Yes, but perhaps not for all of the object's functionality.  There are "threadsafe" Gecko components that are not necessarily OK with being entered via the same API call on multiple threads at once...

Perhaps we need a better label for this sort of thing.

Comment 23

7 years ago
Are there still plans for file/stream example code? I wanted to give this a try today, but I gave up when I realized that nsIScriptableInputStream doesn't appear to be thread-safe.
You need to log in before you can comment on or make changes to this bug.