180217 - nsXULDocument::~nsXULDocument will crash if mDocumentURL is null

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[timeless]

nsXULPrototypeCache::nsIURIKey::HashCode() line 135 + 21 bytes
nsHashtable::Remove(nsHashKey * 0x0012e1e8) line 322 + 11 bytes
nsSupportsHashtable::Remove(nsHashKey * 0x0012e1e8, nsISupports * * 0x00000000)
line 984 + 12 bytes
nsXULPrototypeCache::RemoveFromFastLoadSet(nsXULPrototypeCache * const
0x016df390, nsIURI * 0x00000000) line 631
nsXULDocument::~nsXULDocument() line 533
nsXULDocument::`scalar deleting destructor'() + 15 bytes
nsXULDocument::Release(nsXULDocument * const 0x01d4c3f0) line 575 + 186 bytes
XPCWrappedNative::~XPCWrappedNative() line 547 + 18 bytes
XPCWrappedNative::`scalar deleting destructor'(unsigned int 1) + 15 bytes
XPCWrappedNative::Release(XPCWrappedNative * const 0x01d4d6a0) line 777 + 147 bytes
XPCWrappedNative::FlatJSObjectFinalized(JSContext * 0x004f4e70, JSObject *
0x0141f310) line 897
XPC_WN_NoHelper_Finalize(JSContext * 0x004f4e70, JSObject * 0x0141f310) line 630
js_FinalizeObject(JSContext * 0x004f4e70, JSObject * 0x0141f310) line 1840 + 96
bytes
js_GC(JSContext * 0x004f4e70, unsigned int 5) line 1311 + 11 bytes

-	mDocumentURL	{...}
\+	mRawPtr	0x00000000

There is a real way this could happen in addition to the way i forced it to
happen, the real way is:
(you're really low on memory)
you call NS_NewXULDocument for the first time
    nsXULDocument* doc = new nsXULDocument();
succeeds
    if (NS_FAILED(rv = doc->Init())) {
Init fails.
        NS_RELEASE(doc);
Crash.

There are probably other ways this can fail. but I felt brendan would appreciate
a legitimate sequence.

Comment 1

[timeless]

Attachment: check mDocumentURL for null before trying to remove it from the hash

Comment 2

[Brendan Eich [:brendan]]

timeless, what was the fake (not 'legitimate') way that you made the crash happen?

/be

Comment 3

[Brendan Eich [:brendan]]

Comment on attachment 106276 [details] [diff] [review]
check mDocumentURL for null before trying to remove it from the hash

sr=[e-mail]

/be

Comment 4

[timeless]

const C=Components.classes, I=Components.interfaces;

and one of the following:

var o; for (a in C) if (!/dom|box/i.test(a)) try {o=C[a].getService(); for (i in
I) o instanceof I[i];} catch (e) {}

var o; for (a in C) if (!/dom|box/i.test(a)) try {o=C[a].createInstance(); for
(i in I) o instanceof I[i];} catch (e) {}

I don't remember which variation i was using at the time (and it doesn't
matter). it's also possible i didn't have the dom|box exclusion (which is
designed to save me from hundreds of asserts).  I didn't want to have a
discussion about the legitimacy of my method. There's almost always a real way
for the crashes that I've triggered to happen.

Comment 5

[Brendan Eich [:brendan]]

Timeless: cool, keep it up.  We should regression test such loops regularly. 
Cc'ing pschwartau.

/be

Comment 6

[David Bradley]

I think we really need to initialize mDocumentURL to null in the constructor as
well.

Comment 7

[David Bradley]

Comment on attachment 106276 [details] [diff] [review]
check mDocumentURL for null before trying to remove it from the hash

r=dbradley

Ugh, I was looking at nsDocument and not nsXULDocument

Comment 8

[timeless]

checked in

Comment 9

[Phil Schwartau]

I am still crashing in the current xpcshell on the loops in Comment #4.
Should we re-open this bug? Or are those tests too 'artificial'? 


WINNT STACK TRACE
nsGenericFactory::GetHelperForLanguage(nsGenericFactory * const 0x02cb67f4, 
unsigned int 2, nsISupports * * 0x0012d868) line 110 + 6 bytes
XPCWrappedNative::GatherProtoScriptableCreateInfo(nsIClassInfo * 0x02cb67f4, 
XPCNativeScriptableCreateInfo * 0x0012d9e4) line 565 + 38 bytes
XPCWrappedNative::GatherScriptableCreateInfo(nsISupports * 0x02cb67f0, 
nsIClassInfo * 0x02cb67f4, XPCNativeScriptableCreateInfo * 0x0012d9e4, 
XPCNativeScriptableCreateInfo * 0x0012d9d8) line 597 + 13 bytes
XPCWrappedNative::GetNewOrUsed(XPCCallContext & {...}, nsISupports * 0x02cb67f0, 
XPCWrappedNativeScope * 0x00a6d4b0, XPCNativeInterface * 0x00a4c1f0, 
XPCWrappedNative * * 0x0012da1c) line 281 + 61 bytes
XPCConvert::NativeInterface2JSObject(XPCCallContext & {...}, 
nsIXPConnectJSObjectHolder * * 0x0012db68, nsISupports * 0x02cb67f0, const nsID 
* 0x0012db8c, JSObject * 0x0104d950, unsigned int * 0x0012dadc) line 1059 + 30 
bytes
nsXPConnect::WrapNative(nsXPConnect * const 0x00a49630, JSContext * 0x00a5a860, 
JSObject * 0x0104d950, nsISupports * 0x02cb67f0, const nsID & {...}, 
nsIXPConnectJSObjectHolder * * 0x0012db68) line 565 + 29 bytes
nsJSCID::GetService(nsJSCID * const 0x02cb6a90, nsISupports * * 0x0012dd4c) line 
886 + 57 bytes
XPTC_InvokeByIndex(nsISupports * 0x02cb6a90, unsigned int 11, unsigned int 1, 
nsXPTCVariant * 0x0012dd4c) line 106
XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode 
CALL_METHOD) line 2016 + 42 bytes
XPC_WN_CallMethod(JSContext * 0x00a5a860, JSObject * 0x0104d950, unsigned int 0, 
long * 0x00c30e28, long * 0x0012e028) line 1283 + 14 bytes
js_Invoke(JSContext * 0x00a5a860, unsigned int 0, unsigned int 0) line 839 + 23 
bytes
js_Interpret(JSContext * 0x00a5a860, long * 0x0012fe50) line 2803 + 15 bytes
js_Execute(JSContext * 0x00a5a860, JSObject * 0x00c864c0, JSScript * 0x00a74b20, 
JSStackFrame * 0x00000000, unsigned int 0, long * 0x0012fe50) line 1020 + 13 
bytes
JS_ExecuteScript(JSContext * 0x00a5a860, JSObject * 0x00c864c0, JSScript * 
0x00a74b20, long * 0x0012fe50) line 3277 + 25 bytes
Process(JSContext * 0x00a5a860, JSObject * 0x00c864c0, char * 0x00000000, _iobuf 
* 0x1025a828 __iob) line 517 + 22 bytes
ProcessArgs(JSContext * 0x00a5a860, JSObject * 0x00c864c0, char * * 0x00a14524, 
int 0) line 655 + 33 bytes
main(int 0, char * * 0x00a14524) line 912 + 21 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77f1b9ea()


Function at crashpoint:

NS_IMETHODIMP nsGenericFactory::GetHelperForLanguage(PRUint32 language,
                                                     nsISupports **helper)
{
    if (mInfo->mGetLanguageHelperProc)  <<<--------------------- CRASHED HERE
        return mInfo->mGetLanguageHelperProc(language, helper);
    *helper = nsnull;
    return NS_OK;
}


At crashpoint, |mInfo| = 0x00000000