Closed Bug 489440 Opened 11 years ago Closed 11 years ago

QueryInterface in XPCWrappedNative::FlatJSObjectFinalized can reenter JS

Categories

(Core :: XPConnect, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: dbaron, Assigned: mrbkap)

References

Details

(Keywords: fixed1.9.1)

Attachments

(2 files)

mrbkap says I should file this as a bug, and that it's a regression from peterv's change a month ago:

    nsWrapperCache *cache = nsnull;
    CallQueryInterface(mIdentity, &cache);
    if(cache)
        cache->ClearWrapper();

added in http://hg.mozilla.org/mozilla-central/rev/1e484f30d821


Note that leak-monitor might be needed to trigger this; it's on the stack, but only as the caller to js_GC.  mrbkap says you also need a double-wrapped object in a context that's already had its Components object removed.
Attached patch Proposed fixSplinter Review
This seems like the easiest way to fix this bug. I think it's correct, since if we're a double wrapped object, we can't have possibly cached a wrapper.
Assignee: nobody → mrbkap
Status: NEW → ASSIGNED
Attachment #373945 - Flags: superreview?(peterv)
Attachment #373945 - Flags: review?(peterv)
Attachment #373945 - Flags: superreview?(peterv)
Attachment #373945 - Flags: superreview+
Attachment #373945 - Flags: review?(peterv)
Attachment #373945 - Flags: review+
http://hg.mozilla.org/mozilla-central/rev/d50609a6b63e
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Do we want this on branch (bug 484692 is on it)?
I certainly would.  Without it, leak monitor would be pretty crashy, I think.
Flags: wanted1.9.1?
Comment on attachment 373945 [details] [diff] [review]
Proposed fix

Should take this on branch, needed to make Leak Monitor work.
Attachment #373945 - Flags: approval1.9.1?
Comment on attachment 373945 [details] [diff] [review]
Proposed fix

a191=beltzner
Attachment #373945 - Flags: approval1.9.1? → approval1.9.1+
You need to log in before you can comment on or make changes to this bug.