1806831 - Firefox shutdown never finishes if the policy is set to clear the cache on shutdown

Status: VERIFIED FIXED

(Core :: Networking: Cache, defect, P1)

Description

[Daniel]

Attachment: 21-12-2022_14-27-10.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/108.0.0.0 Safari/537.36 Edg/108.0.1462.54

Steps to reproduce:

tries to close firefox

Actual results:

continues to run under background processes

Expected results:

he should have ended himself completely

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Firefox::Theme' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Daniel]

Attachment: 22-12-2022_05-55-47.png

Found a mistake. Cache must not be set to 1!

Comment 3

[Johan Lorenzo [:jlorenzo]]

Hey :florian! I don't know if you're the right person to reach out to. I see this impacts processes and energy saving. Who would be the right contact for this bug?

Comment 4

[Florian Quèze [:florian]]

Hello Daniel. Thanks for the bug report. Could you please describe the problem? Is it that the Firefox processes remain listed in the task manager for longer than expected after you closed the browser, but ended eventually? Or that they never ended? Or something else?

I don't understand what the screenshot attached in comment 2 shows. Is it related?

Comment 5

[Florian Quèze [:florian]]

Daniel replied over email, explaining that the Firefox processes don't end and need to be killed from the task manager if the SanitizeOnShutdown 'Cache' policy is set to 1 in the Windows registry, and that this problem only affects the 109 beta.

Comment 6

[Mike Kaply [:mkaply]]

So the policy is only flipping the preference.

https://searchfox.org/mozilla-central/source/browser/components/enterprisepolicies/Policies.sys.mjs#1855

Comment 7

[Florian Quèze [:florian]]

Attachment: Video from the reporter

The reporter sent me a video of how he reproduces the bug.

Comment 8

[Simona Badau, Desktop QA]

Setting this to the Enterprise Policies component - if this is not the correct one, please move it to a more appropriate one.

Comment 9

[Daniel Bodea [:danibodea]]

I can confirm that the issue reproduces in an installed version of Beta v109.0b6 and an unzipped build of Nightly v110.0a1, but not with mozregression builds. This means the investigation MUST be done manually.

Furthermore, it appears NOT to reproduce on Release v108.0.1 or in Beta v108.0b9.

Checking Nightly builds:
Nightly v109 2022-11-14-21-44-03, the first 109 = affected
Nightly v108 2022-11-01-09-39-31 = affected
Nightly v107 2022-10-15-21-26-05 = affected
Nightly v106 2022-09-15-09-44-51 = affected
Nightly v106 2022-09-01-09-54-52 = affected
Nightly v105 2022-08-20-09-46-21 = affected
Nightly v105 2022-08-17-19-05-33 = affected
Nightly v105 2022-08-16-09-55-03 = affected
Nightly v105 2022-08-15-21-47-39 = affected (FIRST AFFECTED BUILD)
Nightly v105 2022-08-15-09-42-32 = unnafected (LAST GOOD BUILD)
Nightly v105 2022-08-01-03-40-14 = unnafected
Nightly v104 2022-07-15-09-55-45 = unnafected
Nightly v102 2022-05-05-09-42-30 = unnafected
Nightly v97 2022-01-01-09-53-22 = unnafected

Comment 10

[Mike Kaply [:mkaply]]

Any chance you could verify using the preferences and not policy?

It definitely shouldn't be policy specific.

Comment 11

[Florian Quèze [:florian]]

(In reply to Bodea Daniel [:danibodea] from comment #9)

I can confirm that the issue reproduces in an installed version of Beta v109.0b6 and an unzipped build of Nightly v110.0a1, but not with mozregression builds. This means the investigation MUST be done manually.

Thanks a lot for the investigation and verifying it is indeed a regression.

An unzipped nightly and a "mozregression build" should be the same, but they are started in a different way. Do you think it could be possible to further narrow down the regression range using mozregression, but downloading manually the builds that mozregression wants to test? It would be really helpful to know which autoland changeset caused the regression, so that we can ask the patch authors to have a look.

Checking Nightly builds:

Nightly v105 2022-08-15-21-47-39 = affected (FIRST AFFECTED BUILD)
Nightly v105 2022-08-15-09-42-32 = unnafected (LAST GOOD BUILD)

The changes between these builds are https://hg.mozilla.org/mozilla-central/pushloghtml?changeset=1c3c5be93f5897f3265ec5eeef08c09cb09fb774

Comment 12

[Daniel Bodea [:danibodea]]

I have tried again; a simply extracted nightly build reproduces it, but the same build opened with mozregression does not reproduce it. A further investigation could not be provided.

P.S. The method that I found to reproduce the issue is by creating the exact "Cache" key with value 1 in Registry Editor, exactly where the video in comment 7 shows it.

Comment 13

[Florian Quèze [:florian]]

Running the builds from mozregression directly doesn't reproduce the bug (is mozregression somehow killing leftover processes?), but if after closing the build started by mozregression, before answering "good" or "bad", I look for the path where the build was temporarily unpacked, and start firefox.exe myself from that folder, I can reproduce.

Then mozregression says
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=643c507b9fedf7fde3ad5afd9fd748373bbecbf5&tochange=cad6e6b08c7bb7576c31ba80dcef8a85c14ef9d8 -> bug 1705676 which landed in 105.

And I guess bug 1786256 for beta/release which landed in 109, this explains why the 108 release is not affected.

Comment 14

[Valentin Gosu [:valentin] (he/him)]

Thank you for the report.
From what I can tell, this doesn't reproduce if I set the preferences manually, only when the policies are active. I haven't managed to confirm, but my suspicion is that the background task profile gets the same preferences set by the policy and ends up spawning yet another background task when it exits.

Comment 15

[Valentin Gosu [:valentin] (he/him)]

I've confirmed that this is the cause and have a fix for this specific issue.
In the future we'll probably want to add a generic check that background tasks don't endlessly spawn other background tasks.

Comment 16

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1806831 - removeDirectory background tasks keep spawning if the policy is set to clear the cache on shutdown r=#necko

Comment 17

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 9310472 [details]
Bug 1806831 - removeDirectory background tasks keep spawning if the policy is set to clear the cache on shutdown r=#necko

Beta/Release Uplift Approval Request

• User impact if declined: Background tasks will keep spawning consuming machine resources if cache sanitization is enabled via policy.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See comment 7.
  Create keys in regedit COMPUTER\HKEY_LOCAL_MACHINE\SOFTWARE\Policies\Mozilla\Firefox\SanitizeOnShutdown.
  Create REG_DWORD named Cache with value 1.
  Start Firefox. Check about:policies to ensure policy was set. Stop Firefox. Check task manager that all Firefox processes have shut down.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Code will avoid spawning another background task at shutdown if already running in a background task.
• String changes made/needed:
• Is Android affected?: No

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/837b1f1e4939
removeDirectory background tasks keep spawning if the policy is set to clear the cache on shutdown r=necko-reviewers,kershaw

Comment 19

[Florian Quèze [:florian]]

(In reply to Valentin Gosu [:valentin] (he/him) from comment #15)

Thanks for fixing this so quickly after getting the needinfo!

In the future we'll probably want to add a generic check that background tasks don't endlessly spawn other background tasks.

Are you planning to open a follow-up bug for this?

Comment 20

[Noemi Erli[:noemi_erli]]

Backed out changeset 837b1f1e4939 (Bug 1806831) for causing background related failures CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&selectedTaskRun=IxkFzxGORNSzeuQSKX2FEQ.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=837b1f1e4939ef2b5240519606c195f33f97d32a

Log: https://treeherder.mozilla.org/logviewer?job_id=401203559&repo=autoland&lineNumber=2419
https://treeherder.mozilla.org/logviewer?job_id=401203680&repo=autoland&lineNumber=9715
https://treeherder.mozilla.org/logviewer?job_id=401204331&repo=autoland&lineNumber=99188

Backout: https://hg.mozilla.org/integration/autoland/rev/ecefedd04c617dd6825634e101bf382e1c44b7c0

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4e42b5947e4e
removeDirectory background tasks keep spawning if the policy is set to clear the cache on shutdown r=necko-reviewers,kershaw

Comment 22

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/4e42b5947e4e

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9310472 [details]
Bug 1806831 - removeDirectory background tasks keep spawning if the policy is set to clear the cache on shutdown r=#necko

Approved for 109.0b9. Definitely want to get QA verification also.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1fc21e64b682

Comment 25

[Daniel Bodea [:danibodea]]

The fix was verified in Nightly v110.0a1 and Beta v109.0b9 fixed treeherder build (20230104213457).