Closed Bug 462728 Opened 12 years ago Closed 12 years ago

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

Categories

(Core :: Networking: JAR, defect)

x86
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: arno, Assigned: timeless)

Details

(Keywords: crash, fixed1.9.1, topcrash)

Crash Data

Attachments

(2 files, 2 obsolete files)

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.
#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?)
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.
Severity: normal → critical
Keywords: crash
Summary: crash when using alert from docloaderservice onStateChange listener. → crash when using alert from docloaderservice onStateChange listener [@ nsJARChannel::OnStartRequest]
Attached patch look before leaping (obsolete) — Splinter Review
Attachment #347153 - Flags: review?(cbiesinger)
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 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)
Attachment #347153 - Flags: review?(cbiesinger) → review-
(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)
Attached patch biesi's patch (obsolete) — Splinter Review
Assignee: nobody → timeless
Attachment #347153 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #365190 - Flags: superreview?(cbiesinger)
Attachment #365190 - Flags: review+
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
Attachment #365190 - Flags: superreview?(cbiesinger) → superreview+
Attached patch final patchSplinter Review
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...)
Attachment #365190 - Attachment is obsolete: true
Attachment #365630 - Flags: review?(cbiesinger)
Attachment #365630 - Flags: review?(cbiesinger) → review+
http://hg.mozilla.org/mozilla-central/rev/4435f67c38e9
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
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.
Flags: blocking1.9.1?
Keywords: topcrash
a191=beltzner, but not blocking.
Flags: blocking1.9.1? → blocking1.9.1-
Flags: in-testsuite?
Can we get this landed on 1.9.1 asap so we can ensure that it fixes the topcrash?
Keywords: checkin-needed
Whiteboard: [needs 1.9.1 landing]
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
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.
Crash Signature: [@ nsJARChannel::OnStartRequest]
You need to log in before you can comment on or make changes to this bug.