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

RESOLVED FIXED in mozilla12

Status

()

Core
XPConnect
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: bholley, Assigned: bholley)

Tracking

(Blocks: 1 bug)

unspecified
mozilla12
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

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.
Assignee: nobody → bobbyholley+bmo
Blocks: 455548
(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.
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.
Pushed my patches to try: https://tbpl.mozilla.org/?tree=Try&rev=06a39bb2e910

Assuming they go green, I'll upload and flag for review.
Depends on: 717498
(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
All looks good except for one last crash. See bug 676376 comment 6.
(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.
Created attachment 588086 [details] [diff] [review]
Part 1 - Only push null contexts in XPConnect for main thread events, and remove infrastructure from bug 326777. v2
Attachment #588086 - Flags: review?(bzbarsky)
Created attachment 588087 [details] [diff] [review]
Part 2 - Add a release-mode assertion that XPConnect is never used off the main thread. v1
Attachment #588087 - Flags: review?(mrbkap)

Comment 9

6 years ago
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!
Attachment #588086 - Flags: review?(bzbarsky) → review+

Updated

6 years ago
Attachment #588087 - Flags: review?(mrbkap) → review+
Gave this one more push to try: https://tbpl.mozilla.org/?tree=Try&rev=46e2adedf3cd
Doh! Had some other stuff applied. Here's the right try push:

https://tbpl.mozilla.org/?tree=Try&rev=3c1acdd098d7
https://tbpl.mozilla.org/?tree=Try&rev=35133c6b38f5
Pushed to m-i:
http://hg.mozilla.org/integration/mozilla-inbound/rev/a0e3fb36fb1a
http://hg.mozilla.org/integration/mozilla-inbound/rev/ed73f7ef3e7d
And merged to m-c:
https://hg.mozilla.org/mozilla-central/rev/a0e3fb36fb1a
https://hg.mozilla.org/mozilla-central/rev/ed73f7ef3e7d
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
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.
Can you file a separate bug for that so we can get the networking folks involved?
Depends on: 731881
Depends on: 733235
Depends on: 739027

Updated

5 years ago
Depends on: 780770
You need to log in before you can comment on or make changes to this bug.