206754 - Hitting Stop Button while downloading messages doesn't send QUIT to server

Status: NEW

(MailNews Core :: Networking: POP, defect)

Description

[Christian Eyrich]

Currently when hitting stop button nsPop3Protocol::Cancel is reached, calls some
other functions, but none sends the QUIT command to the server.

This causes two problems:
1. Some servers keep the season alive until timeout, so no login possible for a
few minutes.
2. According to RFC 1939 only when the client has issued the QUIT command, the
session enters the UPDATE state. Only in this state mails marked for deletion
going to be deleted on the server.

Comment 1

[Greg K.]

Dup of/related to bug 138632?

Comment 2

[Christian Eyrich]

No, unfortunately not. This bug is about stop while downloading mails.
But 138632 is about stop while logging in/connecting.

Comment 3

[Christian Eyrich]

Attachment: proposed patch

Either that's a feature (I'd say bad design then) to really abort and have the
post-state the pre-state, or a real bug.

If the latter, here's a patch.
It's only this one line because we can't set next_state = POP3_SEND_QUIT and
wait what happens since after Abort() the connection is lost, no more able to
send any byte.

Comment 4

[David :Bienvenu]

This looks potentially OK - You've tested it and it works in the sense that the
quit gets on the wire, as it were? I ask because I think SendData is
asynchronous, and we abort the connection immediately. What happens when we hit
cancel for a connection that never gets established, i.e., trying to connect to
a pop3 server that's down or not a valid host name?

Also, we're not going to try to commit our state in this case, I think - is that
wrong? We're not going through nsPop3Protocol::ProcessProtocolState() in this
case, are we? I think we want to be, if possible...

Comment 5

[Christian Eyrich]

Yes, the QUIT gets on the wire (verified with ethereal) and is recognized by the
server.
Hitting Stop for a connection that never gets established doesn't work at all
(see bug 138632).


And right, we're not going through ProcessProtocolState(). I only wanted to
change as little as possible - to be honest, I don't understand what
nsPop3Protocol::Abort() does.

I now tried
NS_IMETHODIMP nsPop3Protocol::Cancel(nsresult status)  // handle stop button
{
  m_pop3ConData->next_state = POP3_WAIT_FOR_RESPONSE;
  m_pop3ConData->next_state_after_response = POP3_SEND_QUIT;
}

and so going over POP3_SEND_QUIT, POP3_QUIT_RESPONSE and POP3_DONE.
It seems to work too (and same data on the wire according to ethereal).

But we'll have a problem if the server doesn't respond to our QUIT for what
reason soever -> we won't close the connection. And that would be far worse than
the current behaviour.

Comment 6

[David :Bienvenu]

it looks to me like the cancel code wants to get into the POP3_INTERRUPTED state

nsPop3Protocol::Abort makes it so the code that's writing the message into a
mailbox during pop retrieval clean up after an error correctly, e.g., not
leaving the mailbox with a partial message at the end.

We want to do things like having CommitState get called, because that handles
our pop3 uidl state (assuming it's in a correct state after stop is pressed).

I think we need to worry about the cleanup here - there's lots of state just
sitting around. We don't want to lose state, and we don't want to leak memory
when stop is pressed. I'm not sure what the answer is but I think we need to
find out...

Comment 7

[Christian Eyrich]

> it looks to me like the cancel code wants to get into the POP3_INTERRUPTED state

Yes, that would be better, it's unused for now. I was inattentive when looking
at the code.

> We want to do things like having CommitState get called, because that handles
> our pop3 uidl state (assuming it's in a correct state after stop is pressed).

Calling POP3_INTERRUPTED what leads to POP3_ERROR_DONE does the same as
nsPop3Protocol::Abort() plus CommitState().

Only the call to AbortMailDelivery() is in an if(m_pop3ConData->msg_closure) in
POP3_ERROR_DONE while it isn't in Abort() - but that looks ok.

Comment 8

[David :Bienvenu]

Comment on attachment 124078 [details] [diff] [review]
proposed patch

cleaning up request queue

Comment 9

[(not reading, please use [e-mail] instead)]

sorry for the spam.  making bugzilla reflect reality as I'm not working on these bugs.  filter on FOOBARCHEESE to remove these in bulk.

Comment 11

[David A. Cobb]

Still open?  What was the hold-up on David's 2004-11-29 patch?

Comment 12

[nuclear worm]

which version it's going to be included in??
It causes many troubles with various antispam/antivirus programs. After closing connections there's no QUIT and server doesn't delete mails (no UPDATE). So TB is stuck and can't download any mail from server. I think there's need to increase priority of this bug.

Comment 13

[Wayne Mery (:wsmwk)]

David? comment 8 still valid?

Comment 14

[Rez]

Guys, I don't understand the coding problem being discussed here, but FYI the STOP button works fine in old Netscape 3, under identical circumstances (or even simultaneously) and with the same POP3 servers. So it's not like it can't be done. I have other comments over at https://bugzilla.mozilla.org/show_bug.cgi?id=138632

Comment 15

[Wayne Mery (:wsmwk)]

<lonely patch looking for owner>

Comment 16

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Comment on attachment 124078 [details] [diff] [review]
proposed patch

This patch still applies! (but has got a r- with no updates since, so obsoleting)

$ patch --dry-run < ~/Desktop/tbTestPatches/206754.diff 
patching file nsPop3Protocol.cpp
Hunk #1 succeeded at 773 (offset 115 lines).

Comment 17

[Ludovic Hirlimann [:Usul]]

Christian,

would you consider submitting a patch addressing Bienvenue's comment ?

Comment 18

[Charles]

Before I go open a new bug, would this be the cause of the Stop button not doing anything in  the middle of a large move/copy of messages (over IMAP) from one server/account to another?

Comment 19

[Ludovic Hirlimann [:Usul]]

Removing myslef on all the bugs I'm cced on. Please NI me if you need something on MailNews Core bugs from me.

Comment 20

[Wayne Mery (:wsmwk)]

Possible for you to test this?

Comment 21

[Rez]

There is a comment over here
https://bugzilla.mozilla.org/show_bug.cgi?id=239616#c38
from 14 years ago, by an Andrew Sutherland, explaining the probable cause of this bug.

Bug is still there on my sundry SeaMonkey installs (2.4.x through 2.5.Current, on various Windows and Linux).

Comment 22

[doug2]

Attachment: TbirdTrace-StopButton.txt

Comment 23

[doug2]

I don't see evidence of a STOP in trace. I requested download and immediately hit the stop button. It appears the process ran through 4 accounts without stopping. See trace file.

Comment 24

[Dan Pernokis]

I tested on each PC by doing a GET MESSAGES (about 10 or 14 in the list) and then pressing the STOP button while the process was running. In all cases, nothing happened -- the GET MESSAGES kept on running, and the STOP button did not appear to "depress" (although other buttons like PRINT were active).

Running TB 91.13.1 (32-bit) on Windows 7 and TB 115.9.0 (64-bit) on Windows 10.

Just as an aside: I don't recall using the STOP button in real life for many years. (In fact I forgot it was there!) But I do think it should functionally stop TB's fetch process, whether or not it disconnects or otherwise affects outside processes.

Comment 25

[Rez]

Stop button appears to depress in SeaMonkey, but as noted, does nothing.

I did use it, in the ancient days when it worked, because timeout on a relatively slow connection can get downright painful. I still often wish for it.