552014 - IPDL: The OnMaybeDequeueOne task shouldn't run in a nested (wrt RPC) context

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Otherwise we can dequeue tasks at inopportune times.  For example:

  A                      B
 ------------           -----------
  spin                   send async A()
  [enqueue Dequeue()]
  CallFoo()
  [wait]
  dequeue A
  [wait]                 send async B()
  dequeue B              reply to Foo()
  do stuff
  spin nested loop
  Dequeue() runs
  Incall(replyToFoo)
  abort()

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I should add that by "nested (wrt RPC)" I meant that it's actually OK (wrt RPC) if this tasks runs in a nested context, there just can't be other RPC code on the stack when it runs.

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Test

Yay?

 (sleeping to wait for nonce ... sorry)
 (sleeping to wait for reply to R ... sorry)
###!!! [RPCChannel][Parent][/home/cjones/mozilla/mozilla-central/ipc/glue/RPCChannel.cpp:410] Assertion (call.is_rpc() && !call.is_reply()) failed.  wrong message type (triggered by rpc)
  RPCChannel 'backtrace':
  [(0) out async PTestNestedLoops::Msg_R(actor=32545) ]
  [(1) in async PTestNestedLoops::Msg_Nonce(actor=2147483647) ]
  [(2) in rpc ???(actor=2147483647) ]
  remote RPC stack guess: 0
  deferred stack size: 0
  out-of-turn RPC replies stack size: 0
  Pending queue size: 0, front to back:
###!!! ABORT: wrong message type: file /home/cjones/mozilla/mozilla-central/ipc/glue/RPCChannel.cpp, line 625
Trace/breakpoint trap

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Don't run OnMaybeDequeueOne from a nested context

(sleeping to wait for nonce ... sorry)
 (sleeping to wait for reply to R ... sorry)
NOTE: child process received `Goodbye', closing down
TEST-PASS | TestNestedLoops | ok

Also passes ipdl/cxx and mochitest-ipcplugins.

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 432235 [details] [diff] [review]
Don't run OnMaybeDequeueOne from a nested context

>+        // bail here, but ensure that each at exit point

"at each exit point"

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to comment #2)
>   RPCChannel 'backtrace':
>   [(0) out async PTestNestedLoops::Msg_R(actor=32545) ]
>   [(1) in async PTestNestedLoops::Msg_Nonce(actor=2147483647) ]
>   [(2) in rpc ???(actor=2147483647) ]

Oh crap ... this is the wrong stack.  I think I know what the problem is, will try to fix in bug 533055.

Comment 6

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Test

Now with hg add.

Comment 7

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

http://hg.mozilla.org/mozilla-central/rev/7828c0055b2d
http://hg.mozilla.org/mozilla-central/rev/29f2eed187b7

Comment 8

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Jim reports that this majorly regressed video perf on windows because we stop processing messages in nested event loops.

Comment 9

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Treat RPC replies received in an event loop nested inside the one from which the call was made as if they had arrived out of order

Still passes the test, so this should be OK.

Comment 10

[Jim Mathies [:jimm]]

Comment on attachment 433507 [details] [diff] [review]
Treat RPC replies received in an event loop nested inside the one from which the call was made as if they had arrived out of order

http://hg.mozilla.org/mozilla-central/rev/a135d2f38e1c

Comment 11

[Benjamin Smedberg]

http://hg.mozilla.org/projects/firefox-lorentz/rev/d3625fa8b843
http://hg.mozilla.org/projects/firefox-lorentz/rev/42a28a2c2859
http://hg.mozilla.org/projects/firefox-lorentz/rev/8c0cdfdaa1bb

Comment 12

[Mike Beltzner [:beltzner, not reading bugmail]]

Blanket approval for Lorentz merge to mozilla-1.9.2
a=beltzner for 1.9.2.4 - please make sure to mark status1.9.2:.4-fixed

Comment 13

[Benjamin Smedberg]

Merged into 1.9.2 at http://hg.mozilla.org/releases/mozilla-1.9.2/rev/84ba4d805430