Closed
Bug 300079
Opened 17 years ago
Closed 15 years ago
XUL chrome precompiled functions don't inherit from current window's Function.prototype
Categories
(Core :: JavaScript Engine, defect, P2)
Core
JavaScript Engine
Tracking
()
VERIFIED
FIXED
mozilla1.9alpha6
People
(Reporter: dominique.arnou, Assigned: brendan)
References
()
Details
(Keywords: helpwanted)
Attachments
(4 files, 9 obsolete files)
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; fr-FR; rv:1.7.8) Gecko/20050511 Firefox/1.0.4 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; fr-FR; rv:1.7.8) Gecko/20050511 Firefox/1.0.4 When I try to implement rico.js and prototype.js in a XUL page, without use their functions, this error appear this._mouseUpHandler.bindAsEventListener is not a function Fichier Source : chrome://tests/content/rico.js Ligne : 1301 This error is not generated with an HTML document... where is the difference ? Reproducible: Always Steps to Reproduce: 1.include scripts prototype.js and rico.js in an empty xul doc 2.call this xul page in your favorite browser Actual Results: the error "... is not a function" appear Expected Results: no error, nothing else
Comment 1•17 years ago
|
||
not a js engine bug
Assignee: general → nobody
Component: JavaScript Engine → XP Toolkit/Widgets: XUL
QA Contact: general → xptoolkit.xul
Comment 2•17 years ago
|
||
Simpler test case: Function.prototype.foo = "bar"; function f(){} alert(f.foo); Alerts "undefined" when executed in chrome (extension).
Assignee | ||
Comment 3•17 years ago
|
||
This sounds like an uninteded XPCNativeWrapper automation effect, if it affects JS prototype and instance behavior in one and only one window. If it affects how content script can or cannot see properties set by chrome script where the chrome manifest does not say xpcnativewrappers=no, then it's expected and desired secure behavior, new in 1.5. In the latter event, the way to get to the content object is via the wrappedJSObject property of the XPCNativeWrapper -- or if you are *sure* it's safe to do so, to set xpcnativewrappers=no in the chrome manifest. See http://developer.mozilla.org/en/docs/XPCNativeWrapper for docs and feel free to help fix them if you are able. /be
Status: UNCONFIRMED → NEW
Component: XP Toolkit/Widgets: XUL → XP Miscellany
Ever confirmed: true
Assignee | ||
Comment 4•17 years ago
|
||
If this is all within one window (as it sounds like -- attaching a testcase would cinch it), then this sounds like something broke in "brutal sharing", which for a long time has precompiled JS functions once only, sharing their scripts among N windows loading the XUL via cloned function objects. A cloned functoin object f' delegates to its prototype f, the clone-parent. And f's prototype is the object referenced by Function.prototype when the script in which f is declared or expressed was compiled. I only now noticed the report was against 1.0.4. Is this broken also in Firefox 1.5b1? Argh. Any idea when this broke since Firefox 1.0 (when I believe, or at least hope, it worked)? /be
Comment 5•17 years ago
|
||
Here is some XUL test case run from HTTP (not chrome): <http://home.arcor.de/martin.honnen/mozillaBugs/xul/FunctionPrototypeTest1.xul> The test script used is here: <http://home.arcor.de/martin.honnen/mozillaBugs/xul/FunctionPrototypeTest1.js> A HTML test case using the same script is here: <http://home.arcor.de/martin.honnen/mozillaBugs/xul/FunctionPrototypeTest1.html> Output for the XUL test case with Firefox 1.5 beta nightly (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050912 Firefox/1.4): Function.prototype.GOD: Kibo f.GOD: undefined f instanceof Function: false f.constructor: function Function() { [native code] } f.constructor.GOD: undefined Output for XUL test case with Mozilla suite 1.7.11 release (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.11) Gecko/20050728) Function.prototype.GOD: Kibo f.GOD: undefined f instanceof Function: true f.constructor: function Function() { [native code] } f.constructor.GOD: undefined So the only difference between Mozilla 1.7.11 and Firefox 1.5 beta 1 nightly is that 'instanceof Function' gives true in the former while it yields false in the latter. The property added to Function.prototype is not inherited at all in both builds. For HTML everything is the same in both builds and the prototype based inheritance is there: Function.prototype.GOD: Kibo f.GOD: Kibo f instanceof Function: true f.constructor: function Function() { [native code] } f.constructor.GOD: Kibo
Comment 6•17 years ago
|
||
I have now tried the XUL test case with Mozilla 1.0 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.0.0) Gecko/20020530) where the result is already showing that properties added to Function.prototype are not inherited to functions: Function.prototype.GOD: Kibo f.GOD: undefined f instanceof Function: true f.constructor: function Function() { [native code] } f.constructor.GOD: undefined
Comment 7•17 years ago
|
||
Further tests with: - Mozilla 1.1 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.1) Gecko/20020826) - Mozilla 1.2.1 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.2.1) Gecko/20021130) - Mozilla 1.4 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4) Gecko/20030624) - Mozilla 1.5 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5) Gecko/20031007) - Mozilla 1.6 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6) Gecko/20040113) - Mozilla 1.7 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040616) all show the same result for the XUL document: Function.prototype.GOD: Kibo f.GOD: undefined f instanceof Function: true f.constructor: function Function() { [native code] } f.constructor.GOD: undefined So as far as I see that currently adding properties to Function.prototype in script in XUL documents has never made those properties available to functions defined in the script.
Assignee | ||
Comment 8•17 years ago
|
||
Yup, this goes back to 2001 or so, the dawn of "brutal sharing" (precompilation of XUL scripts, including their functions). There is a Function.prototype per chrome window, but functions precompiled against a dummy singleton scope object, or more recently against null, do not delegate to it. To fix this we would need to do some JS engine magic. /be
Assignee: nobody → brendan
Component: XP Miscellany → JavaScript Engine
OS: Windows 2000 → All
Hardware: PC → All
Comment 9•17 years ago
|
||
*** Bug 313411 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 10•17 years ago
|
||
*** Bug 321231 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 11•17 years ago
|
||
Bug 321231 complained about instanceof changing from Firefox 1 to 1.5, but as comment 5 et seq. show, instanceof lied in Firefox 1. This is arguably a bug in XUL's use of the JS API, but we'll need API extensions to fix it, so I'm leaving it in Core / JavaScript Engine. /be
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla1.9alpha
Assignee | ||
Updated•17 years ago
|
Summary: In XUL, a js function don't inherit from Function Object (and its prototype properties) → In XUL, a precompiled function doesn't inherit from the current window's Function object
Comment 12•17 years ago
|
||
Are there any new information on this bug? Is there any hope this could be fixed for 1.9?
Assignee | ||
Comment 13•17 years ago
|
||
This will be fixed for 1.9. /be
![]() |
||
Updated•17 years ago
|
Flags: blocking1.9a2?
Updated•16 years ago
|
Flags: blocking1.9a2? → blocking1.9-
Whiteboard: [wanted-1.9]
Assignee | ||
Comment 14•16 years ago
|
||
*** Bug 350655 has been marked as a duplicate of this bug. ***
Assignee | ||
Updated•16 years ago
|
Summary: In XUL, a precompiled function doesn't inherit from the current window's Function object → XUL chrome precompiled functions don't inherit from current window's Function.prototype
Comment 15•16 years ago
|
||
I tested the following on a js script loaded over HTTP by a remote XUL page: Function.prototype.test = function() { alert('Test'); } testme = function() { } testme.test(); The test method was not invoked so it seems that this bug also happens for remote XUL pages (not Chrome). On an HTML page it worked fine. A similar test with Object also works fine. Has it been fixed in 1.9a? If not, when can we expect a fix? Thanks.
![]() |
||
Comment 16•16 years ago
|
||
If it had been fixed, it would have been resolved fixed. And it might not get fixed in 1.9, though drivers would like to try to find resources somewhere to deal with it -- see status whiteboard and blocking flags.
Keywords: helpwanted
Assignee | ||
Comment 17•16 years ago
|
||
I'll give it a go. /be
Assignee | ||
Comment 19•15 years ago
|
||
Ok, working on this. /be
Target Milestone: mozilla1.9alpha1 → mozilla1.9alpha6
Assignee | ||
Comment 20•15 years ago
|
||
The essential fix here is the one-line change to js_CloneFunctionObject. The rest is hygiene inspired by bug 383269 and highlighted by this patch's change to cloned function object prototyping. Since with this patch, a cloned function object no longer holds onto its "clone-parent" function object via its proto slot, this patch changes fun_trace to trace fun->object unconditionally. This may run into problems with the cycle collector -- see bug 375999 and bug 375808 before it, both still open. Testing needed, I wanted to throw this out for review and others' testing. /be
Attachment #268622 -
Flags: review?(igor)
Assignee | ||
Comment 21•15 years ago
|
||
I should move ATOM_OFFSET and OFFSET_TO_ATOM to jsatom.h and remove the originals in jsapi.c, rather than dup'ing those two into jsfun.c. /be
Comment 22•15 years ago
|
||
I will review the patch tomorrow.
Assignee | ||
Comment 23•15 years ago
|
||
Attachment #268622 -
Attachment is obsolete: true
Attachment #268631 -
Flags: review?(igor)
Attachment #268622 -
Flags: review?(igor)
Assignee | ||
Comment 24•15 years ago
|
||
No need for JSVAL_TO_STRING from id or from ATOM_KEY(atom). /be
Attachment #268631 -
Attachment is obsolete: true
Attachment #268691 -
Flags: review?(igor)
Attachment #268631 -
Flags: review?(igor)
Assignee | ||
Comment 25•15 years ago
|
||
This patch adds an important trust-boundary check before making f'.prototype.__proto__ == f.prototype for cloned function object f' and clone-parent f. /be
Attachment #268691 -
Attachment is obsolete: true
Attachment #268710 -
Flags: review?(igor)
Attachment #268691 -
Flags: review?(igor)
Assignee | ||
Updated•15 years ago
|
Attachment #268710 -
Flags: review?(mrbkap)
Assignee | ||
Comment 26•15 years ago
|
||
Comment on attachment 268710 [details] [diff] [review] proposed fix, with same-object-principal check on parentProto This is wrong, of course: without the proto link to the clone-parent, call_resolve can't find hidden-atom-named properties for formals and locals to reflect. More work in fun_resolve is needed. /be
Attachment #268710 -
Attachment is obsolete: true
Attachment #268710 -
Flags: review?(mrbkap)
Attachment #268710 -
Flags: review?(igor)
Assignee | ||
Comment 27•15 years ago
|
||
Attachment #268719 -
Flags: review?(igor)
Assignee | ||
Comment 28•15 years ago
|
||
Comment on attachment 268719 [details] [diff] [review] proposed fix, with correct hidden property resolution This works in my Minefield build. Running the js/tests suite now. /be
Attachment #268719 -
Flags: review?(mrbkap)
Comment 29•15 years ago
|
||
Comment on attachment 268719 [details] [diff] [review] proposed fix, with correct hidden property resolution jsregexp.c changes are unrelated to the bug and it will be nice to move them to a separated bug. r+ with that.
Assignee | ||
Comment 30•15 years ago
|
||
js/tests pass with known failures (AFAICT). jsregexp.c patch split out as bug 384846. /be
Attachment #268719 -
Attachment is obsolete: true
Attachment #268751 -
Flags: review+
Attachment #268719 -
Flags: review?(mrbkap)
Attachment #268719 -
Flags: review?(igor)
Assignee | ||
Comment 31•15 years ago
|
||
Fixed on trunk: js/src/jsapi.c 3.328 js/src/jsatom.h 3.62 js/src/jsfun.c 3.190 Tony, IIRC some support JS for Safe Browsing works around this bug. It should not need to in 1.9/Fx3. /be
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
![]() |
||
Comment 32•15 years ago
|
||
I backed this out. This had the salutary effect of fixing the crash on our leak tinderbox that was preventing us from gettting leak numbers and keeping anyone from checking in (8 hours after this was checked in, too). Unfortunately, the tinderbox stack has the usual symbol issue; if someone has access to the tbox they could try running it through fix-linux-stack.pl on there...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
![]() |
||
Comment 33•15 years ago
|
||
Applying the patch to one of my debug builds gives me a consistent shutdown crash due to us trying to log a release on the mRawPtr of an nsCOMPtr<nsIAtom>. The vtable pointer of said mRawPtr is 0xdadadada. The stack is compatible with the crash tinderbox is seeing.
Assignee | ||
Comment 34•15 years ago
|
||
Sorry, had to be out of town early till late today. Will investigate -- I didn't see that shutdown crash, FWIW. /be
Status: REOPENED → ASSIGNED
Assignee | ||
Comment 35•15 years ago
|
||
(In reply to comment #34) > Will investigate -- I didn't see that shutdown crash, FWIW. I thought I tested all the way thru shutdown, but I must not have -- sorry again. So this patch seems to cause the last GC to happen after XPCOM shutdown, during which the Gecko static atoms have been nuked. Details in an attachment next, but it's scary that we have this fragile shutdown logic. Old news, I suppose. /be
Assignee | ||
Comment 36•15 years ago
|
||
![]() |
||
Comment 37•15 years ago
|
||
Benjamin, see comment 35 and comment 36... We really shouldn't have stuff alive this late in shutdown unless we basically leaked, right?
Assignee | ||
Comment 38•15 years ago
|
||
Comments more than welcome. I'd like to re-land this as is and get to the bottom of the issues raised in the comment in a followup bug (which might be one of the bugs cited there). /be
Attachment #268922 -
Flags: review?(igor)
Assignee | ||
Comment 39•15 years ago
|
||
(In reply to comment #37) > Benjamin, see comment 35 and comment 36... We really shouldn't have stuff > alive this late in shutdown unless we basically leaked, right? There was in fact a leak due to the tracing of the fun->object "back pointer". /be
Updated•15 years ago
|
Attachment #268922 -
Flags: review?(igor) → review+
![]() |
||
Comment 40•15 years ago
|
||
Hmm... Should we be clearing the prototype cache early on in shutdown? Would that help?
Comment 41•15 years ago
|
||
It's not clear from the comments above which atoms we're talking about, and what "last GC after XPCOM shutdown" means. If we're talking about something in the prototype cache being held after the layout module has shut down, that probably means we need to hold onto nsLayoutStatics from the object that's being kept alive. What side effects does clearing the prototype cache have?
![]() |
||
Comment 42•15 years ago
|
||
These are the Gecko static atoms (nsGkAtomList), as far as I can tell....
Assignee | ||
Comment 43•15 years ago
|
||
(In reply to comment #41) > It's not clear from the comments above which atoms we're talking about, and > what "last GC after XPCOM shutdown" means. From the attachment: #3 0x2c012169 in NS_PurgeAtomTable () at nsAtomTable.cpp:376 and: #12 0x2303e716 in js_GC (cx=0x22c308c0, gckind=GC_LAST_CONTEXT) at jsgc.c:2982 #13 0x23017952 in js_DestroyContext (cx=0x22c308c0, mode=JSDCM_FORCE_GC) at jscntxt.c:409 #14 0x23008a2e in JS_DestroyContext (cx=0x22c308c0) at jsapi.c:984 #15 0x12036581 in XPCJSContextStack::~XPCJSContextStack (this=0x32eaa0) at xpcthreadcontext.cpp:61 #16 0x12035947 in XPCPerThreadData::Cleanup (this=0x32ea60) at xpcthreadcontext.cpp:483 #17 0x1203716b in XPCPerThreadData::~XPCPerThreadData (this=0x32ea60) at xpcthreadcontext.cpp:492 #18 0x1203726c in xpc_ThreadDataDtorCB (ptr=0x32ea60) at xpcthreadcontext.cpp:527 #19 0x2000a25e in _PR_DestroyThreadPrivate (self=0x3070c0) at prtpd.c:265 #20 0x200206f1 in _pt_thread_death (arg=0x3070c0) at ptthread.c:825 #21 0x200209bb in _PR_Fini () at ptthread.c:999 #22 0x8fe0e33d in __dyld__ZN16ImageLoaderMachO13doTerminationERKN11ImageLoader11LinkContextE () #23 0x8fe030ec in __dyld__ZN4dyld14runTerminatorsEv () #24 0x00002c74 in cxa_atexit_wrapper () > If we're talking about something in > the prototype cache being held after the layout module has shut down, that > probably means we need to hold onto nsLayoutStatics from the object that's > being kept alive. Why would anything DOM-ish want to outlive XPCOM shutdown? Sure, leaks cause that, but it's a situation we might hope to rectify with more work. For instance if the DOM code knows XPCOM is shutting down, and it knows windows will survive that event, perhaps it could put them in some kind of safe mode. At the least, any static atoms managed by XPCOM would have to be safely nulled, or else also survive -- but these are xpcom/ds-hosted atoms, so how can XPCOM shutdown yet let them live? You may know of bugs on making leaks across shutdown less harmful, or otherwise improving the situation. Obviously we should fix all shutdown-surviving leaks, but my question is: can we do more to mitigate their effects? > What side effects does clearing the prototype cache have? Not clear. Could have bad effects per the FIXME in the latest patch here, if a function prototype goes away before a cloned function object is activated in a (presumably leaking) chrome window. Does XPCOM shutdown guarantee that no more chrome scripts or event handlers can run? /be
Assignee | ||
Comment 44•15 years ago
|
||
(In reply to comment #43) > but my question is: can we do more to mitigate their effects? I should have said "mitigate their effects in release builds?" In debug builds, we should have assertions botching (warnings in the short run if too many to fix for a clean tinderbox with assertions fatal) as soon as possible. Ideally at XPCOM shutdown if there are identifiable leaks. /be
Assignee | ||
Comment 45•15 years ago
|
||
Fixed fix landed: js/src/jsapi.c 3.330 js/src/jsatom.h 3.64 js/src/jsfun.c 3.192 /be
Status: ASSIGNED → RESOLVED
Closed: 15 years ago → 15 years ago
Resolution: --- → FIXED
Comment 46•15 years ago
|
||
> #18 0x1203726c in xpc_ThreadDataDtorCB (ptr=0x32ea60) at > xpcthreadcontext.cpp:527 > #19 0x2000a25e in _PR_DestroyThreadPrivate (self=0x3070c0) at prtpd.c:265 ok, that's a serious xpconnect bug... it should be clearing the threadprivates during xpcom-shutdown-threads or perhaps in its module shutdown. I've seen similar bugs before that I thought were fixed. Timeless? And it turns out the rest of my ramble about layout shutdown is irrelevant, because layout isn't actually involved at all. > (presumably leaking) chrome window. Does XPCOM shutdown guarantee that no more > chrome scripts or event handlers can run? At some point late in the sequence, that is considered an invariant, yes. Doesn't mean we actually follow the rules.
Comment 47•15 years ago
|
||
Had to back this out again -- fun->object was being collected too early, causing us not to find local variables and arguments on the scope chain.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
![]() |
||
Comment 48•15 years ago
|
||
I'm hoping we'll check in a test for that with the next version?
Assignee | ||
Comment 49•15 years ago
|
||
Third time's the charm. We don't have js shell tests for this embedding-specific bug. The JS component loader compiles and executes (why does it not just evaluate? Oh well) a script, throwing away the script and its atoms after it has been executed for effects against the global for the component's location. That means the prototype function objects are liable to be GC'ed. Unlike the XUL prototype cache, which hangs onto scripts, the JS component loader exercises a different pattern of API usage. I can write a test that uses the clone() js-shell function to test directly for this bug. /be
Status: REOPENED → ASSIGNED
Assignee | ||
Comment 50•15 years ago
|
||
Assignee | ||
Comment 51•15 years ago
|
||
Attachment #269033 -
Attachment is obsolete: true
Assignee | ||
Comment 52•15 years ago
|
||
Don't trace fun->object if it's null or if it is the same as obj, the function object on which fun_trace was called. This passes the js shell test and Minefield testing including clean shutdown. This may fix the problem encountered with the cycle collector in bug 375999. /be
Attachment #268922 -
Attachment is obsolete: true
Attachment #269035 -
Flags: review?(igor)
Assignee | ||
Updated•15 years ago
|
Attachment #269035 -
Flags: review?(mrbkap)
Assignee | ||
Comment 53•15 years ago
|
||
Attachment #269035 -
Attachment is obsolete: true
Attachment #269038 -
Flags: review?(igor)
Attachment #269035 -
Flags: review?(mrbkap)
Attachment #269035 -
Flags: review?(igor)
Assignee | ||
Updated•15 years ago
|
Attachment #269038 -
Flags: review?(mrbkap)
![]() |
||
Comment 54•15 years ago
|
||
It doesn't have to be a js shell test; we can exercise the XUL proto cache via mochitest, right?
Assignee | ||
Comment 55•15 years ago
|
||
(In reply to comment #54) > It doesn't have to be a js shell test; we can exercise the XUL proto cache via > mochitest, right? The XUL prototype cache protects scripts whose literals include prototypal function objects, so there's no failure case there to test. The js shell test is the most direct and has fewest dependencies. A JS component test could be added on top, but there's no reason to prefer it to the equivalent js shell test. /be
Updated•15 years ago
|
Attachment #269038 -
Flags: review?(mrbkap) → review+
Assignee | ||
Comment 56•15 years ago
|
||
Once more, with feeling -- or unto the breach, as Henry V put it: js/src/jsapi.c 3.332 js/src/jsatom.h 3.66 js/src/jsfun.c 3.194 Igor's review is welcome tomorrow and I'll file followup bugs if needed. I want to get this into builds ASAP, with mrbkap's r+. This was painful, but in hindsight there's no way to cheat Mother-GC and I should not have tried. We really do need to trace fun->object. But we do not want to trace the object currently being traced (this is different from the old mark hook, where the mark bit would be set on that object so that any self-mark attempts by its mark hook would immediately return). /be
Status: ASSIGNED → RESOLVED
Closed: 15 years ago → 15 years ago
Resolution: --- → FIXED
Updated•15 years ago
|
Attachment #269038 -
Flags: review?(igor) → review+
Comment 57•15 years ago
|
||
/cvsroot/mozilla/js/tests/js1_8/extensions/regress-300079.js,v <-- regress-300079.js initial revision: 1.1
Flags: in-testsuite+
This seems to have caused orange (shutdown crashes) on qm-xserve01 and qm-rhel02. Possibly also fxdbug-linux-tbox.
Comment 59•15 years ago
|
||
Tree is closed due to orange.
Assignee | ||
Comment 60•15 years ago
|
||
fxdbug-linux-tbox has non symbols, but this crash at shutdown stack:
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/libxul.so +0x00058EC8]
UNKNOWN [/lib/tls/libpthread.so.0 +0x0000B898]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libgklayout.so +0x000FC977]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libgklayout.so +0x0050D6A2]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libgklayout.so +0x0050D6B5]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libgklayout.so +0x0050D6FC]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libgklayout.so +0x0050D7DE]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libgklayout.so +0x00539ABA]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libgklayout.so +0x00539ADD]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libgklayout.so +0x00539AFF]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libgklayout.so +0x005567F5]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libgklayout.so +0x0055680C]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libgklayout.so +0x005492D4]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libgklayout.so +0x00563A40]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libgklayout.so +0x00576C9B]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libgklayout.so +0x000D7F66]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libgklayout.so +0x000D80E4]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libgklayout.so +0x0060BD79]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libgklayout.so +0x006178A9]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libgklayout.so +0x006091AF]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libgklayout.so +0x006091E5]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libxpconnect.so +0x000435BB]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libgklayout.so +0x005DEC31]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/libmozjs.so +0x0005D3FB]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/libmozjs.so +0x0002B46A]
JS_DestroyContext+0x00000019 [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/libmozjs.so +0x0001873A]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libxpconnect.so +0x0004A5A5]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libxpconnect.so +0x0004991D]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libxpconnect.so +0x00049AAF]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/components/libxpconnect.so +0x00049B9B]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/libnspr4.so +0x000142B8]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/libnspr4.so +0x000311DA]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/libnspr4.so +0x00031553]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/libnspr4.so +0x00008E70]
UNKNOWN [/builds/tinderbox/Fx-Trunk-Debug/Linux_2.6.9-42.ELsmp_Depend/mozilla/obj/dist/bin/libnspr4.so +0x00036C1A]
UNKNOWN [/lib/ld-linux.so.2 +0x0000C907]
exit+0x00000077 [/lib/tls/libc.so.6 +0x0002A527]
__libc_start_main+0x000000DD [/lib/tls/libc.so.6 +0x00014DED]
looks like the one in attachment 268914 [details].
So there's an XPCOM/JS leak that defers last GC until after XPCOM has shut down and purged atoms stashed in statics by XSLT code, or similar.
This is a bug I'd like to fix directly instead of backing out again. More in a few minutes (by 10am PDT).
/be
Assignee | ||
Comment 61•15 years ago
|
||
I tried landing this patch: Index: jsfun.c =================================================================== RCS file: /cvsroot/mozilla/js/src/jsfun.c,v retrieving revision 3.194 diff -p -u -8 -r3.194 jsfun.c --- jsfun.c 20 Jun 2007 06:37:15 -0000 3.194 +++ jsfun.c 20 Jun 2007 17:01:46 -0000 @@ -1508,17 +1508,17 @@ fun_hasInstance(JSContext *cx, JSObject static void fun_trace(JSTracer *trc, JSObject *obj) { JSFunction *fun; fun = (JSFunction *) JS_GetPrivate(trc->context, obj); if (fun) { JS_CALL_TRACER(trc, fun, JSTRACE_FUNCTION, "private"); - if (fun->object != obj) + if (IS_GC_MARKING_TRACER(trc) && fun->object != obj) JS_CALL_TRACER(trc, fun->object, JSTRACE_OBJECT, "object"); if (fun->atom) JS_CALL_TRACER(trc, fun->atom, JSTRACE_ATOM, "atom"); if (FUN_INTERPRETED(fun) && fun->u.i.script) js_TraceScript(trc, fun->u.i.script); } } as this revision: js/src/jsfun.c 3.195 Hoping it helps, will back out to re-open tree otherwise. /be
Comment 62•15 years ago
|
||
(In reply to comment #61) > > Hoping it helps, will back out to re-open tree otherwise. No longer crashing on shutdown, but we are now leaking a bunch instead (Rlk spike).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Please back this out so we can reopen the tree.
Assignee | ||
Comment 64•15 years ago
|
||
(In reply to comment #63) > Please back this out so we can reopen the tree. Tried to ping you on IRC. This comment seems stale because the tree was already open at its timestamp. I'd like to get some time to work on the leak bug(s) that may be biting RLk. If that's ok with you we can let this bug stay closed for now. I will file a bug on the RLk jump unless there's already one on file that someone can point out here. /be
Status: REOPENED → RESOLVED
Closed: 15 years ago → 15 years ago
Resolution: --- → FIXED
Comment 65•15 years ago
|
||
(In reply to comment #64) > > I will file a bug on the RLk jump unless there's already one on file that > someone can point out here. I backed this out because roc needs to land the new textframe code, and we shouldn't do that with Rlk above 800k. Sorry. :/
Assignee | ||
Comment 66•15 years ago
|
||
Sayrer: sorry, didn't see you did indeed back out this patch plus the one-line change to jsfun.c -- thanks for that. Reopening. /be
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 67•15 years ago
|
||
The change in comment 61 fixed orange tinderboxen, so there's clearly something to-do with the cycle collector (the only other user of the JS GC tracing API) that makes for shutdown leaks, which result in crashes due to Gecko XPCOM atoms that are memoized in statics being dereferenced after XPCOM shutdown has purged atoms. That's bug #1. Bug #2 is that with the hack in comment 61, RLk jumped. This suggests that there are uncollected XPCOM/JS cycles involving fun->object that happen not to result in bloat-test crashes, "merely" in leaks. Catch-22. More after a breather. I am not going to check in without running all the bloat tests first. /be
Status: REOPENED → ASSIGNED
Assignee | ||
Updated•15 years ago
|
Attachment #269038 -
Attachment description: tigher logic: fun->object can't be null in fun_resolve or fun_trace → tighter logic: fun->object can't be null in fun_resolve or fun_trace
Comment 68•15 years ago
|
||
Brendan and I tried this patch to try to keep the function atoms alive longer (which we hoped would fix the bustage caused by attachment 268922 [details] [diff] [review]) but it didn't work.
Assignee | ||
Comment 69•15 years ago
|
||
Comment on attachment 269116 [details] [diff] [review] (Failed) Component loader patch This is really the wrong approach, just for the record. We wanted to try a workaround that made JS components work like the XUL prototype cache. The last patch, which landed and was backed out (not including the patch in comment 61) is right. Each function must have a strong reference to the original (clone-parent) compiler-created function object. There are bugs blocking this bug from landing. They'll be filed as needed, marked blocking, and fixed. After that, the last js/src patch will land again, and I'd bet money it will land unchanged except to merge up to trunk tip. /be
Attachment #269116 -
Attachment is obsolete: true
Comment 70•15 years ago
|
||
>> #18 0x1203726c in xpc_ThreadDataDtorCB (ptr=0x32ea60) at >> xpcthreadcontext.cpp:527 >> #19 0x2000a25e in _PR_DestroyThreadPrivate (self=0x3070c0) at prtpd.c:265 > ok, that's a serious xpconnect bug... it should be clearing the threadprivates > during xpcom-shutdown-threads or perhaps in its module shutdown. I've seen > similar bugs before that I thought were fixed. Timeless? The general idea is that for any non main thread you do want xpconnect to clear its things at thread exit. there is no other time/place to do it, because if the objects were created on a certain thread and aren't thread safe, they really need to be destroyed on that thread. Note that this code is all ancient, if you guys have added any events in the past 5 years, that's all long after this code was written. -- and yes, I know that currently xpconnect and gc don't behave properly wrt gc'ing threadunsafe objects, i had work related to that in a tree back in the states, I'm not sure whether I can find that or if I'll have to reimplement it, it wasn't a very big priority for me then and atm it really isn't -- iirc for a while nspr wasn't thread exiting the main thread (i think that might have been fixed relatively recently, I'm having some trouble keeping track of all the fixes people are doing). But yes, it's probably a good idea for xpconnect to at module shutdown clear things (need to be careful here of course, but hey).
Assignee | ||
Comment 71•15 years ago
|
||
Re-landed: js/src/jsapi.c 3.334 js/src/jsatom.h 3.68 js/src/jsfun.c 3.197 Of course, while testing and trying to debug, I pulled on a string and ended up updating my tree with a bunch of recent changes, one or more of which fixed the latent cycle-collector-related leak. No significant changes to fxdbug-linux-tbox's leak stats (the A number wandered up to a previous high water mark from before my landing). Wish I knew what fixed the shutdown leak this patch exposed, but if it stays fixed, we can move on and spend energy finding unfixed bugs. (However, I'm a little suspicious that my luck won't hold.) /be
Status: ASSIGNED → RESOLVED
Closed: 15 years ago → 15 years ago
Resolution: --- → FIXED
Updated•15 years ago
|
Flags: wanted1.9+
Whiteboard: [wanted-1.9]
Assignee | ||
Comment 73•14 years ago
|
||
The test for this bug is js1_8/extensions/regress-300079.js and it does use expression closures, but it really is testing the JS_CloneFunctionObject API, which has constraints I hadn't thought through when I developed the bug 452498 patches. Ulp. Anyway, test should probably use ES3-conformant function syntax and move to an API tests subdir, if we have one, or else whatever version (js1_x) in which we added JS_CloneFunctionObject to the API (JS1.5 IIRC -- check me). /be
Comment 74•14 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/001292143fde /cvsroot/mozilla/js/tests/js1_5/extensions/regress-300079.js,v <-- regress-300079.js initial revision: 1.1 Removing js/tests/js1_8/extensions/regress-300079.js; /cvsroot/mozilla/js/tests/js1_8/extensions/regress-300079.js,v <-- regress-300079.js new revision: delete; previous revision: 1.1
You need to log in
before you can comment on or make changes to this bug.
Description
•