Closed
Bug 63529
Opened 24 years ago
Closed 24 years ago
zero pointer deref in nsWebshell::OnStateChange
Categories
(Core :: DOM: Navigation, defect)
Core
DOM: Navigation
Tracking
()
RESOLVED
FIXED
mozilla0.9.1
People
(Reporter: bernd_mozilla, Assigned: rpotts)
References
Details
Attachments
(2 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
http://lxr.mozilla.org/seamonkey/source/docshell/base/nsWebShell.cpp#905 channel can be zero. I just crashed there by switching between pages back and forward. I think there should be zero pointer check of the do_QueryInterface result. Win98 CVS20001221.
TB23406579Q, I crashed here reading news.
Keywords: crash
*** Bug 63523 has been marked as a duplicate of this bug. ***
Comment 3•24 years ago
|
||
Comment 4•24 years ago
|
||
Why is |channel| null, though? Maybe |aRequest| is null? Rick, is this patch okay or is there more going on here?
Severity: normal → critical
Assignee | ||
Comment 5•24 years ago
|
||
The channel should never be null in OnStateChange(...) I'm looking into why this is sometimes happening. It appears that the channel is sometimes null if the document has been cancelled... This is why you mostly see the crash when using back/forward... I believe that adding a null-pointer check in nsWebShell should be harmless (for now)...
Comment 6•24 years ago
|
||
Hrm...so should I check this in in the interim (r=rpotts?)?
Assignee | ||
Comment 7•24 years ago
|
||
I've checked in a null-pointer test at the top of nsWebShell::OnStateChange(...) This should stop the crash, but I'm leaving the bug open until I can figure out why the channel is null in the first place. -- rick
Comment 10•24 years ago
|
||
*** Bug 63623 has been marked as a duplicate of this bug. ***
Comment 11•24 years ago
|
||
*** Bug 63548 has been marked as a duplicate of this bug. ***
Comment 12•24 years ago
|
||
*** Bug 63637 has been marked as a duplicate of this bug. ***
Comment 13•24 years ago
|
||
adding mostfreq, this breaks the Bugzilla Helper. Adding dogfood keyword.
Reporter | ||
Comment 14•24 years ago
|
||
*** Bug 63638 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 16•24 years ago
|
||
Are people still seeing this crash - after the patch was checked in? If not, I'd like to remove the keywords and leave the bug open until I can investigate (and fix) why the channel is being passed in as null... -- rick
No, I'm not seeing this crash anymore.
Reporter | ||
Comment 18•24 years ago
|
||
*** Bug 63812 has been marked as a duplicate of this bug. ***
Comment 19•24 years ago
|
||
This is still the #1 topcrash in current builds. Perhaps |aRequest| is garbage rather than null? See ftp://ftp.mozilla.org/pub/data/crash-data/ for more details. In talkback it looks like the problem started in the evening builds on 2000-12-21, although this bug was filed earlier in the day. Any chance of looking through checkins to see what might have caused it?
Keywords: topcrash
Reporter | ||
Comment 20•24 years ago
|
||
the earliest crash is Build: 2000122021 CrashDate: 2000-12-21 fix for bug 15345 by rpotts 12/20/2000 15:18
Comment 21•24 years ago
|
||
*** Bug 63849 has been marked as a duplicate of this bug. ***
Comment 22•24 years ago
|
||
Is bug 62643 a dupe of this bug? I am wondering because the stacktrace for bug 62738 which was duped to 62643, and that of bug 63630 duped to this one are nearly identical. Though crash in bug 63630 disappeared after the patch here, 62643 still crashes. Moreover, I have started to get crash as in bug 65181 that gives me the same stacktrace.
Comment 23•24 years ago
|
||
Can't we get this null-check patch in, at least temporary? This is a topcrash, mostfreq and dogfood bug. So we can't just stay idle about it. r=hwaara on the patch if that matters...
Comment 24•24 years ago
|
||
A slightly different null pointer check was already checked in (nsWebShell.cpp rev 1.508).
Updated•24 years ago
|
Target Milestone: --- → mozilla0.9
Comment 25•24 years ago
|
||
*** Bug 63634 has been marked as a duplicate of this bug. ***
Comment 26•24 years ago
|
||
anyone have a test case for this?
Comment 27•24 years ago
|
||
<Hixie> Is this still a crasher? Should it really be targetted at mozilla 0.9?
Assignee | ||
Comment 28•24 years ago
|
||
Removing keywords and moving out to a later milestone... I'm working on a "correct patch" which prevents the nsIRequest from ever being NULL :-)
Assignee | ||
Comment 29•24 years ago
|
||
Assignee | ||
Comment 30•24 years ago
|
||
I've just attached a patch that fixes this bug... It also fixes bug #55867 bug #26295 bug #63247... All of these are related bugs :-)
Assignee | ||
Comment 31•24 years ago
|
||
These patches fix the following bugs: ------------------------------------- * bug #55867 which was opened to provide a real fix of the problem described in bug #27687. * bug #63529 which resulted from mDocumentRequest being null. A while ago I added a null pointer check in the DocShell/WebShell... But these patches make sure that mDocumentRequest is nenver null if the first place.. * bug #26295 which was the assertion that either the LoadGroup or DocLoader was still busy after being canceled. * bug #63247 which was tracking the need to change the arguments being passed on nsIHTTPEventSink::OnRedirect(...) These patches also affect the following bugs: ---------------------------------------------- * bug #76286 - I reworked the ASSERTION that waterson added to verify that mDocumentRequest was never null. I also reworked the way the mDocumentRequest is being set - to guarentee that it is NEVER null :-) * bug #59827 - I reworked the way that the STATE_TRANSFERRING notification is being fired * bug #27687 - I removed the band-aid patch in nsHTTPChannel since the real fix (in bug #55867) is provided. Here's a quick overview of the changes that these patches introduce... ---------------------------------------------------------------------- * nsLoadGroup has been modified to prevent new requests from being added while it is in the process of canceling its requests... * OnRedirect(...) has been reworked in nsHTTPChannel. The nsIHTTPEventSink::OnRedirect notification is now fired *before* the redirect occurs - and it is now cancelable (via a FAILURE return code) When a channel is redirected the status code passed out through OnStopRequest is now NS_BINDING_REDIRECTED * The signature of nsIHTTPEventSink::OnRedirect(...) has changed to be two channels - the old one and the new one... * nsDocLoader::OnStartRequest has been changed to only rely on the LOAD_DOCUMENT_URI flag to determine if a request represents the "document channel" or not... This is a more reliable check that seeing if the count == 1. It ensures that mDocumentRequest is never null during a page load... * nsDocLoader::DocLoaderIsEmpty() was changed to not pass a status code. In the current code, the argument was ignored - because it is useless :-) thanks, -- rick
Assignee | ||
Comment 32•24 years ago
|
||
patch checked in. -- rick
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•