Closed Bug 621446 Opened 14 years ago Closed 13 years ago

Crash [@ mozilla::net::HttpChannelParent::RecvRedirect2Verify ]

Categories

(Core :: Networking: HTTP, defect)

ARM
Android
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla15
Tracking Status
fennec - ---

People

(Reporter: jdm, Assigned: mayhemer)

References

(Blocks 1 open bug)

Details

(Keywords: crash, Whiteboard: [mobile-crash])

Crash Data

Attachments

(6 files, 2 obsolete files)

Signature mozilla::net::HttpChannelParent::RecvRedirect2Verify UUID 942948b3-19a1-467a-b718-ca65b2101226 Time 2010-12-26 04:52:25.734260 Uptime 29 Last Crash 45242 seconds (12.6 hours) before submission Install Age 93314 seconds (1.1 days) since version was first installed. Product Fennec Version 4.0b3 Build ID 20101221205132 Branch 1.9 OS Linux OS Version 0.0.0 Linux 2.6.32.9 #1 Mon Oct 25 17:39:35 KST 2010 armv7l CPU arm CPU Info Crash Reason SIGSEGV Crash Address 0x0 User Comments App Notes samsung GT-I9000 samsung/GT-I9000/GT-I9000/GT-I9000:2.2/FROYO/XXJPO:user/release-keys Processor Notes EMCheckCompatibility False Frame Module Signature [Expand] Source 0 libxul.so mozilla::net::HttpChannelParent::RecvRedirect2Verify netwerk/protocol/http/HttpChannelParent.cpp:346 1 libxul.so mozilla::net::PHttpChannelParent::OnMessageReceived PHttpChannelParent.cpp:623 2 libxul.so mozilla::dom::PContentParent::OnMessageReceived PContentParent.cpp:604 3 libxul.so mozilla::ipc::AsyncChannel::OnDispatchMessage ipc/glue/AsyncChannel.cpp:262 4 libxul.so mozilla::ipc::RPCChannel::OnMaybeDequeueOne ipc/glue/RPCChannel.cpp:440 5 libxul.so RunnableMethod<mozilla::ipc::RPCChannel, bool , Tuple0>::Run ipc/chromium/src/base/task.h:308 6 libxul.so mozilla::ipc::RPCChannel::DequeueTask::Run RPCChannel.h:475 7 libxul.so MessageLoop::RunTask ipc/chromium/src/base/message_loop.cc:344 8 libxul.so MessageLoop::DeferOrRunPendingTask ipc/chromium/src/base/message_loop.cc:354 9 libxul.so MessageLoop::DoWork ipc/chromium/src/base/message_loop.cc:451 10 libxul.so mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:115 11 libxul.so MessageLoop::RunInternal ipc/chromium/src/base/message_loop.cc:220 12 libxul.so MessageLoop::Run ipc/chromium/src/base/message_loop.cc:512 13 libxul.so nsBaseAppShell::Run widget/src/xpwidgets/nsBaseAppShell.cpp:198 14 libxul.so nsAppStartup::Run toolkit/components/startup/src/nsAppStartup.cpp:192 15 libxul.so XRE_main toolkit/xre/nsAppRunner.cpp:3693 16 libxul.so GeckoStart toolkit/xre/nsAndroidStartup.cpp:131 17 libc.so libc.so@0x111b3 18 libc.so libc.so@0x10ca3 Specifically, we're crashing here: 346 mRedirectCallback->OnRedirectVerifyCallback(result);
Josh, any STR or log from that session? Can you reproduce in any way and create a log? (best with attachment 496689 [details] [diff] [review] applied and NSPR_LOG_MODULES=timestamp,nsHttp:5,nsSocketTransport:5) Could that be caused by a redirect to a protocol different from http(s) ? Could you check mRedirectCallback value? I assume it is null, but just make sure it is.
Josh, ping for answers on comment 1, thanks.
Sorry, this isn't a crash that I received, it just came up in the daily crash reports I was looking at.
Thanks.
This is not that rare crash in Fennec. I think we should investigate.
tracking-fennec: --- → ?
All crashes I can find are happening on builds older then or including 2010-12-21. On 2010-11-23 I landed a fix for this crash. What's strange is that crash-stats lead to a file revision that already contains the fix. Changelog till the point crash-stats directs to: http://hg.mozilla.org/mozilla-central/shortlog/59591 Changelog till my fix: http://hg.mozilla.org/mozilla-central/shortlog/58111 Changelog the crashing build was made of could not be determined, the nightly build with reference to it has been deleted two days ago :(
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → DUPLICATE
https://crash-stats.mozilla.com/report/index/dfcb3858-e24c-4059-8f7c-dd4432110204 This crash has been seen in 4.0b4, so there's obviously some situation in which it can still occur.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
I didn't find this report. Thanks Josh, I'll try to figure it out.
Assignee: nobody → honzab.moz
Status: REOPENED → ASSIGNED
tracking-fennec: ? → 2.0+
Very low occurrences on crash-stats, but would like to get a fix in
tracking-fennec: 2.0+ → 2.0-
Crash Signature: [@ mozilla::net::HttpChannelParent::RecvRedirect2Verify ]
Also appearing significantly on mobile trunk now. A better list of crashes is https://crash-stats.mozilla.com/report/list?range_value=4&range_unit=weeks&signature=mozilla%3A%3Anet%3A%3AHttpChannelParent%3A%3ARecvRedirect2Verify - shows all crashes with this signature in the last 4 weeks.
This is a patch that before we would crash is trying to catch some states by PR_Abort'ing based on some added debug information.
Attachment #581409 - Flags: review?(jduell.mcbugs)
Comment on attachment 581409 [details] [diff] [review] investigation patch Review of attachment 581409 [details] [diff] [review]: ----------------------------------------------------------------- Is this crash still happening? I seem to get {0} results on the crash reports site when I search for mozilla::net::HttpChannelParent::RecvRedirect2Verify on fennec for the last 28 days. (But I've found it hard to actually locate crashes before). Obviously we'd expect this to have dropped off given that we're only using e10s on tablets now, but I would have expected some # of crashes. Anyway, if this is still happening, this is a reasonable way to proceed with debugging. ::: netwerk/protocol/http/HttpChannelParent.h @@ +139,5 @@ > RequestHeaderTuples *mHeadersToSyncToChild; > + > + bool mSentRedirect1Begin : 1; > + bool mSentRedirect1BeginFailed : 1; > + bool mReceviedRedirect2Verify : 1; misspelled "Received"
Attachment #581409 - Flags: review?(jduell.mcbugs) → review+
Status: ASSIGNED → RESOLVED
Closed: 14 years ago13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
This was not a fix. Just a debugging patch.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → ASSIGNED
Comment on attachment 581409 [details] [diff] [review] investigation patch [Approval Request Comment] Regression caused by (bug #): None User impact if declined: None Testing completed (on m-c, etc.): None needed, see bellow Risk to taking this patch (and alternatives if risky): None, see bellow This patch is trying to figure out a cause of an existing crash. If the crash would occur anyway, the patch instead aborts on a certain line (with optimization disabled) to indicate what is the state of the object: + if (!mRedirectCallback) { + // Bug 621446 investigation (optimization turned off above) + if (mReceviedRedirect2Verify) + ::PR_Abort(); + if (mSentRedirect1BeginFailed) + ::PR_Abort(); + if (mSentRedirect1Begin && NS_FAILED(result)) + ::PR_Abort(); + if (mSentRedirect1Begin && NS_SUCCEEDED(result)) + ::PR_Abort(); + if (!mRedirectChannel) + ::PR_Abort(); + } + + mReceviedRedirect2Verify = true; + > mRedirectCallback->OnRedirectVerifyCallback(result); On the > line the crash occurs. The reason I would like to get this landed on Aurora or Beta is to get the results sooner. The patch has so far landed on m-c (Fennec 12). Since actively is used only Beta and Release where the crash gets recorded, it would help identify the cause a bit sooner then after some 13 weeks to get the patch to Beta.
Attachment #581409 - Flags: approval-mozilla-beta?
Attachment #581409 - Flags: approval-mozilla-aurora?
OS: Linux → Android
Hardware: x86 → ARM
Severity: normal → critical
Adding #pragma GCC optimize ("O0") breaks the possibility to build Firefox 12 on Ubuntu. See https://bugs.launchpad.net/ubuntu/+source/firefox/+bug/918763
Comment on attachment 581409 [details] [diff] [review] investigation patch [Triage Comment] Approving for Aurora to speed up the investigation, but we're not going to take a patch for investigation at this point in FF10.
Attachment #581409 - Flags: approval-mozilla-beta?
Attachment #581409 - Flags: approval-mozilla-beta-
Attachment #581409 - Flags: approval-mozilla-aurora?
Attachment #581409 - Flags: approval-mozilla-aurora+
(In reply to Vivien Nicolas (:vingtetun) from comment #20) > Adding #pragma GCC optimize ("O0") breaks the possibility to build Firefox > 12 on Ubuntu. See > https://bugs.launchpad.net/ubuntu/+source/firefox/+bug/918763 Please see https://bugs.launchpad.net/ubuntu/+source/firefox/+bug/918763/comments/9
Depends on: 721502
I've figured out that using PR_Abort() was not the right way to invoke a reported crash with the correct signature. According Benjamin's advice I need to turn this to NS_RUNTIMEABORT.
Attachment #597971 - Flags: review?(jduell.mcbugs)
Attachment #597971 - Flags: review?(jduell.mcbugs) → review+
Comment on attachment 597971 [details] [diff] [review] Investigation patch update https://hg.mozilla.org/integration/mozilla-inbound/rev/5002199145a9 PLEASE LEAVE THE BUG OPEN, THIS IS ONLY FOR INVESTIGATION.
Attachment #597971 - Flags: checkin+
Whiteboard: [mobile-crash] → [mobile-crash] [leave open]
Target Milestone: mozilla12 → mozilla13
Comment on attachment 597971 [details] [diff] [review] Investigation patch update [Approval Request Comment] Please see comment 19 and comment 21 (request and approval). The reasons are the same for this update patch.
Attachment #597971 - Flags: approval-mozilla-aurora?
Still broken. On lucid, putting "//" in front of the #pragma GCC optimize ("O0") was sufficient for my build to continue. Lucid is using gcc-4.1.5.
Comment on attachment 597971 [details] [diff] [review] Investigation patch update [Triage Comment] Approved for Aurora 12.
Attachment #597971 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Crash Signature: [@ mozilla::net::HttpChannelParent::RecvRedirect2Verify ] → [@ mozilla::net::HttpChannelParent::RecvRedirect2Verify] [@ __libc_android_abort | PR_Abort]
This line, added in comment 23's cset... > #pragma warning(disable : 4068) ... is MSVC-specific, and it introduces the following build warning on GCC 4.6: > HttpChannelParent.cpp:336:0: warning: ignoring #pragma warning [-Wunknown-pragmas] Elsewhere in the codebase, we wrap this sort of #pragma in "#ifdef _MSC_VER" so that other compilers won't see it / complain about it. The attached followup patch does that here, as well.
Attachment #605532 - Flags: review?(jduell.mcbugs)
Attachment #605532 - Flags: review?(jduell.mcbugs) → review+
(In reply to Daniel Holbert [:dholbert] from comment #32) > Pushed followup to fix build warning: > http://hg.mozilla.org/integration/mozilla-inbound/rev/90f6b58ce0fa https://hg.mozilla.org/mozilla-central/rev/90f6b58ce0fa
Thanks, Daniel.
Attached patch Patch (obsolete) — Splinter Review
This patch do not use the #pragma GCC optimize("0O") directive if GCC <= 4.4. In practive this will not change anything for the investigation since people with such a version of GCC are commenting this line (or even removing it in the case of Ubuntu).
Attachment #606580 - Flags: review?(jduell.mcbugs)
Comment on attachment 606580 [details] [diff] [review] Patch Review of attachment 606580 [details] [diff] [review]: ----------------------------------------------------------------- We are able to compile B2G again in 10.04 with GCC 4.4.x with this patch
(In reply to Guillermo López (:willyaranda) from comment #36) > Comment on attachment 606580 [details] [diff] [review] > Patch > > Review of attachment 606580 [details] [diff] [review]: > ----------------------------------------------------------------- > > We are able to compile B2G again in 10.04 with GCC 4.4.x with this patch Doesn't work for me today with GCC 4.4.5. Could it be only for 64 bits?
(In reply to Vivien Nicolas (:vingtetun) from comment #37) > (In reply to Guillermo López (:willyaranda) from comment #36) > > Comment on attachment 606580 [details] [diff] [review] > > Patch > > > > Review of attachment 606580 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > We are able to compile B2G again in 10.04 with GCC 4.4.x with this patch > > Doesn't work for me today with GCC 4.4.5. Could it be only for 64 bits? We are compiling for 32bits, sorry :(
Comment on attachment 606580 [details] [diff] [review] Patch Review of attachment 606580 [details] [diff] [review]: ----------------------------------------------------------------- Looks fine. Honza, you may want to have a look if you know more about which gcc versions you want this to be applied on.
Attachment #606580 - Flags: review?(jduell.mcbugs) → review+
Version: unspecified → Trunk
Hm... I think the best would be to condition the investigation code to build just when building Fennec. This is not needed to build for desktop Firefox at all. However, if all platforms are now OK, then let's don't change this any more.
Blocks: 741104
(In reply to Guillermo López (:willyaranda) from comment #36) > attachment 606580 [details] [diff] [review] Thanks all! This attachment fixes bug 741104 (internal compiler error on Ubuntu 10.04 64bit) for me.
Attached patch Patch v0.2 (obsolete) — Splinter Review
This is the same patch as before except that it check for x86_64 since it looks like this is the only platform that fails.
Attachment #606580 - Attachment is obsolete: true
Attachment #611700 - Flags: review?(jduell.mcbugs)
Comment on attachment 611700 [details] [diff] [review] Patch v0.2 Review of attachment 611700 [details] [diff] [review]: ----------------------------------------------------------------- Looks fine to me, though I'm not an expert in compiler version macros. But I want Honza to sign off on this--he seemed to think we might want to instead limit usage only to fennec. I have no strong opinion on it.
Attachment #611700 - Flags: review?(jduell.mcbugs) → review?(honzab.moz)
Guys, do we have a MOZ_something macro for build of Fennec for Android? It would be simpler to just condition the optimization (and maybe the whole assertion(false) too) code under that #ifdef then to figure out on what platforms are the compiler options correct or not. This will be some 5th patch in this area of code that will be gone ones anyway...
Hey Honza, nsHostResolver used to have #ifdef ANDROID to change the values of some #defines for the Android build. AFAIK, it's all android, not just XUL or native builds.
Vivien, can you please use #ifdef ANDROID in your Patch v0.2?
People are seeing compiler errors on gcc 4.4 on desktop that are fixed by this patch, so I don't think we want to limit it to Android.
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #47) > People are seeing compiler errors on gcc 4.4 on desktop that are fixed by > this patch, so I don't think we want to limit it to Android. AIUI, he's suggesting to use the pragma on Android only, instead of enabling it on all platforms. However, it seems to me Vivien and others are having the problem when building B2G, and I think that defines ANDROID too.
Comment on attachment 611700 [details] [diff] [review] Patch v0.2 Review of attachment 611700 [details] [diff] [review]: ----------------------------------------------------------------- I suggest to wrap this to #ifdef ANDROID. Josh, I can do that my self if you are out of time. I messed this up.
Attachment #611700 - Flags: review?(honzab.moz)
> I suggest to wrap this to #ifdef ANDROID. Please don't. As said in comment 41, I run into this on desktop Ubuntu 64bit, too. The only way I could compile Firefox and Thunderbird was to apply that attachment.
Comment on attachment 611700 [details] [diff] [review] Patch v0.2 see above
Attachment #611700 - Flags: review?(honzab.moz)
Attachment #611700 - Flags: review?(honzab.moz)
(In reply to Ben Bucksch (:BenB) from comment #50) > > I suggest to wrap this to #ifdef ANDROID. > > Please don't. As said in comment 41, I run into this on desktop Ubuntu > 64bit, too. desktop Ubuntu 64bit defines ANDROID ?
To explain again: I want to wrap all my added experimental investigation code under #ifdef ANDROID since I search for the cause on Android devices only. Those ABORTS don't even need to be added on any other platform. At least, the optimization pragmas don't need.
> Review of attachment 611700 [details] [diff] [review] [diff] [review]: > I suggest to wrap this to #ifdef ANDROID. Honza, I guess I misunderstood your "this". As long as #pragma GCC optimize ("O0") is not applied on gcc 4.4, we should be fine, yes. I would suggest that you make a comment that this crashes gcc 4.4, though. So: +#ifdef ANDROID +// Compiling with a version of GCC <= 4.4 fails with an internal compiler +// error. #pragma GCC optimize ("O0") +#endif would be fine with me, yes.
(In reply to Ben Bucksch (:BenB) from comment #54) > > Review of attachment 611700 [details] [diff] [review]: > > I suggest to wrap this to #ifdef ANDROID. > > Honza, I guess I misunderstood your "this". > > As long as > #pragma GCC optimize ("O0") > is not applied on gcc 4.4, we should be fine, yes. I would suggest that you > make a comment that this crashes gcc 4.4, though. So: > > +#ifdef ANDROID > +// Compiling with a version of GCC <= 4.4 fails with an internal compiler > +// error. > #pragma GCC optimize ("O0") > +#endif > > would be fine with me, yes. That wouldn't be true, since android builds do use gcc 4.4.
Attachment #611700 - Attachment is obsolete: true
Attachment #612491 - Flags: review?(honzab.moz)
> Vivien, can you please use #ifdef ANDROID in your Patch v0.2? I did that. Honza, could you please more clear what exactly you want?
Comment on attachment 612496 [details] [diff] [review] gcc 4.4 internal compiler error, all pragmas on ANDROID only - Fix v4 Review of attachment 612496 [details] [diff] [review]: ----------------------------------------------------------------- ::: netwerk/protocol/http/HttpChannelParent.cpp @@ +343,3 @@ > #pragma GCC optimize ("O0") > +#endif > +#endif This will effectively disable the pragma on the builds Mozilla produces for Android, since they use gcc 4.4. This is the direct opposite of what we want.
Attachment #612496 - Flags: review?(honzab.moz) → review-
I don't know what you want. Can you please just make a patch that does what you want and fixes the compile on desktop? Thanks. All this "no, not that either" doesn't lead anywhere.
(In reply to Ben Bucksch (:BenB) from comment #60) > I don't know what you want. Can you please just make a patch that does what > you want and fixes the compile on desktop? Thanks. All this "no, not that > either" doesn't lead anywhere. I want nothing. It's not even clear to me what is failing and what is not. Are Vivien and other having problems building B2G building for a phone or desktop? (cf. comment 48)
Well, you said: > This is the direct opposite of what we want. My interest is described in its entirety in comment 41.
Okay, re-reading my message, I understand the "I don't know what you want." better. So here is my take: the pragma is there to avoid PR_Aborts being merged by the compiler, so that problems happening on Android can be debugged more accurately (knowing which PR_Abort is triggered). Anything that will effectively disable the pragma on our Android builds will make that moot. Now, until it is clear what fails and what doesn't with that pragma, it's hard to go further. Obviously, it's not "gcc 4.4", since Android builds with gcc 4.4.
On the other hand, if the investigation is only for Android, why is there a specific pragma for msvc ?
So, do I understand this correctly that you want to set #pragma GCC optimize ("O0") if and only if ANDROID or (!) gcc > 4.4. I.e. if not ANDROID, don't do -O0. If gcc <= 4.4, don't do -O0 ?
> I.e. if not ANDROID, don't do -O0. If gcc <= 4.4, don't do -O0 Ignore that, the last part is wrong. Is this what you want "if (and only if) ANDROID or (!) gcc > 4.4, set #pragma GCC optimize ("O0")" ?
(In reply to Ben Bucksch (:BenB) from comment #66) > > I.e. if not ANDROID, don't do -O0. If gcc <= 4.4, don't do -O0 > > Ignore that, the last part is wrong. Is this what you want > "if (and only if) ANDROID or (!) gcc > 4.4, set #pragma GCC optimize ("O0")" > ? Honza seems to want if ANDROID set pragma. What I'm saying in comment 48 is that I think B2G does have ANDROID when building for phones, and it's not clear whether that fails or not. If it does, then if ANDROID is not going to change anything for B2G. If it doesn't then fine.
(In reply to Mike Hommey [:glandium] from comment #67) > Honza seems to want if ANDROID set pragma. Yes. > What I'm saying in comment 48 is > that I think B2G does have ANDROID when building for phones, and it's not > clear whether that fails or not. If it does, then if ANDROID is not going to > change anything for B2G. If it doesn't then fine. Do we have anyone that could simply (i.e. with low effort) figure this out?
Comment on attachment 612491 [details] [diff] [review] gcc 4.4 internal compiler error - Fix v3 Review of attachment 612491 [details] [diff] [review]: ----------------------------------------------------------------- That's it! Thank you. ::: netwerk/protocol/http/HttpChannelParent.cpp @@ +335,4 @@ > // merged to a single address. > #ifdef _MSC_VER > #pragma warning(disable : 4068) > #endif You can probably remove this Windows specific part now. Please push this to try, no tests needed. (I can do that my self if you can't).
Attachment #612491 - Flags: review?(honzab.moz) → review+
(In reply to Honza Bambas (:mayhemer) from comment #68) > (In reply to Mike Hommey [:glandium] from comment #67) > > Honza seems to want if ANDROID set pragma. > > Yes. > > > What I'm saying in comment 48 is > > that I think B2G does have ANDROID when building for phones, and it's not > > clear whether that fails or not. If it does, then if ANDROID is not going to > > change anything for B2G. If it doesn't then fine. > > Do we have anyone that could simply (i.e. with low effort) figure this out? ANDROID is set: see http://mxr.mozilla.org/mozilla-central/source/configure.in#327 What you can use instead is MOZ_WIDGET_ANDROID.
Please help, i just downloaded the latest copy of firefox 12 and tried compiling on the Ubuntu 10.04 LTS (64-bit), and got the following errors: http://pastebin.com/J7wgvRNK Reading the source code's pragma leads me to this present bug thread, how can I continue to compile successfully?
https://crash-stats.mozilla.com/query/query?product=Fennec&product=FennecAndroid&version=ALL%3AALL&range_value=4&range_unit=weeks&date=04%2F18%2F2012+17%3A14%3A26&query_search=signature&query_type=contains&query=HttpChannelParent%3A%3ARecvRedirect2Verify&reason=&build_id=&process_type=any&hang_type=any&do_query=1 doesn't return anything helpful. Either NS_RUNTIMEABORT to force a crash that gets reported doesn't work or the bug has gone. I assume the former is more likely. If I don't get any useful data soon, I'll just add a null check and close this bug. It's inconsistent behavior, but better get a report of a broken page load we could one day investigate, then to let the application continuously crash.
Attached patch fix v1Splinter Review
- removes NS_RUNTIMEABORTS - leaves some of the investigation logic and provides log - adds a non-null check before call to mRedirectCallback (workaround)
Blocks: 745296
Comment on attachment 620774 [details] [diff] [review] fix v1 I remember I was asking for r here when attaching the patch... Jason will you have time to look at this? Quit simple...
Attachment #620774 - Flags: review?(jduell.mcbugs)
Attachment #620774 - Flags: review?(jduell.mcbugs) → review+
Whiteboard: [mobile-crash] [leave open] → [mobile-crash]
Target Milestone: mozilla13 → mozilla15
Status: ASSIGNED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: