Crash when right click save as this image in an imap message

VERIFIED FIXED in mozilla0.8

Status

MailNews Core
Backend
P2
normal
VERIFIED FIXED
18 years ago
5 years ago

People

(Reporter: (not reading, please use seth@sspitzer.org instead), Assigned: Bienvenu)

Tracking

({crash})

Trunk
mozilla0.8
x86
All
crash

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta1+])

Attachments

(2 attachments)

I'll attach the image.  it happened for me on winnt and linux.

to reproduce, send yourself the image and then in 6.0, right click on it to save as.

here's the stack:

ns_if_addref(nsIURI * 0x03145384) line 1101 + 15 bytes
nsImapMockChannel::GetURI(nsImapMockChannel * const 0x03146c10, nsIURI * *
0x0012e098) line 6734 + 19 bytes
XPTC_InvokeByIndex(nsISupports * 0x03146c10, unsigned int 11, unsigned int 1,
nsXPTCVariant * 0x0012e098) line 139
nsXPCWrappedNativeClass::CallWrappedMethod(JSContext * 0x04230630,
nsXPCWrappedNative * 0x04648920, const XPCNativeMemberDescriptor * 0x0108f9a8,
nsXPCWrappedNativeClass::CallMode CALL_GETTER, unsigned int 0, long *
0x00000000, long * 0x0012eb58) line 913 + 43 bytes
nsXPCWrappedNativeClass::GetAttributeAsJSVal(JSContext * 0x04230630,
nsXPCWrappedNative * 0x04648920, const XPCNativeMemberDescriptor * 0x0108f9a8,
long * 0x0012eb58) line 922
WrappedNative_GetProperty(JSContext * 0x04230630, JSObject * 0x00fa3978, long
45463760, long * 0x0012eb58) line 272 + 24 bytes
js_Interpret(JSContext * 0x04230630, long * 0x0012ece0) line 2468 + 1603 bytes
js_Invoke(JSContext * 0x04230630, unsigned int 1, unsigned int 2) line 837 + 13
bytes
js_InternalInvoke(JSContext * 0x04230630, JSObject * 0x00f20648, long 16393248,
unsigned int 0, unsigned int 1, long * 0x0012ee78, long * 0x0012ee08) line 909 +
20 bytes
JS_CallFunctionValue(JSContext * 0x04230630, JSObject * 0x00f20648, long
16393248, unsigned int 1, long * 0x0012ee78, long * 0x0012ee08) line 3193 + 31 bytes
nsJSContext::CallEventHandler(nsJSContext * const 0x042307e0, void * 0x00f20648,
void * 0x00fa2420, unsigned int 1, void * 0x0012ee78, int * 0x0012ee74, int 0)
line 907 + 33 bytes
nsJSEventListener::HandleEvent(nsIDOMEvent * 0x04646054) line 154 + 64 bytes
nsEventListenerManager::HandleEventSubType(nsListenerStruct * 0x042851e0,
nsIDOMEvent * 0x04646054, nsIDOMEventTarget * 0x04230c90, unsigned int 1,
unsigned int 7) line 788 + 19 bytes
nsEventListenerManager::HandleEvent(nsIPresContext * 0x0427b0c0, nsEvent *
0x0012f3c8, nsIDOMEvent * * 0x0012f38c, nsIDOMEventTarget * 0x04230c90, unsigned
int 7, nsEventStatus * 0x0012f3ec) line 1365 + 39 bytes
GlobalWindowImpl::HandleDOMEvent(GlobalWindowImpl * const 0x04230c80,
nsIPresContext * 0x0427b0c0, nsEvent * 0x0012f3c8, nsIDOMEvent * * 0x0012f38c,
unsigned int 1, nsEventStatus * 0x0012f3ec) line 510
DocumentViewerImpl::LoadComplete(DocumentViewerImpl * const 0x0427a2b0, unsigned
int 0) line 669 + 47 bytes
nsWebShell::OnEndDocumentLoad(nsWebShell * const 0x03cc57a8, nsIDocumentLoader *
0x03cb57a0, nsIChannel * 0x04252230, unsigned int 0) line 954
nsDocLoaderImpl::FireOnEndDocumentLoad(nsDocLoaderImpl * 0x03cb57a0, nsIChannel
* 0x04252230, unsigned int 0) line 805
nsDocLoaderImpl::DocLoaderIsEmpty(unsigned int 0) line 612
nsDocLoaderImpl::OnStopRequest(nsDocLoaderImpl * const 0x03cb57a4, nsIChannel *
0x042bc110, nsISupports * 0x00000000, unsigned int 0, const unsigned short *
0x100a56c8 gCommonEmptyBuffer) line 555
nsLoadGroup::RemoveChannel(nsLoadGroup * const 0x03cb5740, nsIChannel *
0x042bc110, nsISupports * 0x00000000, unsigned int 0, const unsigned short *
0x100a56c8 gCommonEmptyBuffer) line 566 + 39 bytes
nsJARChannel::OnStopRequest(nsJARChannel * const 0x042bc114, nsIChannel *
0x042bec80, nsISupports * 0x00000000, unsigned int 0, const unsigned short *
0x100a56c8 gCommonEmptyBuffer) line 709
nsOnStopRequestEvent::HandleEvent(nsOnStopRequestEvent * const 0x042be720) line 302
nsStreamListenerEvent::HandlePLEvent(PLEvent * 0x042be6d0) line 97 + 12 bytes
PL_HandleEvent(PLEvent * 0x042be6d0) line 580 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00ac97b0) line 513 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x0c110170, unsigned int 49320, unsigned int 0,
long 11311024) line 1049 + 9 bytes
USER32! 77e71820()
mscott was suprised that right click on the image even works!

this worked in 4.x, so marking 4xp.
Keywords: 4xp
(Assignee)

Comment 3

18 years ago
yep, crashes for me too, with a release commercial build.
(Assignee)

Comment 4

18 years ago
nominating for rtm - didn't notice this was assigned to me...may not finish it
today.
Status: NEW → ASSIGNED
Keywords: rtm
(Assignee)

Comment 5

18 years ago
doesn't seem to crash any more - just brings up save dialog which spins forever.
Probably not worth fixing for rtm, if that's other people's experience  as well
(i.e., spins but doesn't crash).

Comment 6

18 years ago
rtm-, assuming the dialog responds to the canel button so there's a way out.  

Comment 7

18 years ago
OK, putting rtm- in the whiteboard now...
Whiteboard: [rtm-]

Comment 8

18 years ago
I hit a crash with different messages, different images (gifs) on both Mac and
Win98 today using the oct30 mn6 commercial branch build.  When right clicking
then saving, I saw a default image name such as INBOX>4330 and when it had such
a default name I crashed. After restarting my mac machine, I no longer would see
such default image names and would not crash when saving.  I submitted a couple
talkbacks for mac, here's its stack info, don't know if this would qualify as
the same problem as seth saw:

Incident 20083102

 Call Stack:     (Signature = 0x61aa61a8 ba983939)
     0x61aa61a8
     CODE.10
     XPTC_InvokeByIndex() [xptcinvoke_mac.cpp, line 129]
     nsXPCWrappedNativeClass::CallWrappedMethod() [xpcwrappednativeclass.cpp,
line 907]
     WrappedNative_GetProperty() [xpcwrappednativejsops.cpp, line 272]
     js_Interpret() [jsinterp.c, line 2468]
     js_Invoke() [jsinterp.c, line 837]

Incident 20081711

 Call Stack:     (Signature = 0x61aa61a8 ba983939)
     0x61aa61a8
     CODE.10
     XPTC_InvokeByIndex() [xptcinvoke_mac.cpp, line 129]
     nsXPCWrappedNativeClass::CallWrappedMethod() [xpcwrappednativeclass.cpp,
line 907]
     WrappedNative_GetProperty() [xpcwrappednativejsops.cpp, line 272]
     js_Interpret() [jsinterp.c, line 2468]
     js_Invoke() [jsinterp.c, line 837]
<../images/spacer.gif>

Updated

18 years ago
QA Contact: esther → pmock
(Assignee)

Comment 9

18 years ago
changing summary, since it doesn't crash anymore, it just fails.
Summary: crash when I right click save as this image in an imap message → right click save as this image in an imap message doesn't work

Comment 10

17 years ago
Crash is reproducible on macos and linux commercial build following original 
steps.  It crashes immediately.  The build I was testing with was 
 Macos commercial seamonkey build 2000-110800-mn6 and Linux commercial build 
2000-110807-mn6

Steps to reproduce:
1) Send yourself a mail message with an attach gif/jpeg attachment or an 
inserted gif/jpeg image. 
2) Retrieve mail message
3) View mail message
   I was viewing mail message in the message pane
4) Right click on the displayed image
   The save dialog appears with a file name such as "Inbox>2343..."
5) Save the image
   Crash.
   No file is saved on hard drive.

On Win32, it does not crash unless I try to change the default file name.  
Following the steps above, if I change the file name from "Inbox>2343..." to 
something else, I receive a application crash.  No file was saved on hard drive.

The win32 build that I was using 2000-110801-mn6.

Updating summary:
 Old summary - right click save as this image in an imap message doesn't work
 New summary - Crash when right click save as this image in an imap message 
Summary: right click save as this image in an imap message doesn't work → Crash when right click save as this image in an imap message
(Assignee)

Comment 11

17 years ago
adding keywords
Keywords: crash, mail2
Priority: P3 → P2
I'll mark 63467 a DUP of this, even the stack trace is the same..
*** Bug 63467 has been marked as a duplicate of this bug. ***

Comment 14

17 years ago
marking nsbeta1+ and moving to mozilla0.8
Keywords: rtm → nsbeta1
Whiteboard: [rtm-] → [nsbeta1+]
Target Milestone: --- → mozilla0.8
(Assignee)

Comment 15

17 years ago
Created attachment 22400 [details] [diff] [review]
proposed fix

Comment 16

17 years ago
wow, that ref counting change was the only thing preventing this from working?
Interesting...

sr=mscott
(Assignee)

Comment 17

17 years ago
well, no, I'm just fixing the crash. Now, it saves the whole message.

Comment 18

17 years ago
r=naving

Comment 19

17 years ago
*** Bug 59684 has been marked as a duplicate of this bug. ***

Comment 20

17 years ago
Saves the whole message? Not just the image?
(Assignee)

Comment 21

17 years ago
right, I'm just fixing the crash; it saves the whole message.
(Assignee)

Comment 22

17 years ago
There's a problem if the message is a sent html page.
The attached patch causes a crash in the cookie service code because it doesn't
check for a null uri when asking the channel for a uri. Both
nsCookieService::GetCookieStringFromHTTP and nsCookieService::
SetCookieStringFromHttp need to check for a null aFirstUrl. The reason the uri
is null in the channel is because we null it out when we've finished running the
imap url. Some time after that, the cookie code asks our channel for the uri. I
could fix the cookie code not to crash (I'll attach a patch for that) but I'm
not sure if it's going to cause other problems.
(Assignee)

Comment 23

17 years ago
I can also fix this by making nsImapMockChannel return an error when it has a
null URI:

NS_IMETHODIMP nsImapMockChannel::GetURI(nsIURI* *aURI)
{
    *aURI = m_url;
    NS_IF_ADDREF(*aURI);
    // is it an error return a null uri? The cookie viewer code in particular
    // does not check for a null returned uri.
    return (*aURI) ? NS_OK : NS_ERROR_NULL_POINTER;
}

this fixes the crash, but causes a couple assertions when displaying html pages
in mail messages.
NS_ERROR_NULL_POINTER is supposed to indicate an invalid (because null, not
because garbage) pointer argument.  I don't think it should be overloaded to
mean "we would have returned a null result with NS_OK, but that crashes some
callers."

Why can't the channel URI persist after the IMAP operation has finished?  Isn't
that how all other Necko channel impls work?

/be
(Assignee)

Comment 25

17 years ago
Brendan, some of this is explained in the comments in the patch, but basically,
the channel and the imap url have strong references to each other (the channel
has to have a strong reference to the url to keep the url alive until it is
scheduled to run - why the imap mock channel has to have a strong reference to
the url, I don't know; that would be Scott's dept). Given that they have strong
references to each other, the cycle has to be broken, and the last chance we
have to break that cycle is when the url finishes running. There is an added
complication that last reference has to be released on the UI thread or else
Warren's false-sense-of-threadsafety assertions start firing.
Sorry, I should have read the patch.  Why can't the channel continue to refer
strongly to the URI after it has run, but once the URI finishes running, we cut
the strong ref from it to the channel?  Maybe only one link in the cycle needs
to be broken.  I'm still figuring out the mock vs. real relation, sorry if I'm
missing something.

/be
(Assignee)

Comment 27

17 years ago
I tried that orginally, but I needed to break the other link (from mock channel
to uri) in the UI thread or else the url would get deleted on the imap thread,
and trigger a bunch of asserts. Maybe there's some other way I can fix that...
(Assignee)

Comment 28

17 years ago
the assert only happens in a few situations, like copying messages. I might have
to proxy the clearing of the mock channel member variable in
nsImapProtocol::ReleaseUrlState() over to the UI thread, because this release
often causes a bunch of other objects to get freed on the imap thread that
should be released on the ui thread.
Does Necko well define the ability to get a valid URI from a channel while the
channel is alive?  Maybe warren knows; nsIChannel.idl doesn't clearly define the
validity (null vs. non-null) of the URI attribute.  The fact that code breaks
when URI becomes null on a live IMAP channel suggests we have a _de facto_
standard, albeit under- or un-documented.

Here's hoping the mock-channel clearing proxy idea works out.

/be

Comment 30

17 years ago
Since a channel is created from a URI, I think that a channel should always be
able to produce its URI.  Another way to look at it is that a channel is your
access to a resource, without a URI, how do you know what resource you're
accessing?  It seems to me that a channel is meaningless without an associated
URI... right?!?
(Assignee)

Comment 31

17 years ago
I fixed this by releasing the mock channel from the protocol before releasing
the last reference to the url from the protocol object (the latter is proxied).
Now,  the releases all end up happening on the UI thread. Previously, the
channel was released from the protocol at the end of
nsImapProtocol::ReleaseUrlState(); now it's released at the start of
nsImapProtocol::ReleaseUrlState().
(Assignee)

Comment 32

17 years ago
marking fixed. I'll open a new bug for it not working. bug 65329
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 33

17 years ago
Verified as fixed on win32, linux, and macos using the following builds:
 win32 commercial seamonkey build 2001-011708-mtrunk installed on P500 Win98
 linux commercial seamonkey build 2001-011708-mtrunk installed on P200 RedHat 
6.2
 macos commercial seamonkey build 2001-011708-mtrunk installed on G3/400 OS 9.04

It does not crash anymore.
Status: RESOLVED → VERIFIED
Product: MailNews → Core
Product: Core → MailNews Core

Updated

5 years ago
Blocks: 894848

Updated

5 years ago
No longer blocks: 894848
You need to log in before you can comment on or make changes to this bug.