Closed
Bug 144735
Opened 22 years ago
Closed 22 years ago
Sending a signed e-mail often leads to crash. [@nsMsgMailSession::GetTopmostMsgWindow]
Categories
(MailNews Core :: Composition, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: lapo, Assigned: nhottanscp)
References
Details
(Keywords: crash, intl, Whiteboard: [adt1 rtm])
Crash Data
Attachments
(1 file)
(deleted),
patch
|
bugzilla
:
review+
Bienvenu
:
superreview+
jesup
:
approval+
|
Details | Diff | Splinter Review |
1.open a new message
2.write it
3.enable digital signature
4.press send
5.either it is sent or Mozilla suddenly crashes and you are presented with
talktabk dialog
Unfortunately I just upgraded to 1.0rc2 and my 10 crashes of this king with
1.0rc1 are lost (well, they all were from 'lapo@lapo.it' and contained "sending
a signed message" as description).
Latest, with 1.0rc2 is: TB6284031Z
Comment 1•22 years ago
|
||
signature: nsMsgMailSession::GetTopmostMsgWindow 3eea5b7a
Product ID Gecko1.0
Build ID 2002051008
Trigger Time 2002-05-14 14:47:09
Platform Win32
Operating System Windows NT 5.1 build 2600
Module msgbase.dll
URL visited
User Comments sending a signed message
Trigger Reason Access violation
Source File Name d:\builds\seamonkey\mozilla\mailnews\base\src\nsMsgMailSession.cpp
Trigger Line No. 408
Stack Trace
nsMsgMailSession::GetTopmostMsgWindow
[d:\builds\seamonkey\mozilla\mailnews\base\src\nsMsgMailSession.cpp, line 408]
nsMsgComposeAndSend::GetNotificationCallbacks
[d:\builds\seamonkey\mozilla\mailnews\compose\src\nsMsgSend.cpp, line 271]
nsMsgComposeAndSend::DeliverFileAsMail
[d:\builds\seamonkey\mozilla\mailnews\compose\src\nsMsgSend.cpp, line 3245]
nsMsgComposeAndSend::DeliverMessage
[d:\builds\seamonkey\mozilla\mailnews\compose\src\nsMsgSend.cpp, line 3106]
nsMsgComposeAndSend::GatherMimeAttachments
[d:\builds\seamonkey\mozilla\mailnews\compose\src\nsMsgSend.cpp, line 1120]
nsMsgAttachmentHandler::UrlExit
[d:\builds\seamonkey\mozilla\mailnews\compose\src\nsMsgAttachmentHandler.cpp,
line 1174]
FetcherURLDoneCallback
[d:\builds\seamonkey\mozilla\mailnews\compose\src\nsMsgAttachmentHandler.cpp,
line 476]
nsURLFetcher::OnStopRequest
[d:\builds\seamonkey\mozilla\mailnews\compose\src\nsURLFetcher.cpp, line 325]
nsDocumentOpenInfo::OnStopRequest
[d:\builds\seamonkey\mozilla\uriloader\base\nsURILoader.cpp, line 255]
nsFileChannel::OnStopRequest
[d:\builds\seamonkey\mozilla\netwerk\protocol\file\src\nsFileChannel.cpp, line 522]
nsOnStopRequestEvent::HandleEvent
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsRequestObserverProxy.cpp, line 213]
PL_HandleEvent [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 597]
PL_ProcessPendingEvents [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c,
line 530]
_md_EventReceiverProc [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line
1078]
USER32.dll + 0x3c076 (0x77d4c076)
USER32.dll + 0x3c076 (0x77d4c076)
_except_handler3()
kernel32.dll + 0x3bb86 (0x77e7bb86)
.
Assignee: ducarroz → nhotta
Severity: normal → critical
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: crash
Summary: Sending a signed e-mail often leads to crash. → Sending a signed e-mail often leads to crash. [@nsMsgMailSession::GetTopmostMsgWindow]
Assignee | ||
Comment 4•22 years ago
|
||
I have not been able to reproduce this using 5/14 commercial branch.
But I noticed that some of the calls I use in the function may return NS_OK even
the output value is not valid. Probably I should also check if the returned
objects are null.
http://lxr.mozilla.org/seamonkey/source/mailnews/base/src/nsMsgMailSession.cpp#394
394 rv = windowEnum->GetNext(getter_AddRefs(windowSupports));
395 NS_ENSURE_SUCCESS(rv, rv);
396
397 topMostWindow = do_QueryInterface(windowSupports, &rv);
398 NS_ENSURE_SUCCESS(rv, rv);
399
400 rv = topMostWindow->GetDocument(getter_AddRefs(domDocument));
401 NS_ENSURE_SUCCESS(rv, rv);
402
403 rv = domDocument->GetDocumentElement(getter_AddRefs(domElement));
404 NS_ENSURE_SUCCESS(rv, rv);
405
406 rv = domElement->GetAttribute(NS_LITERAL_STRING("windowtype"),
windowType);
407 NS_ENSURE_SUCCESS(rv, rv);
Comment 5•22 years ago
|
||
I am not able to reproduce the problem too. But looks like
rv = domDocument->GetDocumentElement(getter_AddRefs(domElement));
return a null domElement despite it doesn't return an error!
Reporter, can you tell use which other windows are open and in which order. That
would help use to reproduce the problem.
Comment 6•22 years ago
|
||
crash bug. [adt1 rtm]
Reporter | ||
Comment 7•22 years ago
|
||
It seems that with rc2 it is less frequent, or just the occurrence.
Usually I have a single browser windows with many tabs (10 or more) and a single
mailnews windows, and the compose mail windows of course.
I will try more signed message to try catching some more occurrences.
Reporter | ||
Comment 8•22 years ago
|
||
Said, done.
I have another crash report: TB6284031Z
Same open windows (also described in the report itself).
More infos: I use a 2048bit cert, but the first crashes in 1.0rc1 were using the
old 1024bit cert if I remember correctly.
Assignee | ||
Comment 9•22 years ago
|
||
TB6284031Z also crashes in the same place.
I still cannot reproduce but I believe that the extra error check will prevent
the crash.
Assignee | ||
Comment 10•22 years ago
|
||
Comment 11•22 years ago
|
||
Comment on attachment 83809 [details] [diff] [review]
Add ponter checks after the dom function calls.
r=ducarroz
Attachment #83809 -
Flags: review+
Comment 12•22 years ago
|
||
Comment on attachment 83809 [details] [diff] [review]
Add ponter checks after the dom function calls.
sr=bienvenu - my guess is that its the GetDocument call that's succeeding but
returning null - I have a vague memory of seeing that before. I know some of
the reviewers@mozilla.org aren't going to like this fix, but I'd rather not
crash.
Attachment #83809 -
Flags: superreview+
Comment 13•22 years ago
|
||
I can't reproduce this either with 20020512pr1 build. Reporter can you verify
this works for you when fixed.
Assignee | ||
Comment 14•22 years ago
|
||
checked in to the trunk
Reporter, please try tomorrow's trunk build.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 15•22 years ago
|
||
Sent some 5 mails with 2002051606, but too few to say "ok it is now tested",
more testing this evening.
Reporter | ||
Comment 16•22 years ago
|
||
Got a new crash signing email with 2002051606 =(
Still waiting for talkback.netscape.com to accept the ticket...
Reporter | ||
Comment 17•22 years ago
|
||
Finally... it's TB6467193W
Reopening this =(
Or maybe that snapshot was too old to have the patch?
Installing the new snapshot...
Assignee | ||
Comment 18•22 years ago
|
||
This is strange, the same line number (TB6284031Z and TB6467193W) after adding
few lines.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter | ||
Comment 19•22 years ago
|
||
Definitely to be reopened, same error in today's snap (2002051906).
TB6481255Q
Assignee | ||
Comment 20•22 years ago
|
||
Since I put a null check already, it could be the pointer is not null but invalid.
My original change was written for message viewing to identify the top window.
But this is called for message send.
The caller of topmostwidnow is
nsMsgComposeAndSend::GetNotificationCallbacks
266 nsresult
nsMsgComposeAndSend::GetNotificationCallbacks(nsIInterfaceRequestor** aCallbacks)
267 {
268 // TODO: stop using mail3pane window!
269 nsCOMPtr<nsIMsgWindow> msgWindow;
270 nsCOMPtr<nsIMsgMailSession> mailSession(do_GetService(kMsgMailSessionCID));
271 mailSession->GetTopmostMsgWindow(getter_AddRefs(msgWindow));
272 if (msgWindow) {
273 nsCOMPtr<nsIDocShell> docShell;
274 msgWindow->GetRootDocShell(getter_AddRefs(docShell));
275 nsCOMPtr<nsIInterfaceRequestor> ir(do_QueryInterface(docShell));
276 if (ir) {
277 *aCallbacks = ir;
278 NS_ADDREF(*aCallbacks);
279 return NS_OK;
280 }
281 }
282 return NS_ERROR_FAILURE;
283 }
I want to know why the code needs topmost window in what situation.
cc to mscott
Scott, why does the code need topmost window? Is this anything to do with
progress dialog?
Assignee | ||
Comment 21•22 years ago
|
||
>My original change was written for message viewing to identify the top window.
Actually, that was for reply/quote to find the original message window.
Assignee | ||
Comment 22•22 years ago
|
||
Looking at the talkback data for TB6481255Q, it indicates the branch build is used.
Gecko1.0 mozilla.exe 1.0.0.0
Netscape Win32 (2002051909)
Trigger Time: 05/20/2002 14:27:54
Set Bug ID Delete This Incident
Back to List No Previous Incident Incident ID: 6481255
For trunk build, it is supposed to have this line instead of "Geko1.0 ...".
MozillaTrunk Mozilla.exe 0.0.0.0
The patch has not been landed on 1.0.0 branch yet. Please try again using the
trunk build, thanks.
Status: REOPENED → RESOLVED
Closed: 22 years ago → 22 years ago
Resolution: --- → FIXED
Updated•22 years ago
|
Comment 23•22 years ago
|
||
adding adt1.0.0+. Please get drivers approval and then check into the 1.0 branch.
Reporter | ||
Comment 24•22 years ago
|
||
TB6781047K TB6780851Z
I hope this only means that this fix didn't make into RC3 ^_^
Comment 25•22 years ago
|
||
Lapo, your latest talkback reports you are not using the trunk builds. This was
fixed on a trunk build and is a candidate for a branch build but has not landed
on the branch yet. I can not verify this bug and resolve it as verified because
I don't have a reproducible case. If you could take a trunk build (the nightly
builds are trunk builds) instead of a branch build and try this that would be
great.
Reporter | ||
Comment 26•22 years ago
|
||
I already verified it on trunk and it's OK, only I thought that it landed before
rc3, just it isn't.. no problem ^_^
Comment 27•22 years ago
|
||
changing to adt1.0.1+ for checkin to the 1.0 branch for the Mozilla1.0.1
milestone. Please get drivers approval before checking in.
Comment 28•22 years ago
|
||
Comment on attachment 83809 [details] [diff] [review]
Add ponter checks after the dom function calls.
please check into the 1.0.1 branch ASAP. once landed remove the
mozilla1.0.1+ keyword and add the fixed1.0.1 keyword
Attachment #83809 -
Flags: approval+
Updated•22 years ago
|
Keywords: mozilla1.0.1+
Updated•22 years ago
|
Reporter | ||
Comment 30•22 years ago
|
||
It's a pity it disdn't made into 1.0 code (TB7096797G)... it's the version the
majority of the users will use for a long time.
Well, I guess a "not checked for long" patch was worse than nothing for
1.0RC3->1.0 transition...
Comment 31•22 years ago
|
||
I still can't verify this because I don't have a reproducible case. Lapo,
thanks for all your help with this, but I can't tell by your comments if you
tried this on a branch build that includes the fix. If so, could you comment in
the bug that you have verified it was fixed. Thanks again, Esther
Updated•22 years ago
|
Keywords: mozilla1.0.1+
Comment 32•22 years ago
|
||
Lapo emailed me and indicated that while using the branch builds last month
(June) the problem has not been seen. The the fix went into the branch on 5-30.
I will verify this now since the reporter is no longer seeing the problem. We
will reopen if it should come back.
Status: RESOLVED → VERIFIED
Keywords: fixed1.0.1 → verified1.0.1
Updated•20 years ago
|
Product: MailNews → Core
Updated•16 years ago
|
Product: Core → MailNews Core
Updated•13 years ago
|
Crash Signature: [@nsMsgMailSession::GetTopmostMsgWindow]
You need to log in
before you can comment on or make changes to this bug.
Description
•