716167 - Add a release-mode assertion that XPConnect is never used off the main thread

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Bobby Holley (:bholley)]

Now that bug 715757 is well underway, it's time to start ripping thread support out of XPConnect. The first step is to push through a release-mode assertion to make sure extension developers etc aren't doing this.

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Bobby Holley (:bholley) from comment #0)
> Now that bug 715757 is well underway, it's time to start ripping thread
> support out of XPConnect. The first step is to push through a release-mode
> assertion to make sure extension developers etc aren't doing this.

I will bet you beer that extension developers are doing this.

Doesn't mean we shouldn't do this, of course.

Comment 2

[Bobby Holley (:bholley)]

It looks like we'll need some special engineering for the CC thread, so for now I'm just going to assert that we're on the main _or_ CC thread.

Comment 3

[Bobby Holley (:bholley)]

Pushed my patches to try: https://tbpl.mozilla.org/?tree=Try&rev=06a39bb2e910

Assuming they go green, I'll upload and flag for review.

Comment 4

[Bobby Holley (:bholley)]

(In reply to Bobby Holley (:bholley) from comment #3)
> Assuming they go green

They did not. :-(

The nsThread observer stuff turned out to be trickier than I thought, largely because of various things that suck about that code. I made various fixes to the patch in this bug, and also did some more work over in bug 717498.

Trying again: https://tbpl.mozilla.org/?tree=Try&rev=7ce2fa160d08

Comment 5

[Bobby Holley (:bholley)]

All looks good except for one last crash. See bug 676376 comment 6.

Comment 6

[Bobby Holley (:bholley)]

(In reply to Bobby Holley (:bholley) from comment #5)
> All looks good except for one last crash. See bug 676376 comment 6.

Looks like Ms2ger fixed this on trunk yesterday!

All should be green now. Posting patches.

Comment 7

[Bobby Holley (:bholley)]

Attachment: Part 1 - Only push null contexts in XPConnect for main thread events, and remove infrastructure from bug 326777. v2

Comment 8

[Bobby Holley (:bholley)]

Attachment: Part 2 - Add a release-mode assertion that XPConnect is never used off the main thread. v1

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 588086 [details] [diff] [review]
Part 1 - Only push null contexts in XPConnect for main thread events, and remove infrastructure from bug 326777. v2

s/and 'after'/an 'after'/

r=me.  This is way simpler now that we support more than one thread observer!

Comment 10

[Bobby Holley (:bholley)]

Gave this one more push to try: https://tbpl.mozilla.org/?tree=Try&rev=46e2adedf3cd

Comment 11

[Bobby Holley (:bholley)]

Doh! Had some other stuff applied. Here's the right try push:

https://tbpl.mozilla.org/?tree=Try&rev=3c1acdd098d7

Comment 12

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://tbpl.mozilla.org/?tree=Try&rev=35133c6b38f5

Comment 13

[Bobby Holley (:bholley)]

Pushed to m-i:
http://hg.mozilla.org/integration/mozilla-inbound/rev/a0e3fb36fb1a
http://hg.mozilla.org/integration/mozilla-inbound/rev/ed73f7ef3e7d

Comment 14

[Jonathan Kew [:jfkthame]]

And merged to m-c:
https://hg.mozilla.org/mozilla-central/rev/a0e3fb36fb1a
https://hg.mozilla.org/mozilla-central/rev/ed73f7ef3e7d

Comment 15

[Andrew McCreight [:mccr8]]

Here's a crash I noticed on this assert:
https://crash-stats.mozilla.com/report/index/96622599-ebc5-40bc-b0ae-5daaa2120226

I don't know if that is addon related or what.  An nsHttpChannel destructor is causing an XPCTraceableVariant to call its destructor.

Comment 16

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Can you file a separate bug for that so we can get the networking folks involved?