Crash in nsWebShell::DoLoadURL

VERIFIED FIXED in M13

Status

P3
major
VERIFIED FIXED
19 years ago
11 years ago

People

(Reporter: phil, Assigned: mscott)

Tracking

Trunk
x86
Other

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

19 years ago
1. Run mail
2. Load a folder and a message
3. Click away from the thread pane, causing the thread pane to lose focus.
4. Click back on the selected message in the thread pane
5. Boom.

I've seen this a couple times, but I can't make it happen at will. Scott says
he's seen it, though, so I'm filing it.
(Reporter)

Comment 1

19 years ago
Also, the crash happens at the end of a C++ block scope, so I assume the
destructor of some automatic object (like a COM ptr) is forcing the crash.
(Assignee)

Updated

19 years ago
Status: NEW → ASSIGNED
Target Milestone: M13
(Assignee)

Comment 2

19 years ago
I finally figured out what was going on here. Marking as M13 as this is a random
crasher when displaying imap messages that's causing problems.
 And the fix is trivial. I'll attach the patch here. Travis, can you give me a
code review on this since Rick Potts is out of town?

DoLoadUrl had code that looked like the following:

nsCOMPtr<nsIChannel> dummyChannel;
rv = NS_OpenURI(getter_AddRefs(dummyChannel), aUri, nsnull, interfaceRequestor);
if (NS_FAILED(rv)) return rv;
if (SOME CONDITION testing if the url has a ref field)
{
	use the dummy channel and return
}
else if (SOME OTHER CONDITION to test if we are loading from session history)
{
   use the dummy channel and return
}

If neither of these conditions were true, then doLoadURL went ahead and kicked
off a regular url load by calling into the uri loader.

The problem was that for a certain case for imap, we were creating a dummy
channel under some bogus conditions such that the imap protocol handler failed
to create a channel and it returned NS_OK back through NS_OpenURI. So the first
fix is to perculate the error back up. Which I did.

However, that's not good enough because of the if (NS_FAILED(rv)) return rv
statement after creating the dummy channel which causes us to never load the url
in this case. It happens to be for imap that neither of the if conditions in my
example above are ever true. We always want to continue forward with the load
even if we failed to create a dummy channel. Because we are never going to use
the dummy channel.

So my fix for webshell::doloadURl was to stall instantiation of the dummy
channel until we are sure we are going to need it. This meant making the call to
NS_OpenURI inside of each if clause.

if (some condition)
{
  create and use dummy channel returning if failure.
}
else if (some condition)
{
  create and use dummy channel, returning if failure.
}

This fixes the problem because now we don't even try to create a dummy channel
if we really aren't going to use it. And the fix is completely safe because we
aren't altering the flow of control at all.
(Assignee)

Comment 3

19 years ago
Created attachment 4365 [details] [diff] [review]
proposed fix
(Reporter)

Updated

19 years ago
Whiteboard: have fix; waiting for review
(Assignee)

Updated

19 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED
Whiteboard: have fix; waiting for review
(Assignee)

Comment 4

19 years ago
I checked in the fix....

Comment 5

19 years ago
Win32  (2000-04-21-08 M16)
Using the same scenario, I do not see the problem.
Status: RESOLVED → VERIFIED
QA Contact: lchiang → fenella
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.