554466 - RPCChannel CxxStack helpers can enqueue tons of messages

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

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

Attachment: Patch, v1

cjones could explain better, but basically we are over-enqueuing our Dequeue task now with async messages. The async NPIdentifier patch in bug 547359 made this more visible. Patch attached that seems to fix the issue for me.

Comment 1

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

Comment on attachment 434365 [details] [diff] [review]
Patch, v1

This won't catch the case of

  OnMaybeDequeueOne()
    -> RecvFoo()
       -> CallBar()
          [rpc out]

Comment 2

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

Attachment: Only EnqueuePending when there've been sync out-message or rpc out-calls

Comment 3

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

Comment on attachment 434370 [details] [diff] [review]
Only EnqueuePending when there've been sync out-message or rpc out-calls

Works fine for me!

Comment 4

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

I looked over this code again, and we actually only need to EnqueuePending when there's been an *RPC* out-call, as that's the only time received messages get stuffed into mPending without corresponding dequeue tasks.

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

Comment 5

[Benjamin Smedberg]

http://hg.mozilla.org/projects/firefox-lorentz/rev/bcf2d390f21f

Comment 6

[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 7

[Benjamin Smedberg]

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

Comment 8

[Al Billings [:abillings - ex-MoCo]]

I assume that there is nothing for QA to do here to verify this fix?

Comment 9

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

Sure, you can try the testcase from bug 554447, which was a symptom of this bug.  The site (http://now.sprint.com/android/) would be a good litmus test, actually.

Comment 10

[Al Billings [:abillings - ex-MoCo]]

Using the testcase in bug 554447, I'm not getting a crash with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.4pre) Gecko/20100409 Namoroka/3.6.4pre. Marking verified for 1.9.2.