Last Comment Bug 534341 - XPConnect should not unroot globals of non-main thread contexts
: XPConnect should not unroot globals of non-main thread contexts
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla1.9.2
Assigned To: Peter Van der Beken [:peterv]
: Andrew Overholt [:overholt]
Depends on:
  Show dependency treegraph
Reported: 2009-12-11 16:45 PST by Peter Van der Beken [:peterv]
Modified: 2010-01-27 17:25 PST (History)
7 users (show)
mbeltzner: blocking1.9.2+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

v1 (1.15 KB, patch)
2009-12-11 16:45 PST, Peter Van der Beken [:peterv]
bent.mozilla: review+
jst: superreview+
Details | Diff | Splinter Review
v1 (735 bytes, patch)
2009-12-16 09:57 PST, Peter Van der Beken [:peterv]
peterv: review+
peterv: superreview+
dveditz: approval1.9.1.8+
Details | Diff | Splinter Review

Description Peter Van der Beken [:peterv] 2009-12-11 16:45:22 PST
Created attachment 417203 [details] [diff] [review]

Since we don't cycle collect contexts that are not on the main thread.

We found this while running the test in bug 531225. Bent says this causes crashes (I killed it before it crashed).
Comment 1 Peter Van der Beken [:peterv] 2009-12-14 10:54:10 PST
Comment 2 Johnny Stenback (:jst, 2009-12-14 18:24:39 PST
Comment on attachment 417203 [details] [diff] [review]

+    static inline JSBool IsMainThreadContext(JSContext *cx)
+    {
+        return cx->thread == sMainJSThread;
+    }

We already have IsMainThread(JSContext *cx), maybe just use that? :)

sr=jst with that.
Comment 3 Peter Van der Beken [:peterv] 2009-12-15 11:12:35 PST
Comment 4 Peter Van der Beken [:peterv] 2009-12-16 09:57:41 PST
Created attachment 417945 [details] [diff] [review]
Comment 5 Daniel Veditz [:dveditz] 2009-12-16 16:55:22 PST
Comment on attachment 417945 [details] [diff] [review]

Approved for, a=dveditz for release-drivers
Comment 6 Peter Van der Beken [:peterv] 2009-12-17 16:47:58 PST
Comment 7 Henrik Skupin (:whimboo) 2010-01-27 17:25:50 PST
Peter, any chance for QA to test this fix with a 3.5.8pre build? Do we have automated tests which cover code lines around the given patch and we can trust?

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