Closed Bug 1180686 Opened 9 years ago Closed 8 years ago

[e10s] we don't always run microtasks between chromium tasks

Categories

(Core :: IPC, defect)

Product:
Core
Component:
IPC
Version:
36 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED WONTFIX
Tracking Flags:
Tracking Status
e10s - ---

People

(Reporter: smaug, Unassigned)

References

Details

The current setup makes us miss rather important stuff which non-e10s builds do in nsXPConnect::AfterProcessNextEvent
Blocks: 1170484
I'm in the middle of reworking this in bug 1179909.
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #1)
> I'm in the middle of reworking this in bug 1179909.

Thanks for the heads-up!

Bill, you know the ipc message loop a lot better than me, what do you think? Where should we handle all the stuff AfterProcessNextEvent does? MessageChannel::OnMaybeDequeueOne seems like a reasonable place to me, but I don't know this code at all.

The story is that after each task we should perform a microtask checkpoint and execute all the pending microtasks, but we don't do that in MessageChannel.
Flags: needinfo?(wmccloskey)
I don't see why it would only be microtasks, IPC messages should behave like a task, IMO.
yes. The same callbacks should be called, whether there is IPC message or nsIRunnable being handled.
How urgent is this?  I could do this on top of my other refactorings sometime in the next month or so.
Not sure about urgency, but blocks e10s in some way.
We'll need to add a hook (OnMessageDispatchComplete or something) that runs on the top-level protocol after each message is processed. Then we can implement it in ContentParent/ContentChild and it can do whatever microtask stuff we need.
Flags: needinfo?(wmccloskey)
Actually, I'm a little confused here. I have only a vague understanding of the event loop, but I thought that a Chromium task (which is what the OnMaybeDequeueOne thing is) are run based on this code here:
https://dxr.mozilla.org/mozilla-central/source/ipc/glue/MessagePump.cpp#171
That gets scheduled using a normal nsITimer, which presumably goes through the event loop.

So it seems like we should run microtasks when all the pending Chromium tasks are done. It's just that we don't run microtasks in between Chromium tasks if there are multiple to run at a time. How important is it that we do that?
I looked at this a little closer and I think comment 8 is correct. Here's the stack for RefCountedTask::Run:

#8  0x00007f580da92bb1 in mozilla::ipc::MessageChannel::RefCountedTask::Run (this=0x7f58036506c0)
    at ../../dist/include/mozilla/ipc/MessageChannel.h:446
#9  0x00007f580da92d86 in mozilla::ipc::MessageChannel::DequeueTask::Run (this=0x7f57f0876e80)
    at ../../dist/include/mozilla/ipc/MessageChannel.h:463
#10 0x00007f580da1d677 in MessageLoop::RunTask (this=0x7ffff1e16390, task=0x7f57f0876e80)
    at /home/billm/moz/gecko/ipc/chromium/src/base/message_loop.cc:364
#11 0x00007f580da1d6ef in MessageLoop::DeferOrRunPendingTask (this=0x7ffff1e16390, 
    pending_task=...) at /home/billm/moz/gecko/ipc/chromium/src/base/message_loop.cc:372
#12 0x00007f580da1db4a in MessageLoop::DoWork (this=0x7ffff1e16390)
    at /home/billm/moz/gecko/ipc/chromium/src/base/message_loop.cc:459
#13 0x00007f580da8db4e in mozilla::ipc::DoWorkRunnable::Run (this=0x7f580365aa00)
    at /home/billm/moz/gecko/ipc/glue/MessagePump.cpp:220
#14 0x00007f580d55a9f5 in nsThread::ProcessNextEvent (this=0x7f58036c3400, aMayWait=false, 
    aResult=0x7ffff1e14def) at /home/billm/moz/gecko/xpcom/threads/nsThread.cpp:848
#15 0x00007f580d5b3e54 in NS_ProcessNextEvent (aThread=0x7f58036c3400, aMayWait=false)
    at /home/billm/moz/gecko/xpcom/glue/nsThreadUtils.cpp:265
#16 0x00007f580da8d4fa in mozilla::ipc::MessagePump::Run (this=0x7f5803651dd0, 
    aDelegate=0x7ffff1e16390) at /home/billm/moz/gecko/ipc/glue/MessagePump.cpp:95
#17 0x00007f580da8de66 in mozilla::ipc::MessagePumpForChildProcess::Run (this=0x7f5803651dd0, 
    aDelegate=0x7ffff1e16390) at /home/billm/moz/gecko/ipc/glue/MessagePump.cpp:289

I stepped through the nsThread::ProcessNextEvent and we do get to the AfterProcessNextEvent code and we run all the expected stuff there.

So I think the only problem is that we can run multiple messages in the same "Task".
I should have attached full stack :( doing it as promised to Olli earlier.

(In reply to Bill McCloskey (:billm) from comment #9) 
> I stepped through the nsThread::ProcessNextEvent and we do get to the
> AfterProcessNextEvent code and we run all the expected stuff there.
> 
> So I think the only problem is that we can run multiple messages in the same
> "Task".

I'm going to debug this some further to see if that is our problem. If it is
not sure how should we fix it though...

full stack:

#0  mozInlineSpellChecker::ReplaceWord (this=0x7fffd10cd500, 
    aNode=0x7fffcca6b8a0, aOffset=8, newword=...)
    at /home/gabor/development/mozilla-central/extensions/spellcheck/src/mozInlineSpellChecker.cpp:938
#1  0x00007fffeef16c93 in NS_InvokeByIndex (that=0x7fffd10cd500, 
    methodIndex=11, paramCount=3, params=0x7fffffff6d08)
    at /home/gabor/development/mozilla-central/xpcom/reflect/xptcall/md/unix/xptcinvoke_x86_64_unix.cpp:176
#2  0x00007fffefb888a7 in CallMethodHelper::Invoke (this=0x7fffffff6cc0)
    at /home/gabor/development/mozilla-central/js/xpconnect/src/XPCWrappedNative.cpp:2080
#3  0x00007fffefb865c3 in CallMethodHelper::Call (this=0x7fffffff6cc0)
    at /home/gabor/development/mozilla-central/js/xpconnect/src/XPCWrappedNative.cpp:1417
#4  0x00007fffefb6b860 in XPCWrappedNative::CallMethod (ccx=..., 
    mode=XPCWrappedNative::CALL_METHOD)
    at /home/gabor/development/mozilla-central/js/xpconnect/src/XPCWrappedNative.cpp:1384
#5  0x00007fffefb74c35 in XPC_WN_CallMethod (cx=0x7fffe4f95840, argc=3, 
    vp=0x7fffdd2d1118)
    at /home/gabor/development/mozilla-central/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1144
#6  0x00007ffff3c95cc5 in js::CallJSNative (cx=0x7fffe4f95840, 
---Type <return> to continue, or q <return> to quit---
    native=0x7fffefb748d5 <XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*)>, args=...)
    at /home/gabor/development/mozilla-central/js/src/jscntxtinlines.h:235
#7  0x00007ffff3c658b2 in js::Invoke (cx=0x7fffe4f95840, args=..., 
    construct=js::NO_CONSTRUCT)
    at /home/gabor/development/mozilla-central/js/src/vm/Interpreter.cpp:711
#8  0x00007ffff3c74ae3 in Interpret (cx=0x7fffe4f95840, state=...)
    at /home/gabor/development/mozilla-central/js/src/vm/Interpreter.cpp:2959
#9  0x00007ffff3c654df in js::RunScript (cx=0x7fffe4f95840, state=...)
    at /home/gabor/development/mozilla-central/js/src/vm/Interpreter.cpp:655
#10 0x00007ffff3c659f3 in js::Invoke (cx=0x7fffe4f95840, args=..., 
    construct=js::NO_CONSTRUCT)
    at /home/gabor/development/mozilla-central/js/src/vm/Interpreter.cpp:731
#11 0x00007ffff3c65dc3 in js::Invoke (cx=0x7fffe4f95840, thisv=..., fval=..., 
    argc=1, argv=0x7fffdd2d10a0, rval=...)
    at /home/gabor/development/mozilla-central/js/src/vm/Interpreter.cpp:768
#12 0x00007ffff42f2d35 in js::DirectProxyHandler::call (
    this=0x7ffff7803a40 <js::CrossCompartmentWrapper::singleton>, 
    cx=0x7fffe4f95840, proxy=..., args=...)
    at /home/gabor/development/mozilla-central/js/src/proxy/DirectProxyHandler.cpp:77
#13 0x00007ffff42f03b9 in js::CrossCompartmentWrapper::call (
    this=0x7ffff7803a40 <js::CrossCompartmentWrapper::singleton>, 
---Type <return> to continue, or q <return> to quit---
    cx=0x7fffe4f95840, wrapper=..., args=...)
    at /home/gabor/development/mozilla-central/js/src/proxy/CrossCompartmentWrapper.cpp:289
#14 0x00007ffff42f6547 in js::Proxy::call (cx=0x7fffe4f95840, proxy=..., 
    args=...)
    at /home/gabor/development/mozilla-central/js/src/proxy/Proxy.cpp:391
#15 0x00007ffff42f79bc in js::proxy_Call (cx=0x7fffe4f95840, argc=1, 
    vp=0x7fffdd2d1090)
    at /home/gabor/development/mozilla-central/js/src/proxy/Proxy.cpp:697
#16 0x00007ffff3c95cc5 in js::CallJSNative (cx=0x7fffe4f95840, 
    native=0x7ffff42f78da <js::proxy_Call(JSContext*, unsigned int, JS::Value*)>, args=...)
    at /home/gabor/development/mozilla-central/js/src/jscntxtinlines.h:235
#17 0x00007ffff3c657b0 in js::Invoke (cx=0x7fffe4f95840, args=..., 
    construct=js::NO_CONSTRUCT)
    at /home/gabor/development/mozilla-central/js/src/vm/Interpreter.cpp:699
#18 0x00007ffff3c74ae3 in Interpret (cx=0x7fffe4f95840, state=...)
    at /home/gabor/development/mozilla-central/js/src/vm/Interpreter.cpp:2959
#19 0x00007ffff3c654df in js::RunScript (cx=0x7fffe4f95840, state=...)
    at /home/gabor/development/mozilla-central/js/src/vm/Interpreter.cpp:655
#20 0x00007ffff3c659f3 in js::Invoke (cx=0x7fffe4f95840, args=..., 
    construct=js::NO_CONSTRUCT)
    at /home/gabor/development/mozilla-central/js/src/vm/Interpreter.cpp:731
---Type <return> to continue, or q <return> to quit---
#21 0x00007ffff3c65dc3 in js::Invoke (cx=0x7fffe4f95840, thisv=..., fval=..., 
    argc=1, argv=0x7fffffffb028, rval=...)
    at /home/gabor/development/mozilla-central/js/src/vm/Interpreter.cpp:768
#22 0x00007ffff42f2d35 in js::DirectProxyHandler::call (
    this=0x7ffff7803a40 <js::CrossCompartmentWrapper::singleton>, 
    cx=0x7fffe4f95840, proxy=..., args=...)
    at /home/gabor/development/mozilla-central/js/src/proxy/DirectProxyHandler.cpp:77
#23 0x00007ffff42f03b9 in js::CrossCompartmentWrapper::call (
    this=0x7ffff7803a40 <js::CrossCompartmentWrapper::singleton>, 
    cx=0x7fffe4f95840, wrapper=..., args=...)
    at /home/gabor/development/mozilla-central/js/src/proxy/CrossCompartmentWrapper.cpp:289
#24 0x00007ffff42f6547 in js::Proxy::call (cx=0x7fffe4f95840, proxy=..., 
    args=...)
    at /home/gabor/development/mozilla-central/js/src/proxy/Proxy.cpp:391
#25 0x00007ffff42f79bc in js::proxy_Call (cx=0x7fffe4f95840, argc=1, 
    vp=0x7fffffffb018)
    at /home/gabor/development/mozilla-central/js/src/proxy/Proxy.cpp:697
#26 0x00007ffff3c95cc5 in js::CallJSNative (cx=0x7fffe4f95840, 
    native=0x7ffff42f78da <js::proxy_Call(JSContext*, unsigned int, JS::Value*)>, args=...)
    at /home/gabor/development/mozilla-central/js/src/jscntxtinlines.h:235
---Type <return> to continue, or q <return> to quit---
#27 0x00007ffff3c657b0 in js::Invoke (cx=0x7fffe4f95840, args=..., 
    construct=js::NO_CONSTRUCT)
    at /home/gabor/development/mozilla-central/js/src/vm/Interpreter.cpp:699
#28 0x00007ffff3c65dc3 in js::Invoke (cx=0x7fffe4f95840, thisv=..., fval=..., 
    argc=1, argv=0x7fffffffb4f0, rval=...)
    at /home/gabor/development/mozilla-central/js/src/vm/Interpreter.cpp:768

#29 0x00007ffff41d28fb in JS_CallFunctionValue (cx=0x7fffe4f95840, obj=..., 
    fval=..., args=..., rval=...)
    at /home/gabor/development/mozilla-central/js/src/jsapi.cpp:4567
#30 0x00007ffff0245b9e in nsFrameMessageManager::ReceiveMessage (

    this=0x7fffdcefd870, aTarget=0x7fffdcfe3b00, aTargetFrameLoader=0x0, 
    aTargetClosed=false, aMessage=..., aIsSync=false, 
    aCloneData=0x7fffffffb770, aCpows=0x7fffffffb790, aPrincipal=0x0, 
    aRetVal=0x0)
    at /home/gabor/development/mozilla-central/dom/base/nsFrameMessageManager.cpp:1250
#31 0x00007ffff0244433 in nsFrameMessageManager::ReceiveMessage (
    this=0x7fffdcefd870, aTarget=0x7fffdcfe3b00, aTargetFrameLoader=0x0, 
    aMessage=..., aIsSync=false, aCloneData=0x7fffffffb770, 
    aCpows=0x7fffffffb790, aPrincipal=0x0, aRetVal=0x0)
    at /home/gabor/development/mozilla-central/dom/base/nsFrameMessageManager.cpp:1067
#32 0x00007ffff1a82ed0 in mozilla::dom::TabChild::RecvAsyncMessage(nsString cons---Type <return> to continue, or q <return> to quit---
t&, mozilla::dom::ClonedMessageData const&, nsTArray<mozilla::jsipc::CpowEntry>&&, IPC::Principal const&) (this=0x7fffd732c400, aMessage=..., aData=..., 
    aCpows=<unknown type in /home/gabor/development/mozilla-central/obj-x86_64-unknown-linux-gnu/dist/bin/libxul.so, CU 0xb64486d, DIE 0xb792242>, 
    aPrincipal=...)
    at /home/gabor/development/mozilla-central/dom/ipc/TabChild.cpp:2741
#33 0x00007fffef82bc85 in mozilla::dom::PBrowserChild::OnMessageReceived (
    this=0x7fffd732c5a0, msg__=...)
    at /home/gabor/development/mozilla-central/obj-x86_64-unknown-linux-gnu/ipc/ipdl/PBrowserChild.cpp:2512
#34 0x00007fffef90e797 in mozilla::dom::PContentChild::OnMessageReceived (
    this=0x7fffe4f1d030, msg__=...)
    at /home/gabor/development/mozilla-central/obj-x86_64-unknown-linux-gnu/ipc/ipdl/PContentChild.cpp:5435
#35 0x00007fffef41e23b in mozilla::ipc::MessageChannel::DispatchAsyncMessage (
    this=0x7fffe4f1d098, aMsg=...)
    at /home/gabor/development/mozilla-central/ipc/glue/MessageChannel.cpp:1279
#36 0x00007fffef41dce7 in mozilla::ipc::MessageChannel::DispatchMessage (
    this=0x7fffe4f1d098, aMsg=...)
    at /home/gabor/development/mozilla-central/ipc/glue/MessageChannel.cpp:1198
#37 0x00007fffef41dbde in mozilla::ipc::MessageChannel::OnMaybeDequeueOne (
    this=0x7fffe4f1d098)
    at /home/gabor/development/mozilla-central/ipc/glue/MessageChannel.cpp:1182
---Type <return> to continue, or q <return> to quit---
#38 0x00007fffef4333be in DispatchToMethod<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)()> (obj=0x7fffe4f1d098, 
    method=(bool (mozilla::ipc::MessageChannel::*)(mozilla::ipc::MessageChannel * const)) 0x7fffef41da00 <mozilla::ipc::MessageChannel::OnMaybeDequeueOne()>, 
    arg=...)
    at /home/gabor/development/mozilla-central/ipc/chromium/src/base/tuple.h:387
#39 0x00007fffef432e3e in RunnableMethod<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)(), Tuple0>::Run (this=0x7fffe4f07840)
    at /home/gabor/development/mozilla-central/ipc/chromium/src/base/task.h:310
#40 0x00007fffef42698b in mozilla::ipc::MessageChannel::RefCountedTask::Run (
    this=0x7fffe4f506c0) at ../../dist/include/mozilla/ipc/MessageChannel.h:446
#41 0x00007fffef426b60 in mozilla::ipc::MessageChannel::DequeueTask::Run (
    this=0x7fffcb3b2f40) at ../../dist/include/mozilla/ipc/MessageChannel.h:463
#42 0x00007fffef3b262d in MessageLoop::RunTask (this=0x7fffffffd720, 
    task=0x7fffcb3b2f40)
    at /home/gabor/development/mozilla-central/ipc/chromium/src/base/message_loop.cc:364
#43 0x00007fffef3b26a5 in MessageLoop::DeferOrRunPendingTask (
    this=0x7fffffffd720, pending_task=...)
    at /home/gabor/development/mozilla-central/ipc/chromium/src/base/message_loop.cc:372
#44 0x00007fffef3b2b00 in MessageLoop::DoWork (this=0x7fffffffd720)
---Type <return> to continue, or q <return> to quit---
    at /home/gabor/development/mozilla-central/ipc/chromium/src/base/message_loop.cc:459
#45 0x00007fffef421976 in mozilla::ipc::DoWorkRunnable::Run (
    this=0x7fffe4f5aa00)
    at /home/gabor/development/mozilla-central/ipc/glue/MessagePump.cpp:220
#46 0x00007fffeeef8cd7 in nsThread::ProcessNextEvent (this=0x7fffe4fc94f0, 
    aMayWait=true, aResult=0x7fffffffc17f)
    at /home/gabor/development/mozilla-central/xpcom/threads/nsThread.cpp:848
#47 0x00007fffeef50f74 in NS_ProcessNextEvent (aThread=0x7fffe4fc94f0, 
    aMayWait=true)
    at /home/gabor/development/mozilla-central/xpcom/glue/nsThreadUtils.cpp:265
#48 0x00007fffef421416 in mozilla::ipc::MessagePump::Run (this=0x7fffe4f51dd0, 
    aDelegate=0x7fffffffd720)
    at /home/gabor/development/mozilla-central/ipc/glue/MessagePump.cpp:127
#49 0x00007fffef421c8e in mozilla::ipc::MessagePumpForChildProcess::Run (
    this=0x7fffe4f51dd0, aDelegate=0x7fffffffd720)
    at /home/gabor/development/mozilla-central/ipc/glue/MessagePump.cpp:289
#50 0x00007fffef3b2105 in MessageLoop::RunInternal (this=0x7fffffffd720)
    at /home/gabor/development/mozilla-central/ipc/chromium/src/base/message_loop.cc:234
#51 0x00007fffef3b209a in MessageLoop::RunHandler (this=0x7fffffffd720)
    at /home/gabor/development/mozilla-central/ipc/chromium/src/base/message_loop.cc:227
---Type <return> to continue, or q <return> to quit---
#52 0x00007fffef3b202b in MessageLoop::Run (this=0x7fffffffd720)
    at /home/gabor/development/mozilla-central/ipc/chromium/src/base/message_loop.cc:201
#53 0x00007ffff1d93646 in nsBaseAppShell::Run (this=0x7fffdd1cd200)
    at /home/gabor/development/mozilla-central/widget/nsBaseAppShell.cpp:165
#54 0x00007ffff2c29dc6 in XRE_RunAppShell ()
    at /home/gabor/development/mozilla-central/toolkit/xre/nsEmbedFunctions.cpp:778
#55 0x00007fffef421b04 in mozilla::ipc::MessagePumpForChildProcess::Run (
    this=0x7fffe4f51dd0, aDelegate=0x7fffffffd720)
    at /home/gabor/development/mozilla-central/ipc/glue/MessagePump.cpp:259
#56 0x00007fffef3b2105 in MessageLoop::RunInternal (this=0x7fffffffd720)
    at /home/gabor/development/mozilla-central/ipc/chromium/src/base/message_loop.cc:234
#57 0x00007fffef3b209a in MessageLoop::RunHandler (this=0x7fffffffd720)
    at /home/gabor/development/mozilla-central/ipc/chromium/src/base/message_loop.cc:227
#58 0x00007fffef3b202b in MessageLoop::Run (this=0x7fffffffd720)
    at /home/gabor/development/mozilla-central/ipc/chromium/src/base/message_loop.cc:201
#59 0x00007ffff2c297d4 in XRE_InitChildProcess (aArgc=3, aArgv=0x7fffffffda58, 
    aGMPLoader=0x0)
    at /home/gabor/development/mozilla-central/toolkit/xre/nsEmbedFunctions.cpp:---Type <return> to continue, or q <return> to quit---
614
#60 0x000000000041f377 in content_process_main (argc=5, argv=0x7fffffffda58)
    at /home/gabor/development/mozilla-central/ipc/app/../contentproc/plugin-container.cpp:236
#61 0x000000000041f440 in main (argc=6, argv=0x7fffffffda58)
    at /home/gabor/development/mozilla-central/ipc/app/MozillaRuntimeMain.cpp:11
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #5)
> How urgent is this?  I could do this on top of my other refactorings
> sometime in the next month or so.

It should be done in Q3. As soon as possible is always good of course, but next month sounds great. Do you think your patch will address what Bill described in Comment 8? Or that's something should be done on top of your patch?
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #11)
> (In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #5)
> > How urgent is this?  I could do this on top of my other refactorings
> > sometime in the next month or so.
> 
> It should be done in Q3. As soon as possible is always good of course, but
> next month sounds great. Do you think your patch will address what Bill
> described in Comment 8? Or that's something should be done on top of your
> patch?

That's something that should be done on top of my patch.  It should be pretty trivial to do.  I'm not really sure why we wouldn't consider each chromium message a separate top-level task though.
Depends on: 1179909
Gabor, is this an issue we should look at?
Flags: needinfo?(gkrizsanits)
(In reply to Jim Mathies [:jimm] from comment #13)
> Gabor, is this an issue we should look at?

Until we find an actual case where this causes a problem we should not worry too much about it.
Flags: needinfo?(gkrizsanits)
Hmm, I had thought this was fixed already in some other bug, but apparently no. Our IPC event loop is still totally crazy inconsistent with the rest of the world.
We can't ship e10s with this kind of issue unfixed. It could break all sorts of stuff, MutationObservers and Promises etc.
(In reply to Olli Pettay [:smaug] from comment #15)
> Hmm, I had thought this was fixed already in some other bug, but apparently
> no. Our IPC event loop is still totally crazy inconsistent with the rest of
> the world.
> We can't ship e10s with this kind of issue unfixed. It could break all sorts
> of stuff, MutationObservers and Promises etc.

I felt that way too initially, but have not yet seen any bugs related to this. Bobby and Bill did not seem to think this is a huge issue so I kind of gave it up. Do you have any actual reproducible issue in mind why this might be a blocker?
We've had security bugs and what not when not calling Before/AfterProcessNextTask properly.
Like, do we end calling PushNullJSContext() and PopNullJSContext() properly when handling IPC tasks?
Those used to be super ciritical to fix several security issues, but maybe aren't anymore?

I don't have a reproduceable test.
Priority: -- → P4
ni'ing billm since this is an ipc issue. we'll need to decide if this blocks rollout.
tracking-e10s: + → ?
Flags: needinfo?(wmccloskey)
Priority: P4 → --
Or perhaps khuey tells us that this is already fixed in some other bug :)
Flags: needinfo?(khuey)
(In reply to Olli Pettay [:smaug] from comment #17)
> We've had security bugs and what not when not calling
> Before/AfterProcessNextTask properly.
> Like, do we end calling PushNullJSContext() and PopNullJSContext() properly
> when handling IPC tasks?
> Those used to be super ciritical to fix several security issues, but maybe
> aren't anymore?
> 
> I don't have a reproduceable test.

I don't think that is the issue here, see comment 9 from Bill. The only problem is that we run microtasks only after all the pending chromium tasks are handled.
Summary: [e10s] Once MessageChannel::RefCountedTask::Run has been executed, something similar to AfterProcessNextEvent should be called. → [e10s] we don't always run microtasks between chromim tasks
Sorry for the typo.
Summary: [e10s] we don't always run microtasks between chromim tasks → [e10s] we don't always run microtasks between chromium tasks
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #16)
> (In reply to Olli Pettay [:smaug] from comment #15)
> > Hmm, I had thought this was fixed already in some other bug, but apparently
> > no. Our IPC event loop is still totally crazy inconsistent with the rest of
> > the world.
> > We can't ship e10s with this kind of issue unfixed. It could break all sorts
> > of stuff, MutationObservers and Promises etc.
> 
> I felt that way too initially, but have not yet seen any bugs related to
> this. Bobby and Bill did not seem to think this is a huge issue so I kind of
> gave it up.

My recollection is that we thought this was going to be fixed by some microtask cleanup khuey was doing.
I don't think this is a problem at all. Each IPC message is processed in its own separate Gecko event. As long as we do the right thing for Gecko events, we'll do the right thing for IPC messages.
Flags: needinfo?(wmccloskey)
if that is the case these days (it didn't use to be), great.
I don't see a reason to keep this open until there is an actual bug that is caused by this slight difference. The original reported case is invalid, the actual difference is a won't fix, I close the bug with the later.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
tracking-e10s: ? → -
Flags: needinfo?(khuey)
I do want to know where we fixed this.
Flags: needinfo?(khuey)
I don't think that we have any evidence that this bug ever existed, so there's no fix. This was filed based off of bug 1170484. I have to admit that I don't totally understand that bug, but it looks like the bug turned out to be in editor code. Initially we hypothesized that it had something to do with microtransactions, but I don't think that turned out to be the case.
Turns out we only ever run one Chromium Task at a time, so we shouldn't be able to get microtasks to spill over between Tasks.  Whether or not microtasks can spill from a Task into an nsIRunnable that follows it is still an open question.  Note that the logic at http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/src/XPCJSRuntime.cpp#3677 will trigger here, so that will run microtasks under some conditions.  I don't know that we have anything that runs them unconditionally ...
Flags: needinfo?(khuey)
You need to log in before you can comment on or make changes to this bug.