Last Comment Bug 327716 - XPC_NW_NewResolve should root its cloned function object
: XPC_NW_NewResolve should root its cloned function object
Status: RESOLVED FIXED
[patch]
: fixed1.8.0.4, fixed1.8.1
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Trunk
: x86 Linux
: P1 normal (vote)
: ---
Assigned To: David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch)
: Phil Schwartau
Mentors:
Depends on:
Blocks: 307312
  Show dependency treegraph
 
Reported: 2006-02-18 04:18 PST by David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch)
Modified: 2006-04-05 13:36 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
possible patch (1.09 KB, patch)
2006-02-18 04:20 PST, David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch)
jst: review+
brendan: superreview+
jst: approval‑branch‑1.8.1+
jaymoz: approval1.8.0.4+
Details | Diff | Splinter Review

Description David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2006-02-18 04:18:34 PST
XPC_NW_NewResolve clones a function object and passes it to WrapFunction, which creates another function, which means the first one can be destroyed:

js_GC (/builds/trunk/mozilla/js/src/jsgc.c:1947)
js_NewGCThing (/builds/trunk/mozilla/js/src/jsgc.c:635)
js_NewObject (/builds/trunk/mozilla/js/src/jsobj.c:2008)
js_NewFunction (/builds/trunk/mozilla/js/src/jsfun.c:2030)
JS_NewFunction (/builds/trunk/mozilla/js/src/jsapi.c:3403)
WrapFunction (/builds/trunk/mozilla/js/src/xpconnect/src/XPCNativeWrapper.cpp:236)
XPC_NW_NewResolve (/builds/trunk/mozilla/js/src/xpconnect/src/XPCNativeWrapper.cpp:860)
Comment 1 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2006-02-18 04:20:30 PST
Created attachment 212311 [details] [diff] [review]
possible patch

Not sure whether this is the best approach, but it works.
Comment 2 Brendan Eich [:brendan] 2006-02-20 12:47:05 PST
Comment on attachment 212311 [details] [diff] [review]
possible patch

Sure, that will work and seems necessary in general (alas).

Again it might be better to root as soon as possible, so even before the DEBUG_XPCNativeWrapper paragraph.

/be
Comment 3 Johnny Stenback (:jst, jst@mozilla.com) 2006-02-21 15:14:47 PST
Comment on attachment 212311 [details] [diff] [review]
possible patch

r=jst
Comment 4 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2006-02-21 17:03:16 PST
Checked in to trunk.
Comment 5 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2006-03-10 10:40:52 PST
Checked in to MOZILLA_1_8_BRANCH.
Comment 6 Jay Patel [:jay] 2006-04-05 12:04:28 PDT
Comment on attachment 212311 [details] [diff] [review]
possible patch

Please check in promptly on the 1.8.0 branch.  Thanks!
Comment 7 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2006-04-05 13:36:18 PDT
Fix checked in to MOZILLA_1_8_0_BRANCH.

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