Closed
Bug 7698
Opened 26 years ago
Closed 26 years ago
CRASH in js_Enumerate after running some XPConnect code
Categories
(Core :: JavaScript Engine, defect, P3)
Tracking
()
VERIFIED
FIXED
M7
People
(Reporter: alecf, Assigned: jband_mozilla)
Details
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
I've gotten this same crash multiple times on different linux boxes (haven't gotten a chance to try windows yet) Here's what I'm trying to do: given an XPConnect object, I'm running this function on it: function describe(object, name) { for (var i in object) { var value = object[i]; if (typeof(object[i]) == "function") value = "[function]"; dump(name + "." + i + " = " + value + "\n"); } } When I run this on some XPConnected object (for instance nsIMsgIncomingServer) more than once (i.e. first time is fine) I get this crash, with what looks like the same stacktrace every time: #0 js_Enumerate (cx=0x8527588, obj=0x853bb58, enum_op=JSENUMERATE_DESTROY, statep=0xbfffee38, idp=0x0) at jsobj.c:2290 #1 0x4038e141 in prop_iterator_finalize (cx=0x8527588, obj=0x853bb58) at jsinterp.c:104 #2 0x4039d9ae in js_FinalizeObject (cx=0x8527588, obj=0x853bb58) at jsobj.c:1259 #3 0x4038d58c in js_GC (cx=0x8527588) at jsgc.c:835 #4 0x4038cff0 in js_ForceGC (cx=0x8527588) at jsgc.c:618 #5 0x40374fde in JS_GC (cx=0x8527588) at jsapi.c:867 #6 0x402f7b03 in nsJSContext::GC (this=0x85270a8) at nsJSEnvironment.cpp:328 #7 0x402fd181 in GlobalWindowImpl::SetNewDocument (this=0x8527528, aDocument=0x0) at nsGlobalWindow.cpp:257 #8 0x40d25493 in DocumentViewerImpl::~DocumentViewerImpl (this=0x8329840, __in_chrg=3) at nsDocumentViewer.cpp:225 #9 0x40d25307 in DocumentViewerImpl::Release (this=0x8329840) at nsDocumentViewer.cpp:184 #10 0x40279d0b in nsWebShell::Embed (this=0x84df340, aContentViewer=0x85c3f28, aCommand=0x85e0720 "view", aExtraInfo=0x0) at nsWebShell.cpp:755 #11 0x40277cf1 in nsDocumentBindInfo::OnStartBinding (this=0x84e05d0, aURL=0x85c2210, aContentType=0x85c3b38 "text/html") at nsDocLoader.cpp:1432 #12 0x4026272e in NET_NGLayoutConverter (format_out=38, converter_obj=0x0, URL_s=0x85c3580, context=0x85c22b0) at nsStubContext.cpp:949 #13 0x4024154b in NET_StreamBuilder (format_out=38, URL_s=0x85c3580, context=0x85c22b0) at mkstream.c:237 #14 0x4020f4f1 in NET_CacheConverter (format_out=38, converter_obj=0x0, URL_s=0x85c3580, window_id=0x85c22b0) at mkcache.c:1689 #15 0x4024154b in NET_StreamBuilder (format_out=102, URL_s=0x85c3580, context=0x85c22b0) at mkstream.c:237 #16 0x40180016 in net_finish_setup_http_stream (ce=0x85c23e8) at mkhttp.c:2877 #17 0x4017fb78 in net_setup_http_stream (ce=0x85c23e8) at mkhttp.c:2645 #18 0x40180c16 in net_ProcessHTTP (ce=0x85c23e8) at mkhttp.c:3558 #19 0x4023ac8b in NET_ProcessNet (ready_fd=0x85c25a0, fd_type=2) at mkgeturl.c:3355 #20 0x4024086f in NET_PollSockets () at mkselect.c:298 #21 0x4025d105 in nsNetlibService::NetPollSocketsCallback (aTimer=0x8527500, aClosure=0x80bde18) at nsNetService.cpp:1244 #22 0x400fa404 in TimerImpl::FireTimeout (this=0x8527500) at nsTimer.cpp:76 #23 0x400fa7d4 in nsTimerExpired (aCallData=0x8527500) at nsTimer.cpp:196 #24 0x405c1c81 in g_timeout_dispatch () #25 0x405c0e12 in g_main_dispatch () #26 0x405c140b in g_main_iterate () #27 0x405c15c1 in g_main_run () #28 0x404e6c4b in gtk_main () #29 0x400b3211 in nsAppShell::Run (this=0x809d9d0) at nsAppShell.cpp:197 #30 0x40017d72 in nsAppShellService::Run (this=0x8078d48) at nsAppShellService.cpp:402 #31 0x804a84f in main (argc=2, argv=0xbffff6b4) at nsAppRunner.cpp:514 The call to js_Enumerate looks like /* Fall through ... */ case JSENUMERATE_DESTROY: state = JSVAL_TO_PRIVATE(*statep); =>JS_DestroyIdArray(cx, state->ida); JS_free(cx, state); *statep = JSVAL_NULL; return JS_TRUE; and at this point "state" is "0xc" which is really wierd, but explains why dereferencing it dumps core.
Updated•26 years ago
|
Assignee: norris → jband
Assignee | ||
Updated•26 years ago
|
Component: XPConnect → Javascript Engine
Assignee | ||
Comment 2•26 years ago
|
||
This is really a JS Engine bug. The problem is that the iteration state holder code in the forinloop stuff in jsinterp.c only works right for objects that use the default JSObject enumeration code. The jsinterp.c code builds a propobj object of class prop_iterator_class to hold the iteration state. The idea is that if the forinloop terminates before the enumeration is done then the finalize of the propobj will call the enumerator callback with JSENUMERATE_DESTROY to have it clean up. But, this is broken. prop_iterator_finalize does... OBJ_ENUMERATE(cx, obj, JSENUMERATE_DESTROY, &iter_state, NULL); ... where 'obj' is the propobj itself. So, the default enumerator code gets called and *it* assumes that iter_state is an object of a particular shape (JSNativeIteratorState). Unfortunately, for xpconnect (and others?) who use their own enumeration code and their own state struct this causes havoc. So what should we do? The propobj has a reference to the 'iteratee' in a slot, but there is is no guarantee that it won't be finalized first (right?). We don't want to make the iteratee track its own iteration state(s) (to be free'd upon its own finalization). We don't want to leak these object's iteration states. I'm short on ideas that will cover all the bases and not be absurdly complex. Ideas?
Comment 3•26 years ago
|
||
The object being iterated over is stored in the propobj (in JSSLOT_PARENT). It seems like we could just change the critical line in prop_iterator_finalize: OBJ_ENUMERATE(cx, obj->slots[JSSLOT_PARENT], JSENUMERATE_DESTROY, &iter_state, NULL); Oh, but wait, that won't work: What if the object being iterated over has since become unreachable and has already been finalized ? + We don't want to make the iteratee track its own iteration state(s) (to be + free'd upon its own finalization). I suspect that this is exactly what we will need to do.
Assignee | ||
Comment 4•26 years ago
|
||
Well, we could root the iteratee slot in the propobj and then remove the root upon propobj finalization.
Assignee | ||
Comment 5•26 years ago
|
||
Comment 6•26 years ago
|
||
Testing iteratee != JSVAL_NULL isn't quite right -- if the object is stillborn (due to error around line 1333 in jsinterp.c), the slot will be JSVAL_VOID, not JSVAL_NULL. I don't see code that would set JSSLOT_PARENT to JSVAL_NULL, but if you want to defend, use JSVAL_IS_PRIMITIVE(iteratee) instead to rule out non-object-references. The rooting is annoying, but it's a sufficient fix. Given the JSIdArray malloc and iter_state malloc entailed by native for/in, I don't think that yet another malloc for a root hashtable entry will kill us. We should use double hashing anyway to avoid malloc per entry. /be
Comment 7•26 years ago
|
||
I think that brendan is correct and that a test for JSVAL_IS_VOID() is sufficient since I don't think iteratee can become NULL. JSVAL_IS_PRIMITIVE() is paranoia, but harmless. Other than that, I think this is a good fix.
Assignee | ||
Updated•26 years ago
|
Status: NEW → RESOLVED
Closed: 26 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•