1837907 - Not handle system shutdown (SIGTERM)

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect, P2)

Description

[Alexander]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:109.0) Gecko/20100101 Firefox/116.0

Steps to reproduce:

I shutdown my system by /sbin/poweroff (/sbin/reboot) or by dbus command "dbus-send --system --print-reply --dest=org.freedesktop.login1 /org/freedesktop/login1 "org.freedesktop.login1.Manager.PowerOff" boolean:true", but after next run system FireFox starting as after crash, not as after correct shutdown. If I close FF by "Close" button on program caption or by command "File->Exit" in menu - all Ok.

Actual results:

Not handle system shutdown.

Expected results:

Handle system shutdown.

Comment 1

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

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

Comment 2

[Alexander]

Also bug occur when "killall firefox" and not occur when close by "wmctrl -c 'Firefox'".

Comment 3

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

The severity field is not set for this bug.
:emilio, could you have a look please?

For more information, please visit BugBot documentation.

Comment 4

[Emilio Cobos Álvarez [:emilio]]

Well you're killing Firefox, maybe we could do something more graceful for SIGTERM or use some sort of shutdown blocker and wait for clean shutdown? But not sure how much of a rabbit-hole that would be.

Comment 5

[Timea Zsoldos [:tzsoldos], Desktop QA]

Based on comment 4 change status to "New".

Comment 6

[Martin Stránský [:stransky] (ni? me)]

Yes, it's possible to do that. We'd need to add another handler here:
https://searchfox.org/mozilla-central/rev/d81e60336d9f498ad3985491dc17c2b77969ade4/widget/gtk/nsAppShell.cpp#216

We may copy what Windows does:
https://searchfox.org/mozilla-central/rev/d81e60336d9f498ad3985491dc17c2b77969ade4/widget/windows/nsWindow.cpp#4936

Comment 7

[Martin Stránský [:stransky] (ni? me)]

Alexander, do you mind to try to create a patch for it? a mini-howto can be found here:
https://mastransky.wordpress.com/2023/07/04/no-one-fights-alone-a-guide-to-your-first-firefox-patch-on-linux/
Thanks.

Comment 8

[Alexander]

I do not have ready solution for make patch.
This is requiring research.
https://stackoverflow.com/questions/22009705/how-to-detect-linux-shutdown-reboot
Only signal handling is not enough to solve this problem.
I have handled signals in my program, but it work only for Ctrl+C situation, not for shutdown system.
https://github.com/galaxysite/gorg64_spkplay/blob/main/gorg64_spkplay.pas
May be this problem already resolved in Falkon ?
https://github.com/KDE/falkon
Outwardly it looks like this.
Also I do not have ready solution for dbus.

Comment 9

[Martin Stránský [:stransky] (ni? me)]

You can handle the DBus message 'org.freedesktop.login1.Manager.PowerOff'. Not sure if it's even possible to catch /sbin/poweroff and if that sends any signal to application.

IIUC Windows try to catch SIGTERM here:
https://searchfox.org/mozilla-central/rev/d81e60336d9f498ad3985491dc17c2b77969ade4/widget/windows/nsWindow.cpp#4936
(or something which matches SIGTERM on Windows) so can the code be the same on Linux and may it be executed in SIGTERM handler?

Comment 10

[Martin Stránský [:stransky] (ni? me)]

Looks like poweroff directly uses reboot kernel call (https://man7.org/linux/man-pages/man2/reboot.2.html) and it's kind of emergency shutdown. I doubt any desktop application/environment can handle this kind of shutdown seamlessly.

Comment 11

[Alexander]

Not-not, poweroff is not emergency. This is the standard, non-emergency way for shutdown of a System. poweroff/reboot - command for change runlevel of System: https://www.geeksforgeeks.org/run-levels-linux/ All other ways more modern: poweroff/reboot historical, standard way of shutdown.

Comment 12

[Alexander]

SIGTERM on Windows
Only process messages. Signals are not exactly messages.
In the old days, I solved this problem like this:
...

procedure OnQEnd(Var Msg : TMessage); message WM_QUERYENDSESSION;
procedure OnEnd(Var Msg : TMessage); message WM_ENDSESSION;
procedure OnSysCommand(Var Msg: TWMSysCommand); message WM_SYSCOMMAND;

...

procedure MyMain.OnSysCommand(Var Msg: TWMSysCommand);
begin
if msg.CmdType = SC_CLOSE then begin
if MessageBox(hwnd, PCHar(app_quit), PCHar(app_name), bit6 or bit3) <> ID_YES then begin msg.Result := 0; exit; end;
end;
msg.Result := DefWindowProc(hwnd, msg.msg, TMessage(msg).WParam, TMessage(msg).LParam);
end;

procedure MyMain.OnQEnd(Var Msg : TMessage);
begin
app.terminated := true;
msg.Result := Longint(true);
end;
procedure MyMain.OnEnd(Var Msg : TMessage);
begin
while app.terminated do sleep(0); // during the shutdown of the program and saving its files in 2-st thread
msg.Result := 0;
end;

Comment 13

[Alexander]

May be solution is here ? : https://chromium.googlesource.com/chromium/src/+/78.0.3904.108/chrome/app/shutdown_signal_handlers_posix.cc

Comment 14

[Alexander]

I quest about this systemd creators and get answer:

https://github.com/systemd/systemd/issues/29146
We emit PrepareForShutdown dbus signal before/after initiating the shutdown process: https://www.freedesktop.org/software/systemd/man/org.freedesktop.login1.html#Signals

Comment 15

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1837907 [Linux] Don't catch SIGTERM in nsProfileLock r?jld

We want to handle SIGTERM gracefully and terminate Firefox by standard way.

Comment 16

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1837907 [Linux] Catch SIGTERM in nsAppShell and terminate Firefox by standard way r?jld

SIGTERM is supposed to be a standard way how to terminate Linux application. So don't fire emergency exit but run usual exit sequence.

Depends on D191077

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a78b3fba98aa
[Linux] Don't catch SIGTERM in nsProfileLock r=jld
https://hg.mozilla.org/integration/autoland/rev/ff3a75dbd146
[Linux] Catch SIGTERM in nsAppShell and terminate Firefox by standard way r=jld

Comment 18

[Serban Stanca [:SerbanS]]

Backed out for causing xpcshell failures in marSuccessPartialWhileBackgroundTaskRunning.js.

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | toolkit/mozapps/update/tests/unit_base_updater/marSuccessPartialWhileBackgroundTaskRunning.js | xpcshell return code: 0

L.E. There are also these Mn failures.

• Failure Log

Comment 19

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1837907 [Linux] Install SIGTERM handler from ProcessNextNativeEvent() r?emilio

Install SIGTERM handler only if we use event loop. That keeps default SIGTERM handler for applications without event loop like xpcshell.

Depends on D191079

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7a2134520f7c
[Linux] Don't catch SIGTERM in nsProfileLock r=jld
https://hg.mozilla.org/integration/autoland/rev/a883cbc99803
[Linux] Catch SIGTERM in nsAppShell and terminate Firefox by standard way r=jld
https://hg.mozilla.org/integration/autoland/rev/28174e34160b
[Linux] Install SIGTERM handler from ProcessNextNativeEvent() r=emilio

Comment 21

[Sandor Molnar[:smolnar]]

Backed out for causing xpc failures on toolkit/mozapps/update/tests/unit_base_updater/marSuccessPartialWhileBackgroundTaskRunning.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/e7f56b0ae07ad09c48bb7db75fcbbb2e320fe483

Push with failures

Failure log -> TEST-UNEXPECTED-FAIL | toolkit/mozapps/update/tests/unit_base_updater/marSuccessPartialWhileBackgroundTaskRunning.js

Comment 22

[Martin Stránský [:stransky] (ni? me)]

Surprisingly event loop is also called on xpcshell but when it's terminated. So we need to use something different here.

Comment 23

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1837907 [Linux] Enable AppShell SIGTERM signal handler for Firefox r?emilio

Depends on D191079

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e1d41d593702
[Linux] Don't catch SIGTERM in nsProfileLock r=jld
https://hg.mozilla.org/integration/autoland/rev/8502ba0bf6a6
[Linux] Catch SIGTERM in nsAppShell and terminate Firefox by standard way r=jld
https://hg.mozilla.org/integration/autoland/rev/89b150d0c0ab
[Linux] Disable AppShell SIGTERM signal handler for xpcshell r=emilio

Comment 25

[Cosmin Sabou [:CosminS]]

Backed for causing marionette crashes.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&searchStr=mn&revision=89b150d0c0ab85684103d01707ba0010edbeb7ab&selectedTaskRun=VaRLrf6iTfyjJVQteISVKA.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=437036027&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/9eb2cb31618c369f011b1799ed328122558aa9a4

LE: also these Gtest failures: https://treeherder.mozilla.org/logviewer?job_id=437056477&repo=autoland

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f7551ea88778
[Linux] Catch SIGTERM in nsAppShell and terminate Firefox by standard way r=jld
https://hg.mozilla.org/integration/autoland/rev/1035a42c6a86
[Linux] Enable AppShell SIGTERM signal handler for Firefox r=emilio

Comment 27

[Cristian Tuns]

Backed out for causing xpcshell failures in marSuccessPartialWhileBackgroundTaskRunning.js

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | toolkit/mozapps/update/tests/unit_base_updater/marSuccessPartialWhileBackgroundTaskRunning.js | run_test - [run_test : 115] -15 == -9

And also marionette failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&searchStr=linux%2C18.04%2Cx64%2Cwebrender%2Cdebug%2Ctest-linux1804-64-qr%2Fdebug-marionette-swr%2Cmn-swr&revision=1035a42c6a86393419461fd90d38c7fa183b92f3&selectedTaskRun=YB-XIBOKS9-5VDMu3MCKzA.0

Comment 28

[Martin Stránský [:stransky] (ni? me)]

Latest try: https://treeherder.mozilla.org/jobs?repo=try&revision=617778fc5fabf94c51499ae837414f1f3bb02f34

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7fe397f45f7b
[Linux] Catch SIGTERM in nsAppShell and terminate Firefox by standard way r=jld
https://hg.mozilla.org/integration/autoland/rev/f5150abdafbd
[Linux] Enable AppShell SIGTERM signal handler for Firefox r=emilio

Comment 30

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/7fe397f45f7b
https://hg.mozilla.org/mozilla-central/rev/f5150abdafbd