754311 - Copy properties before nulling out the private of about-to-be-transplanted reflectors

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Bobby Holley (:bholley)]

This is one of the bugs causing bug 752309. Patch coming right up.

Comment 1

[Bobby Holley (:bholley)]

Attachment: Null out the private of soon-to-be-transplanted reflectors _after_ copying their properties onto the holder. v1

Attaching a patch. Flagging mrbkap for review.

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 623180 [details] [diff] [review]
Null out the private of soon-to-be-transplanted reflectors _after_ copying their properties onto the holder. v1

Review of attachment 623180 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/xpconnect/src/XPCWrappedNative.cpp
@@ +1660,5 @@
> +            // replaced anyway by the ensuing brain trainsplant, so it doesn't
> +            // really matter. But it can stick around if we take the
> +            // js_TransplantObjectWithWrapper path, or if we've got a bug somewhere.
> +            // If that happens, we want to crash cleanly with a null dereference
> +            // rather than mucking around with the wrong XPCWN.

This is actually important for another reason as well: at this point in time, there are now two JSObjects with the same XPCWrappedNative and they'll both try to delete they're underlying wrapped native when they get finalized. Even though we're going to brain transplant this object, all that actually means is that we're going to swap() it with another object, so we need to forcibly null out the private here.

Comment 3

[Bobby Holley (:bholley)]

Pushed to m-i with an updated comment per comment 2:

http://hg.mozilla.org/integration/mozilla-inbound/rev/b5bef2ea3fd9

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/b5bef2ea3fd9