Closed Bug 57840 Opened 25 years ago Closed 25 years ago

Crash clicking "What's related" in sidebar [@ nsHTTPChannel::Redirect]

Categories

(SeaMonkey :: Sidebar, defect, P1)

Product:
SeaMonkey
Component:
Sidebar
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: scalkins, Assigned: morse)

References

Details

(Keywords: crash, smoketest, topcrash, Whiteboard: [rtm need info])

Crash Data

Saw this on mac 2000-10-24-08MN6 branch I have OS9.04 on a G3/400mhz 128mb ram Steps to repro: 1)Launch seamonkey and click on any tab other than What's Related 2) Now click on the 'What's related' tab Actual results: You will crash out of seamonkey Nominating rtm for possible data loss and poor first impression. Talkback Incident ID: 19691647 follows: Trigger Type: Program Crash Trigger Reason: PowerPC unmapped memory exception Call Stack: (Signature = .__ptr_glue 3385cc9f) .__ptr_glue nsHTTPChannel::ProcessRedirection() [nsHTTPChannel.cpp, line 2633] nsHTTPChannel::ProcessStatusCode() [nsHTTPChannel.cpp, line 2480] nsHTTPChannel::FinishedResponseHeaders() [nsHTTPChannel.cpp, line 2341] nsHTTPServerListener::FinishedResponseHeaders() [nsHTTPResponseListener.cpp, line 1036] nsHTTPServerListener::OnDataAvailable() [nsHTTPResponseListener.cpp, line 427] <../images/spacer.gif> Registers: CTR LR PC CR XER MSR 0x171b1c80 0x16eba650 0x16edbd34 0x22000828 0x00000000 0x00000000 General Registers R0 0x00000000 R16 0x06fdabcc (sp) R1 0x07756430 R17 0x00000000 (rtoc) R2 0x06fce110 R18 0x06fdbe78 R3 0x00000000 R19 0x0000012e R4 0x07756674 R20 0x07756784 R5 0x000008ac R21 0x0676cc4c R6 0x06fd47d4 R22 0x06fd42fc R7 0x06fd4774 R23 0x077566cc R8 0x06fd60d8 R24 0x00000000 R9 0x06fd40f4 R25 0x00000000 R10 0x06fd7908 R26 0x00000000 R11 0x06fd4064 R27 0x00000000 R12 0x4a291400 R28 0x0676dc1c R13 0x00000000 R29 0x00010000 R14 0x0642e894 R30 0x0656ccf8 R15 0x0676cbe0 R31 0x0642e894 Floating Point Registers FR0 0x2f6677642f736964 FR16 0x0000000000000000 FR1 0x0775648007756510 FR17 0x0000000000000000 FR2 0x0775648006753390 FR18 0x0000000000000000 FR3 0x0677ecf000000010 FR19 0x0000000000000000 FR4 0x07756450068bfacc FR20 0x0000000000000000 FR5 0x172f37340775649c FR21 0x0000000000000000 FR6 0x0775646093000000 FR22 0x0000000000000000 FR7 0x171aef1c062a4068 FR23 0x0000000000000000 FR8 0x077564b006753390 FR24 0x0000000000000000 FR9 0x0676c5e400000028 FR25 0x0000000000000000 FR10 0x07756490067689d0 FR26 0x0000000000000000 FR11 0x06e70eb8062a4068 FR27 0x0000000000000000 FR12 0x07756490068bfacc FR28 0x0000000000000000 FR13 0x172aec6c06f10a48 FR29 0x0000000000000000 FR14 0x0000000000000000 FR30 0x0000000000000000 FR15 0x0000000000000000 FR31 0x0000000000000000 FPSCR 0xa2002100 Exception kind: PowerPC unmapped memory exception Area: 0x07755f75 Address: 0x4a291400 Error: 0x07755f5c Reference: 0x00000001
I also see this on Linux todays branch build 2000-10-24-09MN6
Keywords: crash, rtm
OS: Mac System 9.x → All
Hardware: Macintosh → All
*** Bug 57837 has been marked as a duplicate of this bug. ***
adding smoketest blocker status of duplicate bug. the branch is being held closed for this bug.
Severity: critical → blocker
Keywords: smoketest
On my build it's not crashing for me so i'm having a hell of a time reproducing it with a debug build.
Does this happen on the trunk as well?
Priority: P3 → P1
OK ... apparently it DOESN'T happen on the trunk. Why?
obviously this should be need info ...
Whiteboard: [rtm need info]
I could reproduce this on mac/linux branch bits but did not crash on windows branch.
This crashes for me on windows commercial release branch.
according to don & PDT, branch is open. if we don't get traction on this by 9pm tonight, back it out.
back what out?
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/protocol/http/src/ns HTTPChannel.cpp This is crashing in nsHTTPCHannel.cpp on line 1770 because we are not checking for null on the mLoadGroup and related links does not use load groups. This is crashing on the branch. This looks like morse added this code when fixing a bug and forgot to do this.
Assignee: matt → morse
From what was said, the patch is obvious. But I can't test it because all I have at the moment is a windows debug branch build. I can get a linux debug branch build to try it on, but that will take a while. In the meantime, can someone who does have a build that is failing try out the following patch and let me know if it clears it up. Index: nsHTTPChannel.cpp =================================================================== RCS file: /cvsroot/mozilla/netwerk/protocol/http/src/nsHTTPChannel.cpp,v retrieving revision 1.234.2.6 diff -u -r1.234.2.6 nsHTTPChannel.cpp --- nsHTTPChannel.cpp 2000/10/20 03:15:07 1.234.2.6 +++ nsHTTPChannel.cpp 2000/10/25 01:06:09 @@ -1767,10 +1767,12 @@ if (NS_FAILED(rv)) return rv; nsCOMPtr<nsIChannel> tempChannel; - rv = mLoadGroup->GetDefaultLoadChannel(getter_AddRefs(tempChannel)); - if (NS_SUCCEEDED(rv)) { - if (tempChannel == this) { - mLoadGroup->SetDefaultLoadChannel(channel); + if (mLoadGroup) { + rv = mLoadGroup->GetDefaultLoadChannel(getter_AddRefs(tempChannel)); + if (NS_SUCCEEDED(rv)) { + if (tempChannel == this) { + mLoadGroup->SetDefaultLoadChannel(channel); + } } }
Status: NEW → ASSIGNED
Also, gagan, can you please review this patch.
the declaration of tempChannel should move to inside the check so that you don't create a temporary in this case.
Gagan, good catch. Here's the revised patch. Please review. Thanks. Index: nsHTTPChannel.cpp =================================================================== RCS file: /cvsroot/mozilla/netwerk/protocol/http/src/nsHTTPChannel.cpp,v retrieving revision 1.234.2.6 diff -u -r1.234.2.6 nsHTTPChannel.cpp --- nsHTTPChannel.cpp 2000/10/20 03:15:07 1.234.2.6 +++ nsHTTPChannel.cpp 2000/10/25 02:57:56 @@ -1766,11 +1766,13 @@ rv = channel->SetOriginalURI(mOriginalURI); if (NS_FAILED(rv)) return rv; - nsCOMPtr<nsIChannel> tempChannel; - rv = mLoadGroup->GetDefaultLoadChannel(getter_AddRefs(tempChannel)); - if (NS_SUCCEEDED(rv)) { - if (tempChannel == this) { - mLoadGroup->SetDefaultLoadChannel(channel); + if (mLoadGroup) { + nsCOMPtr<nsIChannel> tempChannel; + rv = mLoadGroup->GetDefaultLoadChannel(getter_AddRefs(tempChannel)); + if (NS_SUCCEEDED(rv)) { + if (tempChannel == this) { + mLoadGroup->SetDefaultLoadChannel(channel); + } } }
My linux branch debug build just finished and I tested. Without applying the patch, I am unable to reproduce the crash. I've now tried this on both windows and linux. So I'm going to need somebody who can reproduce the crash to test the patch and see if it clears things up.
Steve, I'll find somebody to help. When you do get verification that the patch fixes the problem, then check it in since it doesn't need a "++" because it's a tree blocker.
Just for clarification of anybody looking at the above patch. All I added was the line: if (mLoadGroup) { and its corresponding close brace. All the other diffs noted are due to change in indentation.
OK, I'm gonna give this an "r=don". I can't see from the context of the function how that check for null would cause more harm.
However, the only verification I have that it fixes the real problem is the debugging session that matt, rjc, and gagan had yesterday.
OK, checked in fix per don's comments. Never did get verification that it fixes the problem (other than the debugging session cited) but don agrees that it can't possibly make things worse. No sence holding the tree hostage for this.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
I am still seeing this on branch builds for Mac and Windows today....am just starting linux.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
What branch build are you using. The change was checked in at 11 AM. Does the build that you are using include changes made only an hour ago?
Why were the checkin rules bypassed? Or did someone super-review this and not record his stamp of approval? Null checks should be safe, but if they wallpaper over a problem that will bite anyway, they're the wrong fix. See bug 56570 for an example. At least here the pointer is a strong ref (nsCOMPtr), so it won't point at freed memory. Cc'ing mscott and rpotts for help getting to the bottom of this. /be
So far all we have for branch builds are 8 or 9am builds from this am...so maybe the next respin will have this fix? These builds still have the problem. At this writing current branch builds are: Win 2000-10-25-09MN6 Linux 2000-10-25-09MN6 Mac 2000-10-25-08MN6
In that case I'm closing this out once again as fixed. Please reopen if verification fails for a build that includes the patch.
Status: REOPENED → RESOLVED
Closed: 25 years ago → 25 years ago
Resolution: --- → FIXED
I don't think this is fixed. Here is the latest stack trace (after cliking on what's related). 20001025's build. Platform : Winnt. Reopeneing the bug; Stack trace : nsHTTPChannel::Redirect(const char * 0x051d6fa0, nsIChannel * * 0x0012bd08, int 302) line 1770 + 53 bytes nsHTTPChannel::ProcessRedirection(int 302) line 2633 + 46 bytes nsHTTPChannel::ProcessStatusCode() line 2480 + 15 bytes nsHTTPChannel::FinishedResponseHeaders() line 2341 + 8 bytes nsHTTPServerListener::FinishedResponseHeaders() line 1040 + 11 bytes nsHTTPServerListener::OnDataAvailable(nsHTTPServerListener * const 0x051d5e20, nsIChannel * 0x0516a414, nsISupports * 0x051535e0, nsIInputStream * 0x051d6e10, unsigned int 0, unsigned int 0) line 427 + 8 bytes nsOnDataAvailableEvent::HandleEvent(nsOnDataAvailableEvent * const 0x051d1720) line 400 + 47 bytes nsStreamListenerEvent::HandlePLEvent(PLEvent * 0x051d1f80) line 97 + 12 bytes PL_HandleEvent(PLEvent * 0x051d1f80) line 580 + 10 bytes PL_ProcessPendingEvents(PLEventQueue * 0x00aa8cd0) line 513 + 9 bytes _md_EventReceiverProc(HWND__ * 0x07640362, unsigned int 49483, unsigned int 0, long 11177168) line 1049 + 9 bytes USER32! 77e71820() 00aa8cd0()
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Adding topcrash keyword and [@ nsHTTPChannel::Redirect] for tracking. This is #3 on the topcrash list from talkback.
Keywords: topcrash
Summary: Crash clicking "What's related" in sidebar → Crash clicking "What's related" in sidebar [@ nsHTTPChannel::Redirect]
racham: morse's fix is not in the 10/25 build. You have to use the 10/26 build (which will be tomorrow).
Sorry. I take it back. Spoke too soon. I just needed to update my local tree properly and it worked fine. I didn't see any crash reported. With commercial sweetlou builds, we shall the effect only tomorrow as the fix went at around 11 am. Changing the status back to fixed.
Status: REOPENED → RESOLVED
Closed: 25 years ago → 25 years ago
Hmm.. the bug status just says RESOLVED (no FIXED or anything). I am reopenng and marking this resolved/fixed again.
Status: RESOLVED → REOPENED
res/fxd
Status: REOPENED → RESOLVED
Closed: 25 years ago → 25 years ago
Resolution: --- → FIXED
*** Bug 58072 has been marked as a duplicate of this bug. ***
This looks good, no crash when I open up What's Related tab in Sidebar. Suzanne, pls confirm.Thx.
forgot to mention....today's branch builds on win/linux
Verifying all branch builds today 2000-10-26-09MN6 Win and Linux 2000-10-26-08MN6 Mac
Status: RESOLVED → VERIFIED
Keywords: vtrunk
oops, reopening and setting resolved fixed for verification on trunk sorry for the spam
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
setting vtrunk
Status: REOPENED → RESOLVED
Closed: 25 years ago → 25 years ago
Resolution: --- → FIXED
*** Bug 58036 has been marked as a duplicate of this bug. ***
I'm with brendan, I don't see a super reviewer listed for this check in it all. Only a r=don. Steve, why did we think it was ok to bypass the super review requirements of mozilla.org? I'm okay with the null ptr check but I would feel better if someone could tell me WHY we were running into a scenario where the load group for a channel was null.
For the curious: as matt pointed out in one of the earlier comments in this bug, the related links component doesn't use loadgroups (so they are null for its HTTP streams).
verified working on mac/win/linux 20001116 trunk builds.
Status: RESOLVED → VERIFIED
Keywords: vtrunk
*** Bug 58187 has been marked as a duplicate of this bug. ***
Product: Browser → Seamonkey
Crash Signature: [@ nsHTTPChannel::Redirect]
You need to log in before you can comment on or make changes to this bug.