1924918 - Compacting folders should be aborted at shutdown

Status: VERIFIED FIXED

(MailNews Core :: General, defect)

Description

[Geoff Lankow (:darktrojan)]

Like it says in the title. If Thunderbird is shutting down, compacting folders should be abandoned, and the backup files restored.

I think this would solve a bunch of our crashes where we try to access things that are no longer there. And even if it doesn't, we shouldn't keep doing things like compaction after the program goes into shutdown mode.

Comment 1

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1924918 - Stop compacting folders when shutdown begins. r=BenC!,#thunderbird-reviewers

Comment 2

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1924918 - Clean up test_folderCompact.js. r=#thunderbird-reviewers

This test is a mess. Let's drag it out of the dark ages.

Comment 3

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1924918 - Stop compacting folders when shutdown begins. r=BenC!,#thunderbird-reviewers

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/208e26dc06a0
Clean up test_folderCompact.js. r=mkmelin
https://hg.mozilla.org/comm-central/rev/fd3398e939c3
Stop compacting folders when shutdown begins. r=BenC

Comment 5

[Geoff Lankow (:darktrojan)]

Comment on attachment 9431441 [details]
Bug 1924918 - Clean up test_folderCompact.js. r=#thunderbird-reviewers

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined: just a test
Testing completed (on c-c, etc.): in 133 beta 1
Risk to taking this patch (and alternatives if risky): just a test

Comment 6

[Geoff Lankow (:darktrojan)]

Comment on attachment 9431442 [details]
Bug 1924918 - Stop compacting folders when shutdown begins. r=BenC!,#thunderbird-reviewers

[Approval Request Comment]
Regression caused by (bug #): always happened (AFAIK)
User impact if declined: compact keeps running during shutdown, probably causes shutdown hangs and weirdness
Testing completed (on c-c, etc.): on comm-central for nearly 2 weeks, in 133 beta 1
Risk to taking this patch (and alternatives if risky): more than I'd like for the amount of time on beta (i.e. no time), but it shouldn't cause any problems and I'm not aware of any problems on central

Comment 7

[Geoff Lankow (:darktrojan)]

(In reply to Wayne Mery (:wsmwk) from bug 1923526 comment 23)

How will expunge on shutdown factor in? Is it likely to trigger compact?

And there it is, the complicating factor I haven't thought about.

Expunge on shutdown triggers a compaction of the inbox (and presumably we want this). The compaction does happen earlier in shutdown than the patch here would prevent, but I'm unclear if shutdown waits for it to finish before moving to the next phase. I did a simple test and it appears that compaction was completed.

Either way, if it lasts for over a minute the shutdown terminator will kill it and we'll continue to get the crash reports this bug was trying to prevent.

Comment 8

[Geoff Lankow (:darktrojan)]

Comment on attachment 9431441 [details]
Bug 1924918 - Clean up test_folderCompact.js. r=#thunderbird-reviewers

[Approval Request Comment]
See comment 5.

Comment 9

[Geoff Lankow (:darktrojan)]

Comment on attachment 9431442 [details]
Bug 1924918 - Stop compacting folders when shutdown begins. r=BenC!,#thunderbird-reviewers

[Approval Request Comment]
See comment 6.

Comment 10

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

Confirming this issue as verified fixed with 133.0b3(20241107210012) using Windows 11, macOS 14, and Ubuntu 22.

Worth mentioning so the scenario does not get lost as per discussed with Kai.

In order to properly verify that compacting folders are aborted at shutdown and no further folder corruption is triggered afterwards:

1. Quit TB
2. Find your profile folder, subdirectory Mail/Local Folders
3. Copy the two files into that directory.
4. Start TB (TB should show a new mailbox named "copied" in your local folders.)
5. Go to that folder, delete one or two messages.
6. Scroll to the middle of the list of messages, and look for a message that is several megabytes big
7. Then prepare to the following two steps very quickly after each other.
8. Right-click on the folder "copied", and select "compact".
9. Then immediately hit the following keyboard shortcut: CTRL+Q (This should quit Thunderbird immediately, while it is still running.)
10. Start Thunderbird again.(Keep working with the folder "copied".)
11. As soon after having started TB, right click again on the
    "copied" folder, select compact. You might get a status information on
    the status bar that it's running. Wait until that's complete.
12. After it's done, try to look at messages in that folder. Click variying
    messages. I see that some messages are shown empty. If you use "view
    source" (CTRL+U) for those messages, you might see just some random
    strings, not the expected email with headers etc.

If you click on a view of the larger messages in the middle of the
mailbox, you might see some messages that show random strings, or, you
might see messages that display an image, but that don't show any
"from", "subject" etc. information.

Comment 11

[Corey Bryant]

Comment on attachment 9431441 [details]
Bug 1924918 - Clean up test_folderCompact.js. r=#thunderbird-reviewers

[Triage Comment]
Approved for esr128

Comment 12

[Corey Bryant]

Comment on attachment 9431442 [details]
Bug 1924918 - Stop compacting folders when shutdown begins. r=BenC!,#thunderbird-reviewers

[Triage Comment]
Approved for esr128

Comment 13

[Corey Bryant]

Comment on attachment 9431441 [details]
Bug 1924918 - Clean up test_folderCompact.js. r=#thunderbird-reviewers

[Triage Comment]
Approved for release

Comment 14

[Corey Bryant]

Comment on attachment 9431442 [details]
Bug 1924918 - Stop compacting folders when shutdown begins. r=BenC!,#thunderbird-reviewers

[Triage Comment]
Approved for release

Comment 15

[Daniel Darnell [:dandarnell]]

Thunderbird 128.4.3esr:
https://hg.mozilla.org/releases/comm-esr128/rev/2424658332b2
https://hg.mozilla.org/releases/comm-esr128/rev/f29b03347ccb

Comment 16

[Daniel Darnell [:dandarnell]]

Thunderbird 132.0.1:
https://hg.mozilla.org/releases/comm-release/rev/9476a76e27e4
https://hg.mozilla.org/releases/comm-release/rev/e6ced62e2a39

Comment 17

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

Confirming this issue as verified fixed using the same steps as used in comment 10, with 128.4.3(20241108210256) on macOS 14, Windows 11 and Ubuntu 24.

Comment 18

[Wayne Mery (:wsmwk)]

If crash comments are to be believed, the most common signature that correlates to compact during shutdown is Bug 1913805 - Crash in [@ shutdownhang | mozilla::SpinEventLoopUntil<T> | nsThread::WaitForAllAsynchronousShutdowns | nsThreadManager::ShutdownNonMainThreads]

But I don't see that the crash rate for it has noticeably decreased starting with 128.4.3. It maybe that users who crash with this signature for other reasons greatly outnumber those who crash with compact.

Comment 19

[Geoff Lankow (:darktrojan)]

There are many shutdown hangs where the crash shows as happening somewhere in Mork, and with the compactor involved. Those should have stopped or at least gone down a lot.

Comment 20

[Geoff Lankow (:darktrojan)]

Such as this one, clearly this bug fix hasn't stopped them all. :-)