run -installer hit new to create a new profile create profile, and hit finish crash. here's a patch to fix the problem: Index: jsstr.c =================================================================== RCS file: /cvsroot/mozilla/js/src/jsstr.c,v retrieving revision 3.12 diff -p -r3.12 jsstr.c *** jsstr.c 1999/09/28 23:10:55 3.12 --- jsstr.c 1999/09/30 18:00:03 *************** js_GetStringBytes(JSString *str) *** 2379,2384 **** --- 2379,2386 ---- JSHashNumber hash; JSHashEntry *he, **hep; + if (!str) return NULL; + JS_ACQUIRE_LOCK(deflated_string_cache_lock); cache = GetDeflatedStringCache(); here's the stack: #0 0x400c9896 in js_GetStringBytes (str=0x0) at jsstr.c:2394 #1 0x4007ac12 in js_ReportUncaughtException (cx=0x8381b38) at jsexn.c:655 #2 0x40057daa in JS_CallFunction (cx=0x8381b38, obj=0x841b160, fun=0x8537250, argc=1, argv=0xbfffeba4, rval=0xbfffea8c) at jsapi.c:2652 #3 0x403df04a in nsJSContext::CallFunction (this=0x8381618, aObj=0x841b160, aFunction=0x8537250, argc=1, argv=0xbfffeba4, aBoolResult=0xbfffeaf4) at nsJSEnvironment.cpp:228 #4 0x40417585 in nsJSEventListener::HandleEvent (this=0x8537488, aEvent=0x8579e08) at nsJSEventListener.cpp:103 #5 0x40c33840 in nsEventListenerManager::HandleEvent (this=0x8537088, aPresContext=@0x821e9b8, aEvent=0xbfffee98, aDOMEvent=0xbfffee40, aFlags=7, aEventStatus=@0xbffff1a8) at nsEventListenerManager.cpp:646 #6 0x408a50cd in ?? () from /builds/seth/seamonkey/mozilla/dist/bin/components/librdf.so #7 0x40c378c2 in nsEventStateManager::CheckForAndDispatchClick (this=0x8535d10, aPresContext=@0x821e9b8, aEvent=0xbffff288, aStatus=@0xbffff1a8) at nsEventStateManager.cpp:947 #8 0x40c36535 in nsEventStateManager::PostHandleEvent (this=0x8535d10, aPresContext=@0x821e9b8, aEvent=0xbffff288, aTargetFrame=0x851b828, aStatus=@0xbffff1a8, aView=0x8291548) at nsEventStateManager.cpp:418 #9 0x40c7d18d in PresShell::HandleEvent (this=0x82b09e0, aView=0x8291548, aEvent=0xbffff288, aEventStatus=@0xbffff1a8) at nsPresShell.cpp:2091 #10 0x41019499 in nsView::HandleEvent (this=0x8291548, event=0xbffff288, aEventFlags=28, aStatus=@0xbffff1a8, aHandled=@0xbffff14c) at nsView.cpp:827 #11 0x41023b23 in nsViewManager::DispatchEvent (this=0x84f4a30, aEvent=0xbffff288, aStatus=@0xbffff1a8) at nsViewManager.cpp:1662 #12 0x410175f4 in HandleEvent (aEvent=0xbffff288) at nsView.cpp:62 #13 0x40529a20 in ?? () from /builds/seth/seamonkey/mozilla/dist/bin/libwidget_gtk.so #14 0x405297bc in ?? () from /builds/seth/seamonkey/mozilla/dist/bin/libwidget_gtk.so #15 0x40529ae0 in ?? () from /builds/seth/seamonkey/mozilla/dist/bin/libwidget_gtk.so #16 0x4052ab7d in ?? () from /builds/seth/seamonkey/mozilla/dist/bin/libwidget_gtk.so #17 0x4052b746 in ?? () from /builds/seth/seamonkey/mozilla/dist/bin/libwidget_gtk.so #18 0x406b879d in ?? () from /usr/lib/libgtk-1.2.so.0 #19 0x40680037 in ?? () from /usr/lib/libgtk-1.2.so.0 #20 0x4067f52f in ?? () from /usr/lib/libgtk-1.2.so.0 #21 0x4067d800 in ?? () from /usr/lib/libgtk-1.2.so.0 #22 0x406b05b8 in ?? () from /usr/lib/libgtk-1.2.so.0 #23 0x406551a2 in ?? () from /usr/lib/libgtk-1.2.so.0 #24 0x406544da in ?? () from /usr/lib/libgtk-1.2.so.0 #25 0x406f7ab2 in ?? () from /usr/lib/libgdk-1.2.so.0 #26 0x407212c6 in ?? () from /usr/lib/libglib-1.2.so.0 #27 0x40721801 in ?? () from /usr/lib/libglib-1.2.so.0 #28 0x40721979 in ?? () from /usr/lib/libglib-1.2.so.0 #29 0x40653f3a in ?? () from /usr/lib/libgtk-1.2.so.0 #30 0x405144c9 in ?? () from /builds/seth/seamonkey/mozilla/dist/bin/libwidget_gtk.so #31 0x4038dee1 in nsAppShellService::Run (this=0x809edf0) at nsAppShellService.cpp:461 #32 0x409f77a6 in nsProfile::LoadDefaultProfileDir (this=0x811f698, profileURLStr=@0xbffff8a4) at nsProfile.cpp:245 #33 0x409f73d7 in nsProfile::StartupWithArgs (this=0x811f698, cmdLineArgs=0x8071678) at nsProfile.cpp:159 #34 0x804b22b in main1 (argc=2, argv=0xbffffa54) at nsAppRunner.cpp:548 #35 0x804b759 in main (argc=2, argv=0xbffffa54) at nsAppRunner.cpp:702 #36 0x4028bcb3 in ?? () from /lib/libc.so.6

Seth, shouldn't js_GetStringBytes assert str instead? NULL might be a reasonable value to return in a non-debug build, but I'm guessing that calling js_GetStringBytes with a NULL argument is an indicator of some other problem that we'd rather catch earlier rather than handle in this out of band way. (Also, a Brendan-nit as this is Brendan code; consistent formatting puts the if-body on a new line.)

JS_GetStringBytes should perhaps assert, but the patch is bogus. You'd just have to check the return value of JS_GetStringBytes instead of whatever is giving you that NULL pointer. If a JSAPI function is returning a NULL |JSString *|, it's probably telling you that something went wrong.

So hang on a sec: what is js_ReportUncaughtException doing calling it with a NULL pointer? bad code! bad!

accepting. marking all milestone 11.

what's wrong with bulletproofing js_GetStringBytes() to not crash if called with null. Something bad is probably happening, and after checking in the bulletproofing, I can re-assign this bug to a JS person for inspection. crashing is never a good thing.

a different patch, but the same idea: prevent js_GetStringBytes() from crashing, if it was passed a null. Index: jsstr.c =================================================================== RCS file: /cvsroot/mozilla/js/src/jsstr.c,v retrieving revision 3.12 diff -r3.12 jsstr.c 2381a2382 > 2394,2401c2395,2396 < bytes = js_DeflateString(NULL, str->chars, str->length); < if (bytes) { < if (JS_HashTableRawAdd(cache, hep, hash, str, bytes)) { < deflated_string_cache_bytes += str->length; < } else { < free(bytes); < bytes = NULL; < } --- > if (!str) { > bytes = NULL; 2402a2398,2408 > else { > bytes = js_DeflateString(NULL, str->chars, str->length); > if (bytes) { > if (JS_HashTableRawAdd(cache, hep, hash, str, bytes)) { > deflated_string_cache_bytes += str->length; > } else { > free(bytes); > bytes = NULL; > } > } > } can I get one you js guys to review this, and then take over the bug and see why js_GetStringBytes() was getting called with null?

Sorry, my code (it was once) does not null-check at every damn layer, just in case someone screwed up a layer above, when all layers are within the JS engine. That way lies performance death by a thousand cuts, and code bloat, and mental slackness (no offense to sspitzer, who is new on scene). All js_* routines are internal to the engine. JS_* routines may well null-check as they are relatively infrequent API calls -- but high-frequency ones may not, again to hoist the stupidity tax out to callers. In this case, the bug is in js_ReportUncaughtException, and belongs to mccabe. Why didn't he take it? Here it comes! /be

fair enough. you JS guys run a tight ship. The bug still exists (at least on Linux). To reproduce, do this: $ mv ~/.mozilla ~/.mozilla-old $ ./mozilla-apprunner.sh hit the new button to bring up the create profile wizard hit next, then hit finished crash.

Roger Lawrence is doing exception goo currently. Roger, to you... As far as not crashing on NULL... Silently accepting NULL and returning is a dual behavior, and a mistake. Functions are best designed to do *one* thing. Yes, maybe we shouldn't crash in this situation in a shipping product, but this situation should be loudly flagged to developers. Crashing is one way to raise this flag.

rogerl, if you need help getting set up to reproduce this, give me a call at x4129. you can come over to my cube and we can debug it.

The problem is that damn null string - it "shouldn't" happen since it's origin is a 'toString' call on an exception object. sspitzer - thanks for the offer, if i can't reproduce it I'll stop by.

*** Bug 15197 has been marked as a duplicate of this bug. ***

*** Bug 15075 has been marked as a duplicate of this bug. ***

An exception object is being created to reflect a JS error that has occurred but the context that has been passed in is missing the exception prototype objects - so the 'toString' method is not found and a null string created and ... I don't understand why the context is missing the exception prototypes, they're created as part of InitStandardClasses along with all the other native stuff and yet are not available when the error occurs. The contxet being passed in looks fine in all other regards. Is there any chance that a 'non-standard' context is being used?

fixed. vidur's patch has been checked in with rogerl's blessing.

builds for 19991007 Win and Linux no more crashing!

I want to re-open this since I think another bug is being hidden by the patch. What's happenning is that the global object in the context being used to execute the 'onclick' script has already been blown away by clearScope called from : JS_ClearScope(JSContext * 0x01fe22e0, JSObject * 0x01c75960) line 2034 + 17 bytes GlobalWindowImpl::SetNewDocument(GlobalWindowImpl * const 0x01fe24a4, nsIDOMDocument * 0x00000000) line 286 + 32 bytes DocumentViewerImpl::~DocumentViewerImpl() line 259 DocumentViewerImpl::`scalar deleting destructor'(unsigned int 0x00000001) + 15 bytes DocumentViewerImpl::Release(DocumentViewerImpl * const 0x0200c1d0) line 217 + 99 bytes nsWebShell::Destroy(nsWebShell * const 0x01fe1330) line 1161 + 27 bytes nsWebShellWindow::Close(nsWebShellWindow * const 0x01fe18b0) line 454 GlobalWindowImpl::Close(GlobalWindowImpl * const 0x01fe24a8) line 1260 nsAppShellService::Quit(nsAppShellService * const 0x00d232b0) line 488 XPTC_InvokeByIndex(nsISupports * 0x00d232b0, unsigned int 0x00000005, unsigned int 0x00000000, nsXPTCVariant * 0x0012d0cc) line 135 nsXPCWrappedNativeClass::CallWrappedMethod(JSContext * 0x01fe22e0, nsXPCWrappedNative * 0x01eaa930, const XPCNativeMemberDescriptor * 0x01ead1dc, nsXPCWrappedNativeClass::CallMode CALL_METHOD, unsigned int 0x00000000, long * 0x01cf70e4, long * 0x0012d26c) line 767 + 44 bytes WrappedNative_CallMethod(JSContext * 0x01fe22e0, JSObject * 0x01d26b48, unsigned int 0x00000000, long * 0x01cf70e4, long * 0x0012d26c) line 186 + 34 bytes js_Invoke(JSContext * 0x01fe22e0, unsigned int 0x00000000, unsigned int 0x00000000) line 672 + 26 bytes js_Interpret(JSContext * 0x01fe22e0, long * 0x0012dae4) line 2248 + 15 bytes js_Invoke(JSContext * 0x01fe22e0, unsigned int 0x00000000, unsigned int 0x00000000) line 688 + 13 bytes js_Interpret(JSContext * 0x01fe22e0, long * 0x0012e318) line 2248 + 15 bytes js_Invoke(JSContext * 0x01fe22e0, unsigned int 0x00000000, unsigned int 0x00000000) line 688 + 13 bytes js_Interpret(JSContext * 0x01fe22e0, long * 0x0012eb4c) line 2248 + 15 bytes js_Invoke(JSContext * 0x01fe22e0, unsigned int 0x00000001, unsigned int 0x00000000) line 688 + 13 bytes js_Interpret(JSContext * 0x01fe22e0, long * 0x0012f380) line 2248 + 15 bytes js_Invoke(JSContext * 0x01fe22e0, unsigned int 0x00000001, unsigned int 0x00000002) line 688 + 13 bytes js_InternalCall(JSContext * 0x01fe22e0, JSObject * 0x01c74d98, long 0x01c74da0, unsigned int 0x00000001, long * 0x0012f500, long * 0x0012f4b8) line 765 + 15 bytes JS_CallFunction(JSContext * 0x01fe22e0, JSObject * 0x01c74d98, JSFunction * 0x0201f7f0, unsigned int 0x00000001, long * 0x0012f500, long * 0x0012f4b8) line 2650 + 32 bytes nsJSContext::CallFunction(nsJSContext * const 0x01fe2450, void * 0x01c74d98, void * 0x0201f7f0, unsigned int 0x00000001, void * 0x0012f500, int * 0x0012f4fc) line 231 + 39 bytes nsJSEventListener::HandleEvent(nsIDOMEvent * 0x02108a80) line 103 + 48 bytes nsEventListenerManager::HandleEvent(nsIPresContext & {...}, nsEvent * 0x0012f7f8, nsIDOMEvent * * 0x0012f7c0, unsigned int 0x00000007, nsEventStatus & nsEventStatus_eIgnore) line 646 + 21 bytes RDFElementImpl::HandleDOMEvent(RDFElementImpl * const 0x0201ff90, nsIPresContext & {...}, nsEvent * 0x0012f7f8, nsIDOMEvent * * 0x0012f7c0, unsigned int 0x00000001, nsEventStatus & nsEventStatus_eIgnore) line 2894 nsEventStateManager::CheckForAndDispatchClick(nsEventStateManager * const 0x02023b20, nsIPresContext & {...}, nsMouseEvent * 0x0012fb6c, nsEventStatus & nsEventStatus_eIgnore) line 1003 + 42 bytes nsEventStateManager::PostHandleEvent(nsEventStateManager * const 0x02023b20, nsIPresContext & {...}, nsGUIEvent * 0x0012fb6c, nsIFrame * 0x02027ec0, nsEventStatus & nsEventStatus_eIgnore, nsIView * 0x0200ea60) line 474 + 24 bytes PresShell::HandleEvent(PresShell * const 0x0200e604, nsIView * 0x0200ea60, nsGUIEvent * 0x0012fb6c, nsEventStatus & nsEventStatus_eIgnore) line 2102 + 43 bytes nsView::HandleEvent(nsView * const 0x0200ea60, nsGUIEvent * 0x0012fb6c, unsigned int 0x0000001c, nsEventStatus & nsEventStatus_eIgnore, int & 0x00000000) line 834 nsViewManager::DispatchEvent(nsViewManager * const 0x0200eb40, nsGUIEvent * 0x0012fb6c, nsEventStatus & nsEventStatus_eIgnore) line 1670 HandleEvent(nsGUIEvent * 0x0012fb6c) line 63 nsWindow::DispatchEvent(nsWindow * const 0x0200e924, nsGUIEvent * 0x0012fb6c, nsEventStatus & nsEventStatus_eIgnore) line 342 + 10 bytes nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012fb6c) line 363 nsWindow::DispatchMouseEvent(unsigned int 0x0000012d, nsPoint * 0x00000000) line 3306 + 21 bytes ChildWindow::DispatchMouseEvent(unsigned int 0x0000012d, nsPoint * 0x00000000) line 3524 nsWindow::ProcessMessage(unsigned int 0x00000202, unsigned int 0x00000000, long 0x01870078, long * 0x0012fd90) line 2533 + 24 bytes nsWindow::WindowProc(HWND__ * 0x033f0884, unsigned int 0x00000202, unsigned int 0x00000000, long 0x01870078) line 520 + 27 bytes USER32! 77e71250()

Clearing FIXED resolution due to reopen of this bug.

Adding vidur. It looks like the event handler is quitting the app, which takes down the JS environment of the running handler. Maybe there's a way to avoid clearing the window scope until after the handler finishes? /be

Well...that was the question that I asked you in 15197, Brendan. Currently, we're executing window.close() (and, hence, JS_ClearScope) synchronously. Synchronous execution was probably not the case in 4.x because of the threading model. It is the case in 4.x, however, that any script after the window.close() doesn't execute (e.g. window.alert right after the close doesn't seem to execute). It may be possible, with some work, to defer the actual closing of the window (and, hence, the call to JS_ClearScope) till after script execution, but I'm not sure that's correct either. In the case of 15197, there was a call to setTimeout immediately after the window.close(). It turns out that the call was inadvertant and really shouldn't have been there. An exception was thrown since setTimeout no longer existed. The patch that Roger checked in allowed the exception to be forwarded to the error reporter successfully. It's unclear to me that this is worse than defering the window closure and actually executing the setTimeout.

See http://lxr.mozilla.org/mozilla/source/lib/libmocha/lm_win.c#1418 for how MozillaClassic solved this. The next statement after window.close does not fail to run in 4.x, for me: <script> foo=1 window.open("javascript:'<body onload=self.close();opener.foo=3><\/body>'") </script> <body onload='document.form.fooval.value = foo'> <form name=form> <input name=fooval size=4> <input type=button onclick='document.form.fooval.value = foo'> </form> </body> If you click the button after the opened window has closed itself, you'll see that foo was set to 3. If you're using alerts to trace execution, beware 4.x does not put up an alert for a window being torn down, IIRC. It seems to me we should fix this in Mozilla 5 using a setTimeout-like technique to defer the window's destruction till after the handler. It might be simpler and safer to just set a flag that you check on end of script evaluation, though -- what do you say? /be

Ugh...the determination that the window.close() came from script (and from there the determination that it's script in the same window) will be a treat. Other than that, either setting or a timeout like mechanism could work.

Could javascript's window.close() bind to a function that helps resolve those problems (or augment whatever it currently calls)? Maybe pass along a window handle or something?

Added dogfood and PDT+ annotations.

Fix checked in on 10/28/1999. If the calling context of a call to window.close() is the same as the window's script context, the actual closure is defered till the script completes. This is done by setting a termination function for the script context.

changing caps in summary.

build 1999110508

Moving all Profile Manager bugs to new Profile Manager Backend component. Profile Manager component to be deleted.