485995 - not closing imap connections on shutdown

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[David :Bienvenu]

We're no longer closing our connections to the imap server gracefully on shutdown. In particular, we're not logging out or closing (in the imap protocol sense). I suspect this makes gmail in particular lose state changes, but in general, many imap servers would have an issue with this behavior.

This may have been caused by the changes I made to make imap closing happen on the IMAP thread, instead of blocking the UI thread. We attempt to close imap connections on shutdown when we get the notification that we're going offline. When we examine the transports for our cached imap connections, none of them are alive, I believe because necko has killed them as part of the shutdown process.

I believe moving closing of cached connections much earlier in the shutdown process would help with this a lot. I prefer that to the alternative of leaving it where it is but blocking the ui thread while the connections are closed. I'll see how feasible it is to know earlier that we're shutting down.

Comment 1

[David :Bienvenu]

Attachment: close cached connections right after shutdown tasks

this makes us close cached connections right after we finish things we do on exit, like emptying trash or expunging inbox.

I'm not quite sure how happy SeaMonkey will be with this, since this code is triggered by the last mail window getting closed, from nsMsgMailSession.cpp. I could #ifdef MOZ_THUNDERBIRD it if SM would prefer - though I know there's always a debate about whether all of mailnews should be shutdown in SM after the last mail window is closed, or whether it should all be running and checking for new mail.

Comment 2

[[e-mail]]

Comment on attachment 370066 [details] [diff] [review]
close cached connections right after shutdown tasks

We call this on exit? Seems to me that we call this every time we close a standalone message window. I guess you don't bother with them any more?

Comment 3

[David :Bienvenu]

(In reply to comment #2)
> (From update of attachment 370066 [details] [diff] [review])
> We call this on exit? Seems to me that we call this every time we close a
> standalone message window. I guess you don't bother with them any more?

We should only be calling it when the window count on the mail session goes to 0

  if (!mWindows.Count())

http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMsgMailSession.cpp#379

Comment 4

[[e-mail]]

Comment on attachment 370066 [details] [diff] [review]
close cached connections right after shutdown tasks

Oh right, I didn't see that bit. Well, I guess if we're already emptying the trash or whatever, closing cached connections can't do any more harm.

Comment 5

[David :Bienvenu]

Yes, that's my thinking as well.

Comment 6

[David :Bienvenu]

fix checked in.