Closed
Bug 83381
Opened 24 years ago
Closed 24 years ago
replying to a msg crashes
Categories
(MailNews Core :: Composition, defect, P1)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla0.9.1
People
(Reporter: skasinathan, Assigned: mscott)
Details
(Whiteboard: [nsbeta1+] fix in hand r= sr=sspitzer)
Attachments
(4 files)
..so..if you are inside netscape firewall you should have got a msg from
chofmann about mtbf. I replied to that mail and it crashes after throwing this
dialog. "An error occured while sending mail. The mailserver responded DATA line
too long. max 16384....."
Build: Today's build on Win NT.
here goes the stack trace:
nsMsgAsyncWriteProtocol::UnblockPostReader
[d:\builds\seamonkey\mozilla\mailnews\base\util\nsMsgProtocol.cpp, line 1001]
nsMsgProtocolStreamProvider::OnDataWritable
[d:\builds\seamonkey\mozilla\mailnews\base\util\nsMsgProtocol.cpp, line 728]
nsOnDataWritableEvent::HandleEvent
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsStreamProviderProxy.cpp, line
143]
PL_HandleEvent
[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 591]
PL_ProcessPendingEvents
[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 524]
_md_EventReceiverProc
[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 1072]
Comment 2•24 years ago
|
||
event if we are still generating line too long, we should not crash. See the
stack trace, reassign to mscott, cc'ing varada about the line lenght problem.
Assignee: ducarroz → mscott
Comment 3•24 years ago
|
||
I can't reproduce the crash but I do get the line too long problem. Because of
this you can't send the message. Varada could you help look into this? Putting
on 0.9.1 radar.
Priority: -- → P1
Whiteboard: [nsbeta1+]
Target Milestone: --- → mozilla0.9.1
Comment 4•24 years ago
|
||
I try on both Mac and Windows using today's debug build and I cannot crash
either!
I can reproduce this crash at will using today's commercial release build and
mozilla debug build from late last night on Win NT.
fyi: After I click send, "Ask Me' dialog pops up and let the default (Both Plain
text and HTML) selected.
do you see the same stack each time you crash?
Status: NEW → ASSIGNED
Using today's debug build on WinNT as well as yesterday's release builds I am
unable to reproduce either problem - the line is too long or the crash
I see the same stack everytime I tried. Also, I was able to duplicate this (both
crash and Max data alert) using Navin's Win NT system (just to make sure it is
not just my system).
Updated•24 years ago
|
Whiteboard: [nsbeta1+] → [nsbeta1+] - hard to reproduce
Reporter | ||
Comment 10•24 years ago
|
||
Finally got a chance to check on this using 0531 build and I was able to
duplicate it. btw, my SMTP SSL setting was set to "Never", if that matters.
Comment 11•24 years ago
|
||
I don't crash with the 6-1 build on win98, but I do consistently get the error
message "sending message failed".
pulling on my winnt box so I can help debug.
Comment 12•24 years ago
|
||
Comment 13•24 years ago
|
||
If I go to reply to that message, and then hit save, I fail when trying to save
the draft. (note, my drafts folder is local.)
saving a small message works fine.
The original message has a pretty big <pre> block. perhaps when we quote (for
reply) that pre block, it turns into a giant line with no CRLFs?
I'm still rebuilding so I don't have a build I can debug yet.
Comment 14•24 years ago
|
||
As far as I can see the last few lines in the LOG are from nsSocketTransport -
either LeavingProcess or CloseConnection and after that we crash. I hope that
Darin can make something out of this. It doesnt crash on mac or on linux.
Surprisingly enough -Most times I am unable to crash it when I run it thru my
debugger - but the very same debug build crashes consistently when run thru the
same scenario from the command line prompt!!.
Updated•24 years ago
|
Whiteboard: [nsbeta1+] - hard to reproduce → [nsbeta1+] - hard to reproduce; needs darin investigation
Comment 15•24 years ago
|
||
while the crash is hard to reproduce, the failure to send is not.
I'll help debug this today.
Comment 16•24 years ago
|
||
debugging to see why saving the reply as draft (to a local draft folder)
fails. (CopyData() in nsLocalMailFolder is failing).
about the reply that is generated, the <pre> line is HUGE. I'll attach what is
generated.
Comment 17•24 years ago
|
||
Comment 18•24 years ago
|
||
the save as draft fails here, in nsLocalMailFolder.cpp, line 2153:
if (!end && aLength > (PRInt32) maxReadCount)
// this must be a gigantic line with no linefeed; sorry pal cannot handle
return NS_ERROR_FAILURE;
this just confirms that the giant <pre> line is killing us.
Comment 19•24 years ago
|
||
forgive my ignorance of editor:
the original message (in html) has a pre block like this:
<pre>
1
2
3
...
100
</pre>
you leave pre blocks alone since they are preformatted.
when we reply, the original message is wrapped with <blockquote> tags
that turns the pre into this:
<blockquote>
<pre>1<br>2<br>3<br>....100<br></pre>
</blockquote>
that's what's killing us.
if we are inserting <br>, can't we do "<br>\n"?
Comment 20•24 years ago
|
||
or "<br>CRLF" (or whatever).
I'm not able to reproduce the crash, all I can get is the failures to send or
save.
Comment 21•24 years ago
|
||
looks like this is all covered in bug #67334. I'll see if I can help by fixing
that.
Comment 22•24 years ago
|
||
The crash happens only if the recepient is of unknown format preference and when
the dialog box comes up we choose to send as Plaintext & HTML.
Comment 23•24 years ago
|
||
ok, I've got a hack for bug #67334 that will avoid this problem.
I'll attach what the reply looks like when using my hack
Comment 24•24 years ago
|
||
Comment 25•24 years ago
|
||
The crash occuring in nsMsgProtocol.cpp happens because we have already closed
the socket. We are then trying to write again. This ofcourse leads to the crash.
Adding an addref to the mMsgProtocol led to other problems.
Still Investigating.
Comment 26•24 years ago
|
||
> The crash occuring in nsMsgProtocol.cpp happens because we have already closed
> the socket. We are then trying to write again.
as I mentioned over aim to varada, a while back we had these crasher bugs for
IMAP and NNTP where we'd crash when we'd tried to write to a socket and it was
closed.
see bug #72317
> Adding an addref to the mMsgProtocol led to other problems.
huh? can you elaborate? why where you doing that?
Comment 27•24 years ago
|
||
Can we take the hack from bug 67334 for m.9.1 and avoid the crash? Is there an
"avoid the crash" hack we can take for m.9.1? We're almost completely out of
time...
Comment 28•24 years ago
|
||
Ok, so I understand being under the gun. But this is really a "one step forward
two steps back" proposal.
First off the patch has nothing to do with the real cause of the bug, if I
undstand this discussion correctly. Thus we are masking a crasher in this one
instance. Are there other ways to get the crash? We don't know, right?
Next, this patch won't avoid the situation of long <pre> lines in all cases, so
there are other things that the user can do to trigger the same situation (and
presumably crash). This patch will be active for html paste, and mail quoting,
as things stnad now in the source. It will not, though, do anything for normal
text pasting. So if someone copies a bunch of normal text (for instance, out of
a textarea or plaintext editor or a different app) and inserts (pastes) it into
the message inside of a pre somewhere, you will get the same problem. We would
have to expand the patch to cover the text insertion code, and that is the second
riskiest code to touch in the editor (you can break everything with a screwup
there), right after the deletion code.
Third, Seth is right that this will cause regressions. I just confirmed it. In
addition to the regression he described inside mail quotes, you can get
regressions in Composer (mail not involved). For instance, take this source:
<html>
<body>
non pre text
<pre>some pre text
blank lines above
end of pre text</pre>
some non ore text
</body>
</html>
If you load that in mozilla (with the patch) and copy it from the browser window
(an html copy) and paste it into Composer, you won't be able to click on any of
the blank lines. Doh!
Please don't do this. Already editor is one of the most problematic parts of the
app, and half our problems stem from trying to work around issues like this that
really aren't editor caused. We need to start biting the bullet and doing the
"right thing" in the right places if editor is ever going to be decent.
I don't know what we should do short term (but I hope it's not this). Fixing the
real cause of the crash would be best.
Longer term two things should probably happen:
We should get layout&caret code to work together better and do what it takes to
avoid the "dead zones" in pres with newline (and in empty table cells and list
items).
There should be flags to set on the html serializer that will positively
guarantee that long lines are wrapped if the caller asks for it, even if it
alters the source in some meaningful way. After all, what if someone pastes in a
line that is 1000 chars long with no spaces or returns? You can't mail it like
that, and yet to break it up has to alter the content.
The editor is actually the wrong place to solve either of these problems.
Comment 29•24 years ago
|
||
Solving the crash would be great, but my understanding is that we'll still be
left in a state where there are certain messages which will not be able to be
sent. How long term is the long term solution and who are the right people to
involve in it?
Comment 30•24 years ago
|
||
This bug should be solely for the purpose of the crashing issue.
The issue of messages not being able to be sent due to long lines is covered by
at least one other bug (possibly 2 or even more). Discussions/approaches/etc.
for the long lines issue should be covered in bug #67334 or another bug.
Removing the depends since this crasher can be fixed independently of bug #67334
(and a fix to bug #67334 will mask the problem).
No longer depends on: 67334
Comment 31•24 years ago
|
||
jfrancis' argument makes a lot of sense. I respect his decision to move editor
forward, and not hack it up more.
> 1) We should get layout&caret code to work together better...
Is there a bug for this? If that were fixed, it would allows us to keep the
newlines, and help reduce the frequency of the crash and the "fail to send /
fail to copy msg" problem, by reducing the frequency of the long lines.
(but not eliminated it, as you pointed out)
> 2) There should be flags to set on the html serializer that will positively
> guarantee that long lines are wrapped if the caller asks for it
Is there a bug for this? About wrapping, I get a little confused. there is
wrapping when the user views a message, and there's also wrapping on send?
in this case, we don't need a CRLF at 72 or 80 columns. we just need one every
4092 bytes (or something).
To fix (or hide) the crash/ fail to send / fail to copy problems on the 0.9.1
branch, could we do what someone (brade?) suggested and force in a newline
(hopefully in the mailnews code, not editor) to wallpaper over this problem?
I'll investigate such a hack.
Comment 32•24 years ago
|
||
I think the content serializer should have a flag that enforces a line length
(which, as seth should know, should be < 1000 octets so that NNTP works).
Comment 33•24 years ago
|
||
seth: i stepped through the code a bit with varada, and it seemed at first as
though nsMsgProtocolStreamProvider was having its mMsgProtocol member deleted
out from under it. so, we tried making nsMsgProtocolStreamProvider hold a
hard ref to mMsgProtocol. that didn't get us very far... it just moved the
crash to UnblockPostReader() while touching m_outputStream.
so, it seems like things are getting prematurely shutdown. varada said that
we were calling CloseSocket() from the SMTP_FREE state in nsSmtpProtocol::
ProcessProtocolState(). nsMsgProtocol::CloseSocket() seems OK. i'm not sure
why OnDataWritable would still be processed after a call to m_request->Cancel().
nsOnDataWritableEvent::HandleEvent first checks the socket request's status
and will not call OnDataWritable if the socket request's status is a FAILURE
code. perhaps there is some problem with the Cancel call?!?
i'll take another look once i get my winnt build finishes.
Assignee | ||
Comment 34•24 years ago
|
||
darin, don't bother spending anymore time on this. It's not a networking
problem. We're just closing the socket down after an abort call and i still have
async write code that's attempting to write to the socket.
this is my bug.
Assignee | ||
Comment 35•24 years ago
|
||
re-assigning. I believe I have a fix.
Assignee: varada → mscott
Status: ASSIGNED → NEW
Comment 36•24 years ago
|
||
great!
Assignee | ||
Comment 37•24 years ago
|
||
Assignee | ||
Updated•24 years ago
|
Status: NEW → ASSIGNED
Whiteboard: [nsbeta1+] - hard to reproduce; needs darin investigation → [nsbeta1+] fix in hand r=? sr=?
Assignee | ||
Updated•24 years ago
|
Whiteboard: [nsbeta1+] fix in hand r=? sr=? → [nsbeta1+] fix in hand r= sr=sspitzer
Comment 38•24 years ago
|
||
r/sr=bienvenu
Assignee | ||
Comment 39•24 years ago
|
||
fix checked into the tip. branch coming up.
Assignee | ||
Comment 40•24 years ago
|
||
fixed on both the tip and the trunk. However the message still won't be sent
because of the long lines.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Comment 41•24 years ago
|
||
using buildid: 2001053106 trunk builds on win98,linux,mac I was not able to
reproduce the crash. However, I saw the crash on suresh winNT machine. So I
used suresh's machine to verify that the crash was fixed with buildid:
2001-06-06-12-0.9.1
Also checked on branch builds: 2001-06-06-12-0.9.1 win98, 2001060611-0.9.linux,
2001-06-06-14-0.9.1
The original crash problem replying to this email has been fixed. But as mscott
mentioned the send still fails with the long lines error.
As per comments in 2001-06-05 10:00 by Brade not opening a new bug since bug
67334 logged. If this appears to be a differnt problem please comment in the
bug so I can log a new one for send failing with long lines
Keywords: vtrunk
Comment 42•23 years ago
|
||
verified using turnk build
2001081506 win98, linux
2001081706 mac
Status: RESOLVED → VERIFIED
Updated•20 years ago
|
Product: MailNews → Core
Updated•16 years ago
|
Product: Core → MailNews Core
You need to log in
before you can comment on or make changes to this bug.
Description
•