1578273 - Firefox process staying alive after exit

Status: VERIFIED FIXED

(Toolkit :: Data Sanitization, defect, P2)

Description

[[e-mail]]

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

Steps to reproduce:

Hi,
On closing Firefox either using the 'X' or the 'File' and 'Exit', the Firefox processes stay alive.
I have 3 pc's runing Firefox and the issue is on all 3.
This is not unique to v69, it has been happening for quite some time (probably around a year or more!).
The only way to stop the program is to close the processes in Task Manager.
About 30 seconds after exiting the browser if the processes are not closed a Firefox Crashed Message Box appears!

Actual results:

On closing Firefox either using the 'X' or the 'File' and 'Exit', the Firefox processes stay alive.
This is not unique to v69, it has been happening for quite some time (probably around a year or more!).
The only way to stop the program is to close the processes in Task Manager.
About 30 seconds after exiting the browser if the processes are not closed a Firefox Crashed Message Box appears!

Expected results:

The program should exit and the processes stopped.

Comment 1

[[:philipp]]

hi, please submit one of those crash reports from the box that opens up 60 seconds or so after you close the browser. after a restart retrieve the crash id by entering about:crashes into the firefox address bar and pasting it here into the bug.

Comment 2

[[e-mail]]

Attachment: bp-42a2bfd4-00b3-47a3-8622-2bcd20190902.txt

Last crash report.

I hope that is what you want.

There are these going back 6 months.

Comment 3

[[:philipp]]

thank you, so it's basically the same issue that was tried to be diagnosed/debugged in bug 1524200.

{
    "phase":"Places Clients shutdown",
    "conditions":[
        {
            "name":"sanitize.js: Sanitize on shutdown",
            "state":{
                "progress":{
                    "isShutdown":true,
                    "advancement":"session-permission",
                    "step":"promises:1",
                    "sanitizePrincipal":"started"
                }
            },
            "filename":"resource:///modules/Sanitizer.jsm",
            "lineNumber":169,
            "stack":[...]
        }
    ]
}

you've set firefox to clear certain data on exit which is taking too long until in the end an artificial crash is triggered so that the process is really shutting down.

baku, is there any more debugging or information the affected user could provide in order for us to better understand and fix the problem?

Comment 4

[Andrea Marchesini [:baku]]

It seems that Services.clearData.deleteDataFromPrincipal is taking too much time. Wondering if it fails, somehow and it doesn't call 'resolve'.
Is it possible to have a dump of the browser console when it crashes? Maybe you want to enable the console to stdout.

Comment 5

[[e-mail]]

I was not aware that I am deleting anything on exit, except I have an add on 'Expire History By Days' enabled. However with that disabled the crash still occurs.
Could you please tell me how to get the dump of the browser console when it crashes please, and enable console to stdout?

Comment 6

[Andrea Marchesini [:baku]]

Thanks for your help. I would do these steps:

1. take a copy of your profile directory.
2. run firefox by command line in this way: firefox -no-remote -profile <the copy of your profile>. For instance, if you have copied your profile in /tmp/profile, run: firefox -no-remote -profile /tmp/profile
3. in about:config set browser.sanitizer.loglevel pref to "All". You should have "Warn".
4. in about:config set devtools.console.stdout.chrome to true
5. do the same for devtols.console.stdout.content (to true)

Copy everything that happens after the first line containing "Sanitizer.jsm". For instance, in my tests the first line is:
console.log: ** Sanitizer.jsm: "Sanitizing on shutdown"

Copy all, and send it to me: [e-mail]. Email is better because we don't want you to share on bugzilla your visited websites.
Thank you! If you need any additional information, write me an email.

Comment 7

[[e-mail]]

Hello,

Hopefully you will have received my email.

Comment 8

[[e-mail]]

Hello,

I have not heard from you, so I decided to investigate myself.

I created a new profile, and the not closing process was solved.

However, there were so many websites that needed java/javascript enabled, that it was almost impossible to surf the net!
It seems that the new profile prevented java sites from working properly.

So, I have returned to my original profile for now.

Hopefully you can assist with either the java problem in the new profile, and/or the original process not closing problem in my original profile.

Thank you,

Comment 9

[Johann Hofmann [:johannh]]

I'm not sure I can help with that, feel free to file a new bug with the issues you're seeing and we (presumably another team) will triage it. Otherwise I'm sure Andrea has received your email and will take a look whenever he has time for it.

Comment 10

[[e-mail]]

Update:

This bug is not yet solved.

However, I have found the problem with the new profile javascript problem.

It was the NoScript add-on that was blocking the websites. It is now OK.

So that is now OK, but this original bug is still outstanding.

Comment 11

[Garcia Jones]

This pauses the process, letting you continue it later. It won't work in most cases https://www.dqfansurvey.us/

Comment 12

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

The bug is linked to a topcrash signature, which matches the following criteria:

• Top 20 desktop browser crashes on release
• Top 5 desktop browser crashes on Mac on beta

:hpeuckmann, could you consider increasing the severity of this top-crash bug?

For more information, please visit auto_nag documentation.

Comment 13

[Hannah Peuckmann]

Looking at a few of the crash reports, the last sanitization progress step is 'principals-quota-manager', so Services.qms.listOrigins() could be causing this.

I noticed that we can reduce our calls to getAllPrincipals() in the sanitizer, we should try to only call this if sanitizeOnShutdown, clearOnShutdown.cookies and clearOnShutdown.offlineApps are true or the user has session permissions exceptions.

• If sanitizeOnShutdown is true, we can check if offlineApps or cookies are to be cleaned, otherwise we can skip the call to getAllPrincipals() because those are the only categories we clear per principal.

• If sanitizeOnShutdown is true I think we can also skip this 'session-permissions' block that is calling getAllPrincipals() again, from my understanding the session permission origins should already be cleared if sanitizeOnshutdown is true.

Paul, any thoughts on this?

Comment 14

[Paul Zühlcke [:pbz]]

We discussed this in the meeting. Hannah wants to look into optimizing this code to see if we can get the crash rates down. Mid to long term we should also address this issue by showing a progress dialog for clearing on shutdown and then not timing out because the shutdown takes longer.
However, it's also worth noting that the clients affected do not all have sanitize-on-shutdown enabled. In this case optimizing the principal collection or skipping it for the common case could help. Thanks Hannah!

Comment 15

[Hannah Peuckmann]

Attachment: Bug 1578273 - Reduce calls to the PrincipalsCollector. r=pbz!

Comment 16

[Marco Bonardo [:mak]]

Note we can't distinguish if something is taking too long, or if just the promise is not resolved. When the whole stack is made up of async functions, it's more likely stuff taking too long, but as soon as there's some callback in the middle, it's very easy to not invoke a callback if some unexpected exception happen or, even worst, when the callback is only invoked in case of success.
For example we had a lot of cases blocking in formData, so I switched it from callbacks to async functions, and the number of cases (afaict) reduced. Rejecting is better than not resolving for async shutdown.

So apart from optimizing, I suggest to also check in case of exceptions or errors that your callback is being invoked, so the promise is either resolved or rejected in any case.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/733f6ba8c1c6
Reduce calls to the PrincipalsCollector. r=pbz

Comment 18

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/733f6ba8c1c6

Comment 19

[Marian-Vasile Laza]

Backed out 2 changesets (Bug 1578273, Bug 1806620) as req for causing Bug 1816279.
Backout link

Comment 20

[Hannah Peuckmann]

I will fix it and land it again. We are missing an automated test for cookie clearing in a time range I will add one in this patch to prevent regressions like Bug 1816279 from happening again in the future.

Comment 21

[Marian-Vasile Laza]

Backout merged to central

Comment 22

[Donal Meehan [:dmeehan]]

Backed out 2 changesets (Bug 1578273, Bug 1806620) as req for causing Bug 1816279.
Beta Backout link

Comment 23

[Hannah Peuckmann]

Attachment: Bug 1578273 - Add test cases for cookie clearing in time range. r=pbz!

Depends on D162721

Comment 24

[Pascal Chevrel:pascalc]

Backed out 2 changesets (bug 1578273, bug 1806620) for causing Bug 1816279 a=pascalc
https://hg.mozilla.org/releases/mozilla-release/rev/7eab05d73968

Comment 25

[Phabricator Automation]

Comment on attachment 9318789 [details]
Bug 1578273 - Add test cases for cookie clearing in time range. r=pbz!

Revision D170377 was moved to bug 1819529. Setting attachment 9318789 [details] to obsolete.

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6b9b6b406d7d
Reduce calls to the PrincipalsCollector. r=pbz

Comment 27

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/6b9b6b406d7d

Comment 28

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

The patch landed in nightly and beta is affected.
:hpeuckmann, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.Also, don't forget to request an uplift for the patches in the regressions caused by this fix.
• If no, please set status-firefox112 to wontfix.

For more information, please visit auto_nag documentation.

Comment 29

[Anca Soncutean, Desktop QA]

I am not able to reproduce this issue neither with the version mention in the initial report (Fx69), nor with any more recent ones on Windows 10. Paul, can you confirm if time permits, that this issue is fixed on the latest beta build on your side? Thank you!

Comment 30

[[e-mail]]

Yes, all seems go now.