490718 - XPCWrappedNativeScope creates a needless cycle with its principal provider

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch

We could just use a raw ptr, patch attached.

Comment 1

[Peter Van der Beken [:peterv]]

Comment on attachment 375102 [details] [diff] [review]
Patch

>diff --git a/js/src/xpconnect/src/xpcwrappednativescope.cpp b/js/src/xpconnect/src/xpcwrappednativescope.cpp

>@@ -447,17 +452,19 @@ XPCWrappedNativeScope::FinishedMarkPhase

>+#ifndef XPCONNECT_STANDALONE
>+            cur->mScriptObjectPrincipal = nsnull;
>+#endif
>             // Move this scope from the live list to the dying list.

Nit: add a newline after the endif.

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

jsteeeeeeeee!

Comment 3

[Johnny Stenback (:jst)]

Comment on attachment 375102 [details] [diff] [review]
Patch

Duh, totally forgot about this one :(

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 375102 [details] [diff] [review]
Patch

I'd love to get this in for 1.9.1 as it helps windows get collected sooner.

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

Needs to bake on trunk before we'd consider for 191, which means that it might be fodder for a security and stability release, though!

Comment 6

[Benjamin Smedberg]

Comment on attachment 375102 [details] [diff] [review]
Patch

Please request approval when it has baked on trunk.

Comment 7

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Oops, I landed this yesterday, changeset 6dd95b1cba8e.

Comment 8

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 375102 [details] [diff] [review]
Patch

Sorry guys, I neglected to indicate that I had landed already. Dunno how much baking you'd want this to have, though I think we would have noticed right away if this patch caused problems.

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Ben: any idea of risk here?

Comment 10

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 375102 [details] [diff] [review]
Patch

Should ask again for 3.5.1, but I don't know if we understand the full risk here. Feel free to renom if you're willing to let us know.