Closed Bug 1745205 Opened 3 years ago Closed 2 years ago

"Empty Trash on exit" account setting fails to work for imap accounts using oauth2 authentication

Categories

(Thunderbird :: Account Manager, defect)

Thunderbird 96
defect

Tracking

(thunderbird_esr91 wontfix)

RESOLVED FIXED
102 Branch
Tracking Status
thunderbird_esr91 --- wontfix

People

(Reporter: s.rader, Assigned: gds)

References

Details

Attachments

(3 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:95.0) Gecko/20100101 Firefox/95.0

Steps to reproduce:

96.0b1 is the version I am currently using. I am forced to manually delete emails from Trash.

Please use exact wording - it helps move the bug forward more quickly. Thanks.

Component: Untriaged → Account Manager
Summary: I previously had Thunderbird set to empty Trash folder upon closing the app. It no longer seems to do that despite trying to reset various options. → "Empty Trash on exit" account setting fails to work

imap account?
Then a match to Bug 1504831 - Empty Trash on exit broke ( imap) ??

Flags: needinfo?(s.rader)

I am still am experiencing this problem. and need to manually delete emails from the Trash folder both for the Inbox Trash folder as well as the Trash Folder in my Local Folders.

Much like in Outlook email, I was able to set the application to automatically delete files when exiting email. After I had Thunderbird automatically updated, this function no longer worked. I have tried changing settings in the Thunderbird application as well as changing the settings speicif to the Trash Folder. Nothing I have attempted have worked.

I am unclear how to explain this problem in better detail.

Stan Rader
s.rader@cox.net
xxx-xxx-xxx (redacted)

Flags: needinfo?(s.rader)

I have the same problem. Used to work with the settings "Empty Trash on Exit". Now have to delete messages manually.

Please see bug 1504831, specifically bug 1504831 comment 22. Does it make sense?

Using 99 beta on gmail account, this works for me

This has just started working, Just installed 99. and srrestarted a couple of times.

Bug 1504831 comment 22 is more than two years old. Tested with Yahoo(), Hotmail/Outlook and some other non-Gmail IMAP account: All worked. If you use "File > Exit" you can see the messages disappearing before the window closes.
(
) Yahoo behaviour was a bit flaky (not the best mail provider), but it worked in the end.

Closing Thunderbird using the window X does not clear trash the first time. Seems to need two or three close / open cycles to clear the folders (I'm using unified folders). I doubt whether many people use File>Exit.
I'm using POP3 account with btinternet.com - although the provider should not make a difference.

There might be some sort of design issue. There's little time from using "File > Exit" or clicking the [X], even less for the latter, so if the messages don't get deleted due to a somewhat laggy server during that short timespan, well, they don't get deleted. Once the application is closing, especially via the [X], one can't delay it much to wait for a successful IMAP response, it's more like a hit and run.

Gene, where is this implemented? The application exits soon after without a chance to check for successful deletion or even retry.

Flags: needinfo?(gds)

On clicking the X to shutdown, tb runs the "deleteallmessages" url on the trash folder, this causes each message to be marked deleted via imap and then after the server acks each messages as deleted, tb sends expunge. Only after the expunge is acked by the server does tb logout from the server.
I tried this with just one message in trash and with many in trash and it seems to work for me on dovecot server.
With the server's imap port 143 blocked, on shutdown there was a delay due to network error but tb eventually shutdown (maybe 5 second wait) even though it couldn't mark messages deleted and expunge. In this case, on tb restart, with the port un-blocked, the messages were still there, of course. But on next shutdown they were all marked deleted and expunged before tb shutdown.
Does the number of messages in trash have an effect, i.e., the more messages in trash the more likely they don't get removed?
Can you provide an IMAP:5 log that shows the problem of tb shutting down before the messages are all marked deleted and expunged? I would be curious to see if this is really happening as you proposed. Here it appears that tb is allowing plenty of time for the deletions to occurs before going away. I only need to see the log from where the url "deleteallmessages" occurs to the end, so you can edit out the top part of the log before attaching it above.
https://wiki.mozilla.org/MailNews:Logging
Thanks!

Flags: needinfo?(gds)

There are two cases: Deletion from trash works upon exit or it doesn't. In the former case, "deleteallmsgs" is found in the log. In the latter case, it's not found in the log, so no surprise that nothing is deleted. This is what we've done: Copied a message from the inbox to the trash, since we can't keep deleting "good" messages for testing purposes. Then closed the application by clicking on the [X]. Here are all the lines of the log referring to the Yahoo server from the point the message was copied and we opened the trash folder:

2022-04-11 19:27:08.788000 UTC - [Parent 17236: IMAP]: I/IMAP 25770316000:imap.mail.yahoo.com:S-Trash:CreateNewLineFromSocket: 
2022-04-11 19:27:08.788000 UTC - [Parent 17236: IMAP]: I/IMAP 25770316000:imap.mail.yahoo.com:S-Trash:CreateNewLineFromSocket: )
2022-04-11 19:27:08.788000 UTC - [Parent 17236: IMAP]: I/IMAP 25770316000:imap.mail.yahoo.com:S-Trash:STREAM:CLOSE: Normal Message End Download Stream
2022-04-11 19:27:08.788000 UTC - [Parent 17236: IMAP]: I/IMAP 25770316000:imap.mail.yahoo.com:S-Trash:CreateNewLineFromSocket: 23 OK UID FETCH completed
2022-04-11 19:27:13.731000 UTC - [Parent 17236: IMAP]: I/IMAP 25770316000:imap.mail.yahoo.com:S-Trash:SendData: 24 close
2022-04-11 19:27:13.731000 UTC - [Parent 17236: IMAP]: I/IMAP 25770304000:imap.mail.yahoo.com:A:SendData: 93 logout
2022-04-11 19:27:13.731000 UTC - [Parent 17236: IMAP]: I/IMAP 2576c26a000:imap.mail.yahoo.com:S-INBOX:SendData: 53 close
2022-04-11 19:27:13.732000 UTC - [Parent 17236: IMAP]: I/IMAP 25770316000:imap.mail.yahoo.com:S-Trash:SendData: 25 logout
2022-04-11 19:27:13.733000 UTC - [Parent 17236: IMAP]: I/IMAP 2576c26a000:imap.mail.yahoo.com:S-INBOX:SendData: 54 logout
2022-04-11 19:27:14.104000 UTC - [Parent 17236: IMAP]: I/IMAP 25770304000:imap.mail.yahoo.com:A:TellThreadToDie: close socket connection

So are you saying you randomly either see or don't see "deleteallmessages" occur?
By any chance do you see all messages getting marked deleted in trash, e.g.: S-ttrash:SendData: 20 store 1:* +FLAGS.SILENT (\Deleted) even without the deleteallmessages?
Also, is problem only on yahoo?

Call stack when it works:

xul.dll!nsImapService::DeleteAllMessages(nsIMsgFolder * aImapMailFolder, nsIUrlListener * aUrlListener, nsIURI * * aURL) Line 1555	C++
xul.dll!nsImapMailFolder::EmptyTrash(nsIMsgWindow * aMsgWindow, nsIUrlListener * aListener) Line 1394	C++
xul.dll!nsMsgAccountManager::CleanupOnExit() Line 1497	C++
xul.dll!nsMsgMailSession::RemoveMsgWindow(nsIMsgWindow * msgWindow) Line 346	C++

Sadly the Yahoo service is so poor that right now, after one successful attempt, we can't even open the trash folder due to constant connection errors. The code in nsMsgAccountManager::CleanupOnExit() is rather complicated, so it's well possible that EmptyTrash() isn't even called.

We finally managed to access the trash folder and when closing the application, EmptyTrash() was called and failed straight away here:
https://searchfox.org/comm-central/rev/13a350796cd3b395cf511245c91aea751382d9fc/mailnews/imap/src/nsImapMailFolder.cpp#1351
Sadly we couldn't reproduce this a second time, maybe we made a mistake.

However, we managed to exit twice without emptying the trash and EmptyTrash() was not called here:
https://searchfox.org/comm-central/rev/13a350796cd3b395cf511245c91aea751382d9fc/mailnews/base/src/nsMsgAccountManager.cpp#1497

So in summary, there are cases where EmptyTrash() is not called, and unless we were hallucinating, it also failed once on GetTrashFolder(). Can GetRootFolder() fail? It's called here
https://searchfox.org/comm-central/rev/13a350796cd3b395cf511245c91aea751382d9fc/mailnews/base/src/nsMsgAccountManager.cpp#1458
and here:
https://searchfox.org/comm-central/rev/13a350796cd3b395cf511245c91aea751382d9fc/mailnews/imap/src/nsImapMailFolder.cpp#4640

So are you saying you randomly either see or don't see "deleteallmessages" occur?

Yes, see above.

We added some debugging code like so here:
https://searchfox.org/comm-central/rev/13a350796cd3b395cf511245c91aea751382d9fc/mailnews/base/src/nsMsgAccountManager.cpp#1461

      printf("=== 0\n");
      if (root) {
        printf("=== 1\n");
        nsString passwd;
        bool serverRequiresPasswordForAuthentication = true;
        bool isImap = type.EqualsLiteral("imap");
        if (isImap) {
          server->GetServerRequiresPasswordForBiff(
              &serverRequiresPasswordForAuthentication);
          server->GetPassword(passwd);
        }
        printf("=== 2\n");
        if (!isImap || (isImap && (!serverRequiresPasswordForAuthentication ||
                                   !passwd.IsEmpty()))) {
          printf("=== 3\n");

In the cases where it fails, "3" is not printed. So IOW, the account is not connected any more (due to the poor Yahoo performance) and the password is empty, so it wouldn't be able to reconnect. No idea why the password is already gone.

I don't currently have an yahoo account running. I don't remember if yahoo supports oauth2? If so, and you are using it, getting a conventional password may be the problem.

Edit: I set up my yahoo account and, yes, it supports oauth2. However, the connection to any folder (mailbox) is very intermittent and since unable to sometimes access Trash, it skips emptying it on TB exit.

Yes, Oauth2, it's been like this for a while. So you're saying that if the account got disconnected (which happens frequently), checked here:
https://searchfox.org/comm-central/rev/13a350796cd3b395cf511245c91aea751382d9fc/mailnews/imap/src/nsImapIncomingServer.cpp#1854
there won't be a password and emptying the trash won't be even attempted. We'll see whether this makes an improvement:
https://github.com/Betterbird/thunderbird-patches/blob/main/91/bugs/1745205-empty-trash-oauth2.patch

I tried ignoring the lack of a password and it didn't matter. It tries to get a valid connection/select to Trash and it still fails due to flaky yahoo connections.

@@ -1452,20 +1448,22 @@ nsMsgAccountManager::CleanupOnExit() {
       if (root) {
         nsString passwd;
         bool serverRequiresPasswordForAuthentication = true;
         bool isImap = type.EqualsLiteral("imap");
         if (isImap) {
           server->GetServerRequiresPasswordForBiff(
               &serverRequiresPasswordForAuthentication);
           server->GetPassword(passwd);
         }
+        printf("gds: isImap=%d(bool), sRPFA=%d(bool), pwdEmpty=%d(bool)\n",
+            isImap, serverRequiresPasswordForAuthentication, passwd.IsEmpty());
         if (!isImap || (isImap && (!serverRequiresPasswordForAuthentication ||
-                                   !passwd.IsEmpty()))) {
+                                   /* !passwd.IsEmpty()*/1))) {
           nsCOMPtr<nsIUrlListener> urlListener;
           nsCOMPtr<nsIMsgAccountManager> accountManager =
               do_GetService(NS_MSGACCOUNTMANAGER_CONTRACTID, &rv);
           if (NS_FAILED(rv)) continue;
 
           if (isImap) urlListener = do_QueryInterface(accountManager, &rv);
 
           if (isImap && cleanupInboxOnExit) {
             nsTArray<RefPtr<nsIMsgFolder>> subFolders;

Ok, I think we've seen this problem with yahoo before. It doesn't always accept the default of 5 cached connections. I set it down to 4 and it didn't help. But setting to 3 forces tb to try to re-use existing connections when selecting a new folder like Trash without spinning up another 4th connection that apparently yahoo doesn't like. With cached set to 3 (advanced server setting) I don't see problems when selecting different folders like before (connection failed pop-ups) and the response is much better and Trash gets selected and marked deleted and expunged on shutdown.
Addendum
Here's an example of a yahoo user reducing cached conns to 3 to fix a similar problem: bug 1595169
Note also that reporter Stan is on cox network. It possibly uses a yahoo backend server for email, not sure.

I haven't had time to work out to do the logging, however all these comments refer to IMAP, whereas I use POP3. Same principle of timing would apply though. It takes three close / open cycles for all the trash to be cleared.

Anje, can you reproduce?
do you have pop?

Flags: needinfo?(anjeyelf)

I guess I'm not supposed to do this as writing into bug reports saying, "This is happening to me too!" is considered bad practice — but in this case I'll do it anyway.

So far the discussion has focused on Yahoo, I do have this problem with Yahoo. However, I also consistently have this problem with Hotmail as well. All my email accounts are IMAP type.

I couldn't help with fixing this as you guys are operating at an expertise level that is beyond me.

Pehaps the straightforward design change would be to have "Clear trash on exit or start". That would avoid the timing problem. May be simpler than to try to solve the actual problem.

Roger Mullenger said, Pehaps the straightforward design change would be to have "Clear trash on exit or start".

Might there not be a security issue with that? Some folks might be relying on empty on exit for security reasons.

I have had some background communication with Rachel Martin and was advised to download the latest build of Betterbird with a patch trying to address the issues here. I downloaded BetterbirdPortable-91.8.0-bb29-latest-build.en-US.win64.exe and did the following tests using it against two Hotmail accounts and one Yahoo account.

===================================================

Following 3 tests done with Betterbird 'Latest Build' Portable as supplied:

(1) Shutdown using window [X] button. Result: All the trash folders; two Hotmail and one Yahoo got emptied.

(2) Repeat of '(1)' above. Result: None of the Hotmail trash got emptied. Yahoo trash did get emptied.

(3) Repeat of '(1)' above. Result: None of the Hotmail trash got emptied. Yahoo trash did get emptied.

===================================================

Following 3 tests done with Betterbird 'Latest Build' Portable as supplied:

(1) Shutdown using 'File > Exit' route. Result: None of the Hotmail trash got emptied. Yahoo trash did get emptied.

(2) Repeat of '(1)' above. Result: None of the Hotmail trash got emptied. Yahoo trash did get emptied.

(3) Repeat of '(1)' above. Result: None of the Hotmail trash got emptied. Yahoo trash did get emptied.

===================================================

Following 3 tests done with Betterbird 'Latest Build' Portable as supplied but with "Maximum number of server connections to cache" set to '3':

(1) Shutdown using window [X] button. Result: Both Hotmail trash got emptied. Yahoo trash got emptied.

(2) Repeat of '(1)' above. Result: Both Hotmail trash got emptied. Yahoo trash got emptied.

(3) Repeat of '(1)' above. Result: Both Hotmail trash got emptied. Yahoo trash got emptied.

===================================================

Following 3 tests done with Betterbird 'Latest Build' Portable as supplied but with "Maximum number of server connections to cache" set to '3':

(1) Shutdown using 'File > Exit' route. Result: Both Hotmail trash got emptied. Yahoo trash got emptied.

(2) Repeat of '(1)' above. Result: Both Hotmail trash got emptied. Yahoo trash got emptied.

(3) Repeat of '(1)' above. Result: Both Hotmail trash got emptied. Yahoo trash got emptied.

===================================================

As an aside it seemed that setting the "Maximum number of server connections to cache" to '3' cured the long-standing random connection problems I was experiencing with the Yahoo server. (Killed two birds with one stone! Maybe.)

I should have added to the above that when I set "Maximum number of server connections to cache" to '3' I did so for both Hotmail accounts and for the Yahoo account.

Windows 10
TB version 98.8.0

Tested on POP:
Deleted emails
Set to 'Empty Deleted folder on exit'
Exit and restart Thunderbird
Result: Trash got emptied.

Tested on gmail imap:
Deleted emails
Set to 'Empty Deleted folder on exit'
exit and restartThunderbird
Result: Trash got emptied

So all seems ok in version 91.8.0, but reporter is using 96.0b1

Flags: needinfo?(anjeyelf)

We've done some more testing with this code:

        if (isImap) {
          server->GetServerRequiresPasswordForBiff(
              &serverRequiresPasswordForAuthentication);
          server->GetPassword(passwd);
          server->GetAuthMethod(&authMethod);
          printf("=== serverRequiresPasswordForAuthentication %d %d\n", serverRequiresPasswordForAuthentication,
          passwd.IsEmpty());
        }
        if (!isImap || (isImap && (!serverRequiresPasswordForAuthentication ||
                                   !passwd.IsEmpty() ||
                                   authMethod == nsMsgAuthMethod::OAuth2))) {

The print statement showed "=== serverRequiresPasswordForAuthentication 1 1", so the if block would not have been entered with the original code. With our patch, the if block is entered, and, at least as Peter has already confirmed, the trash is in fact emptied for Yahoo. So based on this, we disagree with Gene's comment #18 and we do consider the patch helpful, at least it doesn't do any damage.

Hotmail will need further investigation if we can get the "empty trash on exit" to fail at all.

As for emptying the a local trash for a POP3 account. Please repair, compact and repair the Trash folder, or alternatively, delete it manually from the profile. There doesn't appear to be a reason for emptying a local trash not working other than a broken folder.

Our Hotmail isn't with Oauth2, so our patch makes no difference. We don't see any connection problems, in our testing the trash is always emptied, even with max. connections at 5. We suggest to take the patch for TB to mitigate the issue for Oauth2 servers.

Yes, passwd is always empty with yahoo and probably anything else using oauth2. So the only reason it sometimes works is because serverRequiresPasswordForAuthentication is false since the connection to Trash is sometimes successfully authenticated so EmptyTrash() gets called and trash gets marked deleted and expunged. See https://searchfox.org/comm-central/rev/91923108c54cd5e574fbaeed2b6af0cbeee37d37/mailnews/imap/src/nsImapIncomingServer.cpp#1858
So not sure check for oauth2 is really helpful since EmptyTrash will still fail if there is no authenticated connection when serverRequiresPasswordForAuthentication is true.

FYI, there's another bug related to oauth2 where GetPassword() called here: Bug 1714572 comment 36
Edit to FYI...: Actually this describes it better -- bug 1668833 comment 2

So the only reason it sometimes works is because ...

Well, observed behaviour was: "=== serverRequiresPasswordForAuthentication 1 1", so the user wasn't authenticated at that point (and no password due to Oauth2), however, the trash got emptied. Isn't it possible that the next IMAP action will re-authenticate? That's the idea of the original code: If not authenticated but a password is present, then try emptying the trash since the called function EmptyTrash() will/may/should do the right thing and re-authenticate.

Assuming yahoo only allows 3 connections, it looks to me like (based on testing) it's still going to fail to empty trash, even with suggested oauth2 fix, if there are 3 connections and none of them are selected to Trash since must select Trash (4th connection) to empty it.

Another thing I see is when there is a failure to get the 4th connection, when TB shuts down, I always see serverRequiresPasswordForAuthentication true indicating server is not authenticated. Once there is a successful authentication on any connection, this should be false (based on !m_userAuthenticated). But m_userAuthenticated gets set false when there is a connection error here: https://searchfox.org/comm-central/rev/91923108c54cd5e574fbaeed2b6af0cbeee37d37/mailnews/imap/src/nsImapIncomingServer.cpp#1850. Other that the marked as invalid "turbo" bug mentioned in bug 143848 this doesn't seem to be needed and removing it allows serverRequiresPasswordForAuthentication to remain false so EmptyTrash() is still attempted (but may still fail).

Not sure why oauth2 results in an empty password. I haven't looked into that at all yet. Not sure if ignoring the password when oauth2 is correct and not sure commenting out m_userAuthenticated = false, as I suggest, is right.

Another possibility is to just get rid of the "if" completely and do everything under it unconditionally:

if (!isImap || (isImap && (!serverRequiresPasswordForAuthentication ||
                                   !passwd.IsEmpty()))) {

Or maybe should just leave everything as-is for now since I'm not sure how much time this code has left before it transitions to JS and just remind users they may need to reduce cached conns to 3 for yahoo and other mail providers using yahoo (e.g., AOL).

Note: I just tested again with yahoo and now it's allowing 5 connections and don't see any connection errors. Maybe it rolled to a different back-end server?

Sorry to repeat: We saw "=== serverRequiresPasswordForAuthentication 1 1" repeatedly followed by successful trash emptying. Without our patch, in case of "1 1", the trash emptying wouldn't have happened. That was with 5 connections.

In general, when the user sets up a Yahoo or Hotmail account, why doesn't the system automatically set up 3 connections instead of 5?

gene smith said, "Assuming yahoo only allows 3 connections, it looks to me like (based on testing) it's still going to fail to empty trash, even with suggested oauth2 fix..."

As a mere mortal ordinary everyday user I have to say that with the patch my Yahoo Trash is getting emptied every time (which surely wasn't happening before the patch). If I can say so gene I cannot follow your logic at all on this. There is clear evidence that the patch works but this seems not to figure in your overall reasoning. Even if the patch only offered a 'sometimes' improvement then, from the point of view of ordinary users that would be an improvement to implement. As it is, on my machine, the patched program works every time for Yahoo. On that basis, the decision to include/not include the patch is pretty much a no-brainer — the patch should be implemented. I can't at all see how you don't arrive at that conclusion.

At worst the patch wouldn't do any harm, at best it cures the issue — so what is the problem? I am totally baffled by the tack you seem to be taking, it makes no sense.

gene smith said, "Assuming yahoo only allows 3 connections, it looks to me like (based on testing) it's still going to fail to empty trash, even with suggested oauth2 fix..."

You left off an important part of the quote: "Assuming yahoo only allows 3 connections, it looks to me like (based on testing) it's still going to fail to empty trash, even with suggested oauth2 fix, if there are 3 connections and none of them are selected to Trash since must select Trash (4th connection) to empty it."

If possibly you had made a successful connection and select to Trash before shutting down, tb doesn't have to make a 4th connection to empty Trash since it just uses the existing Trash connection. If you didn't open Trash at any time during the session then maybe I'm wrong. But this is the behavior I observed with my tests with yahoo not liking a 4th connection.

I'll test it again but the last time I tried, yahoo was actually allowing 5 connections with no errors :( .
I haven't really tested this with "hotmail". Can I assume the hotmail is the same as outlook.com (which I do have an account with)? Also, is your hotmail/outlook account also using oauth2 authentication on port 993. My outlook account is current using plain password authentication on port 143 with STARTTLS and I'm now seeing 5 connections to it being OK.

From comment 33:

In general, when the user sets up a Yahoo or Hotmail account, why doesn't the system automatically set up 3 connections instead of 5?

I think this has been suggested before. But it seems to vary depending on which yahoo/hotmail server. Also, several ISPs use yahoo as their server without advertising it (e.g., Verizon possibly) so those may not be possible to know exactly all the time. Also, I've never been asked to support the account setup tb software, and even this bug is in an area I've never been before.

gene:

With the patch Yahoo has emptied every time without fail and this irrespective if I inspect Trash before shutting down or I don't. My reason for keeping the 3 connections with Yahoo is that since I started using that setting I have had not one single 'failed to connect because of server issues" warnings/alerts while using the account's folders. That is a very significant improvement to the usability of Yahoo so it would seem to me that the 3 connections is a setting that works well (irrespective of how it might affect emptying of Trash).

For Hotmail you are asking questions of me to which I don't easily know the answers — to be honest I'm a fairly naïve user of email (as most users would be). However, I have uploaded two image files showing the settings that are being used by both of my Hotmail accounts (though obviously each account has its own unique email address). I should add that I did not make the settings for Hotmail myself, all the settings were automatically created using Thunderbird's wizard at the time, so all settings are straight out of Thunderbird's database.

With respect to looking in the Trash folders prior to shutting down I have now done some experiments on this and would say the following:

For Hotmail accounts clicking on at least four folders (but avoiding the Trash folder) then, yes, that seems to increase the likelihood that the Hotmail Trash won't get emptied. (I would though note of this that using the 'File > Exit' route to shutdown does seem to have some bearing on this — I most often have success with the Hotmail accounts by using that method of shutting down.

For the Yahoo account (and with the patch) the Trash always gets emptied (now) irrespective of what folders I visit and again deliberately avoiding clicking on the Trash before shutdown is made.

Attached image Hotmail - Server Settings.png (deleted) —

Hotmail - Server Settings

P. Cramond, I think you have to go back to 5 cached connections for selecting or not selecting Trash to have an effect on if it's emptied. I'm not sure from your comment 36 if you tested with 5 connection or just remained at 3. With just 3 connection, with no mods/patches (TB release) you should see no problems. (My yahoo account is still accepting 5 connection but my AOL test account, which also uses yahoo, is still only allowing 3, so I can still test with it.)

Your setting for hotmail appear to be the same server as I'm set-up for outlook. So I think we can say that hotmail==outlook.com. However, I'm now getting 5 good connection to the hotmail/outlook server: outlook.office365.com.

My tests in comment 36 were done using 3 connections for both Hotmail accounts and for the Yahoo account.

I've now again done some tests with 5 connections for Yahoo and I'm getting the same result using the patched program, Trash always gets emptied. That said, with 5, I'm back to getting random connection issues on Yahoo with ordinary use of the account (just clicking around on some folders). So, as best as I can see I'm better of with setting Yahoo to 3 connections as this has benefits for ordinary usage, irrespective of what is happening with Yahoo Trash (which is now, with the patch, always being emptied anyway, irrespective of if I'm using 5 or 3 connections).

Hotmail I've now tested with 5 connections again and I'm still getting random results, sometimes the trash gets emptied, sometimes not, which is the same as what is happening when only using 3 connections for Hotmail.

From my own view Yahoo has been dealt with now via the patch — no emptying the Yahoo Trash issues any more — so I would regard the patch as the solution to the Yahoo problem, the patch works, it solves the problem.

P.C. --
Maybe you are doing something like this: Start tb with Inbox previously selected. Copy some messages from INbox to trash and shutdown tb. Yes, in that case Trash will be emptied because you have only made 1 connection. If instead, on startup you click around to several non-Trash folders until you see an error (due to trying to make more than 3 connections) and then shutdown tb, you probably won't see Trash emptied, even with the patch. At least that's what I see.

Also, according to your attachment above, hotmail is not using oauth2, so it won't be affected by the patch for oauth2 accounts like yahoo. Maybe it's seeing an empty normal password too due to ForgetSessionPassword() -- TBD.

Note to self:
My idea was to remove the m_userAuthenticated = false set here: https://searchfox.org/comm-central/rev/91923108c54cd5e574fbaeed2b6af0cbeee37d37/mailnews/imap/src/nsImapIncomingServer.cpp#1850
However, I see that ForgetSessionPassword() is called a lot from Observe(): https://searchfox.org/comm-central/rev/91923108c54cd5e574fbaeed2b6af0cbeee37d37/mailnews/base/src/nsMsgIncomingServer.cpp#106
Observe() is frequently called with parameter aTopic = "passwordmgr-storage-changed" but I'm I don't see that any change has actually been made to the password storage manager and this happens even when there are no errors occurring at all.
There was a proposed changed that caused ForgetSessionPassword() to only be called when the password mgr change is login info in bug 1695703 at bug 1695703 comment 26. This would avoid having to remove m_userAuthenticated = false in ForgetSessionPassword(). However, that change caused a unit test failure and was backed out so ForgetSessionPassword() gets called if loginInfo is null.

gene:

I would say it would be possible to leave Yahoo out of this now. The patch works, period, and I'll stick with 3 connections for Yahoo as that works in solving other long-standing persistent connection problems I experienced which have nothing to do with Trash. Yes, I do know that the patch does nothing as far as Hotmail is concerned.

For Hotmail, yes, what you are saying is true: If I click on the Trash folder as the last folder I click on before before shutting down generally the Trash will get emptied — this is irrespective of whether I'm using 3 or 5 connections. If, as you say, I click on multiple non-Trash folders before shutdown then it seems the Trash will most likely not get emptied on exit — this is irrespective of whether I'm using 3 or 5 connections — that said, sometimes it does get emptied if I do this, so there seems some element of randomness involved at times. I would have to say that for Hotmail it seems not to matter how many multiple folders I click on I never get a 'connection error'. The only thing I would add is that it seems there is more likelihood of Hotmail Trash being emptied if I shutdown using 'File > Exit' route (and not by the window [X] button).

The coding stuff goes right over my head, I'm just an ordinary fairly naïve user. But from what I think I'm reading it seems that for both Hotmail and Yahoo it boils down to passwords being (re-)sent to the servers at the point of initiation of shutdown. If I am right in thinking that then it would seem that the thing to do is send the password anyway, in all instances no matter what is happening.

Please remember: Yahoo uses Oauth2, Hotmail, at least for us, a regular password. So you're comparing apples to oranges. The patch addressed a problem with Oauth2 servers.

I agree that if the trash folder is selected then it stands more chance of being emptied. I have 5 email accounts and they gradually get emptied on each close / open.
I changed POP3 to IMAP to test, and it should be better, however it is a nightmare, being so slow (with btinternet.com) with hangs and "not responding". Also complete gibberish with superimposed texts, after 5 minutes of loading the message, which increased from 3kB to 345MB by joining all the emails in the inbox together. May change back to POP3 although losing synchronisation. [I know it's separate from this thread but obviously IMAP is flaky, with issues with reliability & speed with my system].

Roger, above you say you are using 99. I assume that must be a beta. The problem with joining several emails into one is post-release-91 beta specific. AFAIK, there is no problem like this with the official release 91.* (However, you may need to repair folder that had this joining issue even after going back to 91.* if you choose to do that. Also, to go back you have to create a new profile or run with parameter --allow-downgrade. If you create new profile, of course no repair is needed.)

P.C., I didn't see a difference with the shutdown method as far as what's sent via imap. But I'll check again.

I did see one issue that affects getting a connection to Trash on shutdown. I added an early occurrence of the URL to discover folders called "discoverallboxes": Bug 163964. This allows users with a huge number of folders to start up faster. But after the URL finishes it's connection is not re-used for selection of Trash but instead a new connection is created (or for yahoo, attempted to be created). If I force the now not-busy connection used for discovery to be reused earlier, a 4th connection to yahoo is not needed and trash gets emptied. But this isn't a complete solution.

Actually, yahoo and friends (e.g., AOL) don't have a problem with creating a 4th connection. What happens is that after it is created a failure "rate limit" is reported by yahoo and an imap Bye and then NO response which causes the the server to disconnect the completely authenticated 4th connection. Re: bug 1727971. This seems to be a difficult to handle yahoo bug that can most easily be solved by limiting connections to 3.

Here's some more info on the yahoo connection issue. In my testing I see that yahoo allows up to 3 quickly occurring connections. But if you quickly attempt a 4th connection, yahoo accepts it and allows it to be authenticated (accepts the oauth2 token/password) but on the next commands, e.g., capability, it responds like this and then sends a tcp FIN to drop the connection:

55 capability
* BYE IMAP4rev1 Server logging out
55 NO [LIMIT] CAPABILITY Rate limit hit.

However, if this 4th connection is delayed some (maybe between 30s and 1 minute) the 4th connection completely succeeds and with another 30s to 1 minute wait, a 5th error free connection succeeds without the untagged BYE and tagged NO response to post-authenticate commands. The delay can be accomplished by waiting 30s to 1 minute before selecting other folders. The number of connection actually active can be seen in linux with the command lsof -i tcp:993 -P (probably there is a similar command other OSes, not sure).

Another thing I've discovered is that yahoo only keeps an idle connection alive for 5m before sending tcp FIN to terminate the connection. But the connection is maintained if TB sends an imap command before 5m elapses. TB sends by default a TCP keepalive every minute or so but yahoo ignores it and requires an "application layer" command to avoid dropping the connection. The default "biff" time for TB is 10 minutes. So after 5 minutes yahoo will disconnect and at the 10 minute point, TB will force a reconnect. So to keep a constant connection on INBOX or other folder being checked for new mail, the biff timer should be set to 4 minutes.

Something like the patch above in comment 26 is needed so oauth2 servers allow trash and inbox to be cleaned up on exit. However, this only fixes the problem of the exit cleanup being skipped if max cached connection is set to 3 or less. If cached connection remains at the default of 5, TB will often need to create a 4th connection and, if the previous 3 are still active and enough time has not elapsed so that yahoo drops the 4 connection, the attempt to select Trash and empty on shutdown it will still fail. So with the patch the max cached connections should be set to 3 to work around this bug.

So the attached patch will fix yahoo (and friends, e.g., AOL) as long as the number of cached connection is set to 3 or less. It will also allow other oauth2 imap servers to cleanup inbox and empty trash on TB shutdown even if they support the default 5 connection without rate limiting (as non-yahoo oauth2's do).

Note: I tried to determine why oauth servers return an empty password with the GetPassword() call but was unsuccessful, so I think this is a reasonable work-around. The solution was actually suggested by commenter Rachel Martin in Comment 28 above -- Thanks!

I did a "try" run and I think it's OK: https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=fa1e200adc100b84c4b9551c6fe4e56dbc958501

Assignee: nobody → gds
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #9275657 - Flags: review?(benc)

Updated the "User" field of the patch. Otherwise, still the same and yes I did copy it from the github link in comment 17. Since we discussed and sort of worked on this together I didn't realize there was an attribution issue.

Attachment #9275657 - Attachment is obsolete: true
Attachment #9275657 - Flags: review?(benc)
Attachment #9275733 - Flags: review?(benc)

(In reply to gene smith from comment #49)

Since we discussed and sort of worked on this together I didn't realize there was an attribution issue.

The main effort here was to debug it (comment #15) and identify that this block
https://searchfox.org/comm-central/rev/ee5e9e3c101395191143f08df8d20f6ffe5e594a/mailnews/base/src/nsMsgAccountManager.cpp#1470
wasn't executed. We're grateful for you input pointing out that the password is empty due to the use of OAuth2 at Yahoo.

Comment on attachment 9275733 [details] [diff] [review]
1745205-fix-trash-empty-at-shutdown-on-imap-oauth2-servers.patch

Review of attachment 9275733 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM.
Seems like it'd be worth stepping back some time and figuring out a more general solution.  It seems wrong to have to do oauth-specific checks here. But I suspect that might lead down quite a deep rabbithole...
Attachment #9275733 - Flags: review?(benc) → review+

(In reply to Ben Campbell from comment #51)

Seems like it'd be worth stepping back some time and figuring out a more
general solution. It seems wrong to have to do oauth-specific checks here.
But I suspect that might lead down quite a deep rabbithole...

I kind of went into the "rabbithole" in comment 41 under "Note to self". The 2nd change mentioned there also fixes this bug ( bug 1695703 comment 26, as a side effect) and you had proposed it and it doesn't do oauth checks. However that caused test failure and was reverted. So I decided just to go with the "Rachel Martin" patch.

Target Milestone: --- → 102 Branch

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/391f699f48ed
Allow inbox cleanup & trash emptying on shutdown for imap oauth2 servers. r=benc

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED

I appreciate that this matter is now assessed as closed and the patch will be applied — which I think is a good decision. However, I would like to ask one question: will the patch deal with issue (at least for Yahoo) that in "Advanced Account Settings > Maximum number of server connections to cache" should be set to '3'? Will this be done automatically by the patched Thunderbird or will the user need to be informed of this and make that adjustment themselves?

(In reply to P. Cramond from comment #54)

I appreciate that this matter is now assessed as closed and the patch will be applied — which I think is a good decision. However, I would like to ask one question: will the patch deal with issue (at least for Yahoo) that in "Advanced Account Settings > Maximum number of server connections to cache" should be set to '3'? Will this be done automatically by the patched Thunderbird or will the user need to be informed of this and make that adjustment themselves?

The patch doesn't address the issue of setting Yahoo (and several other mail providers that use yahoo as the imap server, e.g., AOL) to a maximum of 3 cached connections. That would probably best be handled by the initial account setup software. That decision should probably be made by the project leaders, Magnus Melin or Wayne Mery who are CC'd on this bug.

Summary: "Empty Trash on exit" account setting fails to work → "Empty Trash on exit" account setting fails to work for imap accounts using oauth2 authentication

It's a shame yahoo haven't fixed their stuff. I guess we could have some way to get max number of connections, and check that when get get max connections. We could then ship such a pref. We'd ship something like
pref("mailnews.fixMaximumConnectionsNumber", "imap.mail.yahoo.com;3,imap.aol.com;3");

I implemented and tested something like what you suggest using the imap ID response like I just removed for doing the charter/spectrum "force select" thing at bug 1771409. Then I was searching for something and came across some relatively new yahoo developer info where they are adding new features.
Within that there is a link to a more detailed pdf. In the pdf there are listed imap servers for yahoo and aol having the new features:

staging.imap.mail.yahoo.com
staging.imap.mail.aol.com

When I use these as the TB "server name" setting I don't see the "rate" problem and I can make 5 connections on both the yahoo and aol servers, unlike with the default "server name" (same as above but without "staging").

So rather than adding code to adapt to the the yahoo/aol servers, should probably just let them fix the problem on their default servers.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: