462728 - crash when using alert from docloaderservice onStateChange listener [@ nsJARChannel::OnStartRequest]

Status: RESOLVED FIXED

(Core :: Networking: JAR, defect)

Description

[arno renevier]

Attachment: crashtest. needs to be executed with chrome privileges.

Hi,

when adding a progress listener on @mozilla.org/docloaderservice;1, and calling alert from onStateChange, mozilla crashes. Ok, I probably shouldn't do that, but mozilla also should not crash. See testcase.

Comment 1

[arno renevier]

#0  0xb7502c4c in nsJARChannel::OnStartRequest () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libxul.so
#1  0xb746e69c in nsInputStreamPump::OnStateStart () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libxul.so
#2  0xb746ea1f in nsInputStreamPump::OnInputStreamReady () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libxul.so
#3  0xb7bc8877 in nsInputStreamReadyEvent::Run () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libxul.so
#4  0xb7bdb6d4 in nsThread::ProcessNextEvent () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libxul.so
#5  0xb7bad7e4 in NS_ProcessNextEvent_P () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libxul.so
#6  0xb79f489b in nsXULWindow::ShowModal () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libxul.so
#7  0xb79f05bf in nsContentTreeOwner::ShowAsModal () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libxul.so
#8  0xb79d0d9a in nsWindowWatcher::OpenWindowJSInternal () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libxul.so
#9  0xb79d163e in nsWindowWatcher::OpenWindow () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libxul.so
#10 0xb79d2484 in nsPromptService::DoDialog () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libxul.so
#11 0xb79d32eb in nsPromptService::Alert () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libxul.so
#12 0xb79cc2db in nsPrompt::Alert () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libxul.so
#13 0xb78060f9 in nsGlobalWindow::Alert () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libxul.so
#14 0xb7be7503 in NS_InvokeByIndex_P () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libxul.so
#15 0xb7437f71 in XPCWrappedNative::CallMethod () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libxul.so
#16 0xb743f03c in XPC_WN_CallMethod () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libxul.so
#17 0xb7f993bc in js_Invoke () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libmozjs.so
#18 0xb7f88419 in js_Interpret () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libmozjs.so
#19 0xb7f9941d in js_Invoke () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libmozjs.so
#20 0xb74353e8 in nsXPCWrappedJSClass::CallMethod () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libxul.so
#21 0xb74311e5 in nsXPCWrappedJS::CallMethod () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libxul.so
#22 0xb7be800c in PrepareAndDispatch () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libxul.so
#23 0x00000003 in ?? ()
#24 0xb33cff80 in ?? ()
#25 0xbf9d26e4 in ?? ()
#26 0xb3360fa0 in ?? ()
#27 0x00000003 in ?? ()
#28 0x04040000 in ?? ()
#29 0xb79b4312 in nsDocLoader::QueryInterface () from /home/arno/mozilla/obj-i686-pc-linux-gnu/xulrunner/dist/bin/libxul.so
Backtrace stopped: previous frame inner to this frame (corrupt stack?)

Comment 2

[timeless]

this is another case of things not being safely reentrant.

i can give you a patch (or you can find someone to build a try-server build w/ it), but i suspect people won't like my fix as it's very much a bandaid, and the right thing probably requires arch work.

Comment 3

[timeless]

Attachment: look before leaping

Comment 4

[Tuukka Tolvanen (sp3000)]

http://mxr.mozilla.org/mozilla-central/source/modules/libjar/nsJARChannel.cpp?mark=691,696,698,828#668

Comment 5

[Boris Zbarsky [:bzbarsky]]

What I just said on IRC:

> either you constrain where channels are allowed to be open
> or you constrain the sorts of notifications necko will deliver
> or you constrain the things you can do in those notifications

We currently do the third, including no spinning of event loops.

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 347153 [details] [diff] [review]
look before leaping

This isn't going to produce the right results.

The right fix, I think, is to move these lines from AsyncOpen:
707     mListener = listener;
708     mListenerContext = ctx;
709     mIsPending = PR_TRUE;

to before the code that calls AsyncRead (and just reset those variables on failure)

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

(Maybe to elaborate on how this won't produce the right results: With the patch, in the situation described, the listener methods will never be called. That is incorrect behaviour)

Comment 8

[timeless]

Attachment: biesi's patch

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 365190 [details] [diff] [review]
biesi's patch

you need to set this in the else-branch as well, otherwise this would break remote jar: URIs

Comment 10

[timeless]

Attachment: final patch

so... having spent so much time on this, and actually having really written most of the lines, i've decided to record this patch as mine.

we tested remote jar: urls, and they work. of course the obvious way of testing (remote chrome) doesn't work, but that's unrelated (and biesi+i believe someone intentionally 'broke' that a while ago...)

Comment 11

[timeless]

http://hg.mozilla.org/mozilla-central/rev/4435f67c38e9

Comment 12

[Samuel Sidler (old account; do not CC)]

The stack in comment 1 looks just like the stack from bp-7e33866b-68b1-4e55-aa52-e23b22090505, which just happens to be the number 2 topcrash for Firefox 3.5b4. We should get this on the 1.9.1 branch.

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

a191=beltzner, but not blocking.

Comment 14

[Samuel Sidler (old account; do not CC)]

Can we get this landed on 1.9.1 asap so we can ensure that it fixes the topcrash?

Comment 15

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/10b548fd8c74

Comment 16

[chris hofmann]

I'm seeing what looks like very confusing comments and urls in the 3.5b4 crash data.   Can someone explain why this crash might be assoiciated with facebook apps and other web content?  I notice that the facebook apps want to suck in my facebook profile.  Are they using some signed script in a jar file to do that?

Are these actions around web content loading just innocent bystanders hanging around when chome UI is running into trouble and crashing?

nsJARChannel::OnStartRequest(nsIRequest*, nsISupports*) 0x0 http://apps.facebook.com/farmtown/play/ im testing the 3.5 beta and the private browsing is nice, but the freeze and crash rate is terrible. download time is increased as well as page loads in every area of the web that i use.

nsJARChannel::OnStartRequest(nsIRequest*, nsISupports*) 0x0 http://apps.facebook.com/restaurantcity/ why it is always crushing?????????????? it was so very irritated...

nsJARChannel::OnStartRequest(nsIRequest*, nsISupports*) 0x0 http://apps.facebook.com/restaurantcity/?pf_ref=sb please fix this this is disturbing\r\n

nsJARChannel::OnStartRequest(nsIRequest*, nsISupports*) 0x0 http://apps.facebook.com/restaurantcity/?pf_ref=sb why keep low memony and everytime the same problem,now everyday very
 lag,script stop.

This one is also strange

nsJARChannel::OnStartRequest(nsIRequest*, nsISupports*) 0x0 http://mail.live.com/[-----stuff removed---------] firefox quit abruptly when AVG antivirus started running

Comment 17

[chris hofmann]

btw, this is the most often commented on stack signature in all the incoming 3.5b4 crash reports over the last week; so we have info to dig deeper on if we think there might be more bugs to look at here.