334368 - Creating DOM events after layout shutdown not safe (crashes)

Status: RESOLVED WORKSFORME

(Core :: DOM: Events, defect, P5)

Description

[Boris Zbarsky [:bzbarsky]]

I'm not inside layout (in fact I'm in xmlextras).  The only way I have to create DOM events at the moment is from a document (via createEvent).  So that's what I do (in the patch for bug 198595):  create a document via DOMImplementation if I don't have one already, and then call createEvent on it.

I've run into the following crash as a result:

#6  0xb599346a in nsContentUtils::GetDocShellFromCaller ()
    at ../../../../mozilla/content/base/src/nsContentUtils.cpp:945
#7  0xb59ac23f in nsDOMImplementation::CreateDocument (this=0x818a8a0, 
    aNamespaceURI=@0xb7e9bb98, aQualifiedName=@0xb7e9bb98, aDoctype=0x0, 
    aReturn=0xbfffeb10) at ../../../../mozilla/content/base/src/nsDocument.cpp:624
...
#11 0xb4df82f3 in nsXMLHttpRequest::Error (this=0x83795e8, aEvent=0x0)
    at ../../../../../mozilla/extensions/xmlextras/base/src/nsXMLHttpRequest.cpp:1864
...
#16 0xb7e05e31 in nsInputStreamReadyEvent::EventHandler (plevent=0x83a468c)
    at ../../../mozilla/xpcom/io/nsStreamUtils.cpp:120
...
#19 0xb7e395b3 in nsEventQueueImpl::ProcessPendingEvents (this=0x80d3d28)
    at ../../../mozilla/xpcom/threads/nsEventQueue.cpp:419
#20 0xb7dc3f66 in NS_ShutdownXPCOM_P (servMgr=0x806d1dc)
    at ../../../mozilla/xpcom/build/nsXPComInit.cpp:702

So the problem is that the error event happens off shutdown, and in particular after layout has shut down.

I'm not sure how to go about fixing this, offhand, past adding a null-check here.  Maybe Benjamin's shutdown thing with windows will help... Not sure I have a ref to a window here, since mScriptContext is null.

Perhaps what we really need is a better way to create DOM events from outside the layout module?

Comment 1

[Benjamin Smedberg]

My layout-shutdown patch holds layout statics while there are any references to windows *or* documents. So the only way this could happen (with that patch) is if we created a *new* document after layout shuts down (which we should really make impossible).

Comment 2

[Benjamin Smedberg]

I missed where the ref to DOMImplementation comes from?

Comment 3

[Boris Zbarsky [:bzbarsky]]

Yes, I'm creating a new document.  That's the only way I can create an event.

The DOMImplementation is created via createInstance.

What would be nice is if we canceled network activity in a step before we start sending xpcom-shutdown... ;)

Comment 4

[Wayne Mery (:wsmwk)]

(In reply to comment #3)
> Yes, I'm creating a new document.  That's the only way I can create an event.
> 
> The DOMImplementation is created via createInstance.
> 
> What would be nice is if we canceled network activity in a step before we start
> sending xpcom-shutdown... ;)

bz, are you aware of whether this is still broke?

Comment 5

[Boris Zbarsky [:bzbarsky]]

It's not longer a problem in XMLHttpRequest, since that's inside gklayout now. But yes, we have no good way to create an event without a document right now...  nsEventDispatcher::CreateEvent is probably not exposed, right?

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Yeah, nsEventDispatcher::CreateEvent is gklayout only.

Comment 7

[Andrei Purice]

Hey Boris,
Is this crash still occurring for you?

Comment 8

[Andrew McCreight [:mccr8]]

Olli, is this a bug we want to leave open?

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

I guess this is WFM now.