Closed Bug 534341 Opened 14 years ago Closed 14 years ago

XPConnect should not unroot globals of non-main thread contexts

Categories

(Core :: XPConnect, defect)

Product:
Core
Component:
XPConnect
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.9.2
Tracking Flags:
Tracking Status
status1.9.2 --- final-fixed
blocking1.9.1 --- .8+
status1.9.1 --- .8-fixed

People

(Reporter: peterv, Assigned: peterv)

Details

Attachments

(1 file, 1 obsolete file)

v1
735 bytes, patch
peterv
: review+
peterv
: superreview+
dveditz
: approval1.9.1.8+
Details | Diff | Splinter Review
Attached patch v1 (obsolete) — — Splinter 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).
Flags: blocking1.9.2?
Attachment #417203 - Flags: superreview?(jst)
Attachment #417203 - Flags: review?(bent.mozilla)
Attachment #417203 - Flags: review?(bent.mozilla) → review+
http://hg.mozilla.org/mozilla-central/rev/8f6bdbf8e701
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
blocking1.9.1: ? → .7+
status1.9.1: --- → wanted
Comment on attachment 417203 [details] [diff] [review]
v1

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

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

sr=jst with that.
Attachment #417203 - Flags: superreview?(jst) → superreview+
Flags: blocking1.9.2? → blocking1.9.2+
Whiteboard: [has patch][can land 1.9.2]
Attached patch v1 — — Splinter Review 

        Attachment #417203 -
        Attachment is obsolete: true

        Attachment #417945 -
        Flags: superreview+

        Attachment #417945 -
        Flags: review+

        Attachment #417945 -
        Flags: approval1.9.1.7?

    
    

    
  
Comment on attachment 417945 [details] [diff] [review]
v1

Approved for 1.9.1.7, a=dveditz for release-drivers

        Attachment #417945 -
        Flags: approval1.9.1.7? → approval1.9.1.7+

    
    

    
  
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?

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