1361607 - Fail to resolve MozPromise generated by IPC in non browser/content processes

Status: NEW

(Core :: IPC, defect, P3)

Description

[Louis Chang [:louis]]

Attachment: Test IPC MozPromise

In my case, it will hit |MOZ_ASSERT(CycleCollectedJSContext::Get(), "Must be on a script thread!")| in nsContentUtils:RunInStableState when resolving the MozPromise generated by IPC.

The attachment is my test patch which will hit the assertion. My patch is based on the unlanded mortar architecture [1][2]. PPPAPIJSPlugin is top level IPC protocol between chrome and plugin process.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1344942
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=558184

Comment 1

[Kan-Ru Chen [:kanru] (UTC+9)]

Thanks for filing this, Louis!

So the problem here is that the AbstractThread[1] wrapper for nsIEventTarget uses nsContentUtils::RunInStableState to implement tail dispatch. nsContentUtils::RunInStableState requires XPConnect being initialized, that means currently we can't use MozPromise in GMP, Plugin or GPU process main thread.

The fact that the AbstractThread wrapper for MessageLoop doesn't support tail dispatch is also not ideal. Maybe we should implement tail dispatch for all AbstractThreads that hides the dependency of XPCOM or XPConnect.

Thoughts?

[1]: http://searchfox.org/mozilla-central/rev/ae8c2e2354db652950fe0ec16983360c21857f2a/xpcom/threads/AbstractThread.cpp#105

Comment 2

[JW Wang [:jwwang] [:jw_wang]]

TaskQueue is inherently capable of tail dispatching and it also implements the interface of AbstractThread.

http://searchfox.org/mozilla-central/rev/abe68d5dad139e376d5521ca1d4b7892e1e7f1ba/xpcom/threads/TaskQueue.cpp#64
TaskQueue use an nsIEventTarget for its underlying task dispatching. We might extend it to allow a MessageLoop for its underlying delegation.

Comment 3

[Kan-Ru Chen [:kanru] (UTC+9)]

(In reply to JW Wang [:jwwang] [:jw_wang] from comment #2)
> TaskQueue is inherently capable of tail dispatching and it also implements
> the interface of AbstractThread.
> 
> http://searchfox.org/mozilla-central/rev/
> abe68d5dad139e376d5521ca1d4b7892e1e7f1ba/xpcom/threads/TaskQueue.cpp#64
> TaskQueue use an nsIEventTarget for its underlying task dispatching. We
> might extend it to allow a MessageLoop for its underlying delegation.

Looks like switching the MessageLoop AbstractThread wrapper to a TaskQueue can solve the second problem. For the first problem, can we simplify the setup and have the default MainThread wrapper to also use a TaskQueue? I'm not so sure about the semantic difference between nsContentUtils::RunInStableState and TaskQueue::AutoTaskGuard.

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

(In reply to Kan-Ru Chen [:kanru] (UTC+8) from comment #1)
> So the problem here is that the AbstractThread[1] wrapper for nsIEventTarget
> uses nsContentUtils::RunInStableState to implement tail dispatch.
> nsContentUtils::RunInStableState requires XPConnect being initialized, that
> means currently we can't use MozPromise in GMP, Plugin or GPU process main
> thread.

Why is tail dispatching required for using MozPromise?

Comment 5

[Kan-Ru Chen [:kanru] (UTC+9)]

(In reply to JW Wang [:jwwang] [:jw_wang] from comment #4)
> (In reply to Kan-Ru Chen [:kanru] (UTC+8) from comment #1)
> > So the problem here is that the AbstractThread[1] wrapper for nsIEventTarget
> > uses nsContentUtils::RunInStableState to implement tail dispatch.
> > nsContentUtils::RunInStableState requires XPConnect being initialized, that
> > means currently we can't use MozPromise in GMP, Plugin or GPU process main
> > thread.
> 
> Why is tail dispatching required for using MozPromise?

It's not required but the AbstractThread wrapper of main thread requires tail dispatching and it's implemented by using nsContentUtils::RunInStableState.

Maybe IPC can use TaskQueue wrapped main thread internally but it seems not practical to ask IPC users to create/maintain a TaskQueue instead of using AbstractThread::GetCurrent() or AbstractThread::MainThread(). Well, maybe it's OK for GMP/Plugin/GPU processes.