Last Comment Bug 716167 - Add a release-mode assertion that XPConnect is never used off the main thread
: Add a release-mode assertion that XPConnect is never used off the main thread
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla12
Assigned To: Bobby Holley (:bholley) (busy with Stylo)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on: 717498 731881 733235 739027 780770
Blocks: ST-XPConnect
  Show dependency treegraph
 
Reported: 2012-01-06 18:06 PST by Bobby Holley (:bholley) (busy with Stylo)
Modified: 2012-10-22 04:36 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Part 1 - Only push null contexts in XPConnect for main thread events, and remove infrastructure from bug 326777. v2 (13.06 KB, patch)
2012-01-12 10:12 PST, Bobby Holley (:bholley) (busy with Stylo)
bzbarsky: review+
Details | Diff | Splinter Review
Part 2 - Add a release-mode assertion that XPConnect is never used off the main thread. v1 (2.15 KB, patch)
2012-01-12 10:13 PST, Bobby Holley (:bholley) (busy with Stylo)
mrbkap: review+
Details | Diff | Splinter Review

Description Bobby Holley (:bholley) (busy with Stylo) 2012-01-06 18:06:46 PST
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 [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-01-06 18:48:00 PST
(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) (busy with Stylo) 2012-01-10 12:36:31 PST
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) (busy with Stylo) 2012-01-10 12:44:25 PST
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) (busy with Stylo) 2012-01-11 19:29:05 PST
(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) (busy with Stylo) 2012-01-11 23:26:25 PST
All looks good except for one last crash. See bug 676376 comment 6.
Comment 6 Bobby Holley (:bholley) (busy with Stylo) 2012-01-12 10:10:15 PST
(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) (busy with Stylo) 2012-01-12 10:12:49 PST
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
Comment 8 Bobby Holley (:bholley) (busy with Stylo) 2012-01-12 10:13:20 PST
Created attachment 588087 [details] [diff] [review]
Part 2 - Add a release-mode assertion that XPConnect is never used off the main thread. v1
Comment 9 Boris Zbarsky [:bz] (still a bit busy) 2012-01-12 13:24:49 PST
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) (busy with Stylo) 2012-01-13 18:15:34 PST
Gave this one more push to try: https://tbpl.mozilla.org/?tree=Try&rev=46e2adedf3cd
Comment 11 Bobby Holley (:bholley) (busy with Stylo) 2012-01-13 18:29:23 PST
Doh! Had some other stuff applied. Here's the right try push:

https://tbpl.mozilla.org/?tree=Try&rev=3c1acdd098d7
Comment 12 :Ms2ger (⌚ UTC+1/+2) 2012-01-14 00:44:50 PST
https://tbpl.mozilla.org/?tree=Try&rev=35133c6b38f5
Comment 15 Andrew McCreight [:mccr8] 2012-02-29 17:11:08 PST
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 [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-02-29 17:15:42 PST
Can you file a separate bug for that so we can get the networking folks involved?

Note You need to log in before you can comment on or make changes to this bug.