Closed
Bug 770840
Opened 13 years ago
Closed 12 years ago
Add runtime abort for using an XPCWrappedJS off the main thread
Categories
(Core :: XPConnect, defect)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
mozilla24
People
(Reporter: bholley, Assigned: bholley)
References
(Depends on 1 open bug)
Details
Attachments
(2 files)
1.98 KB,
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
3.33 KB,
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
Looks like this actually happens in a few places. :-(
Currently it gets caught by various belt-and-suspenders checking, so it never manages to actually hit our abort. We should fix this code to just runtime-abort.
Assignee | ||
Comment 1•12 years ago
|
||
All of the deps here have now landed, so this should be good to go.
https://tbpl.mozilla.org/?tree=Try&rev=348dbf349cd4
Assignee | ||
Comment 2•12 years ago
|
||
This test used to check that we'd error-out properly, but now it'll just cause us to abort.
Attachment #737017 -
Flags: review?(mrbkap)
Assignee | ||
Comment 3•12 years ago
|
||
Attachment #737018 -
Flags: review?(mrbkap)
Assignee | ||
Comment 4•12 years ago
|
||
Looks like jdm's nsSocketTransport2 patch didn't catch all of the cases where we addref/release XPCWrappedJS off-main-thread. See the crashes in the comment 1 try push.
JDM, is this something you think you can fix?
Flags: needinfo?(josh)
Comment 5•12 years ago
|
||
Yes, I'll try to take some time this week to dig in some more.
Flags: needinfo?(josh)
Assignee | ||
Comment 6•12 years ago
|
||
Awesome, thanks JDM! :-)
Updated•12 years ago
|
Attachment #737017 -
Flags: review?(mrbkap) → review+
Comment 7•12 years ago
|
||
Comment on attachment 737018 [details] [diff] [review]
Part 2 - Add Runtime aborts when using XPCWrappedJS off-main-thread. v2
r=me assuming this is green.
Attachment #737018 -
Flags: review?(mrbkap) → review+
Comment 8•12 years ago
|
||
Comment 9•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1bc459138617
https://hg.mozilla.org/mozilla-central/rev/6be352955252
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Comment 10•12 years ago
|
||
Backed out for causing bug 870782.
https://hg.mozilla.org/mozilla-central/rev/2673016e7df4
This also caused c-c bustage, fwiw.
https://tbpl.mozilla.org/php/getParsedLog.php?id=22823512&tree=Thunderbird-Trunk
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla23 → ---
Updated•12 years ago
|
Comment 11•12 years ago
|
||
Comment 12•12 years ago
|
||
Giving try another shot before landing: https://tbpl.mozilla.org/?tree=Try&rev=274e9ae31511
Comment 13•12 years ago
|
||
It's super effective!
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/593e06824257
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/fca5e32db9d7
Comment 14•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/593e06824257
https://hg.mozilla.org/mozilla-central/rev/fca5e32db9d7
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Assignee | ||
Comment 15•12 years ago
|
||
\o/
Thanks a million, jdm.
![]() |
||
Comment 16•12 years ago
|
||
This completely destroys possibility to write xpcshell tests that have callbacks on non-main thread :(((
Coming at the right moment we have successfully written tests for the new http cache...
Comment 17•12 years ago
|
||
I recommend writing a debug C++ component that proxies the callback to a main thread listener, if possible.
![]() |
||
Comment 18•12 years ago
|
||
(In reply to Josh Matthews [:jdm] from comment #17)
> I recommend writing a debug C++ component that proxies the callback to a
> main thread listener, if possible.
Yes, that is obvious. But I need a result.
You need to log in
before you can comment on or make changes to this bug.
Description
•