grishka / Smithereen

Federated, ActivityPub-compatible social network server with friends, walls, and groups.
The Unlicense
385 stars 30 forks source link

fix: Don't call halt in shutdown hook #59

Closed kgilpin closed 7 months ago

kgilpin commented 1 year ago

Calling halt prevents any other shutdown hooks from running, which can interfere with operation of other code such as Java agents.

grishka commented 1 year ago

Hmm. But how would it set the exit code to 0 then? I added this because iirc it was something else by default and systemd wasn't happy about it.

kgilpin commented 1 year ago

The shutdown hook is being called because something already initiated the program exit. So I don’t think it should be desirable or necessary to change it in the shutdown hook. I can look into it a bit more if you like.

On Mon, May 15, 2023 at 5:52 PM Gregory K @.***> wrote:

Hmm. But how would it set the exit code to 0 then? I added this because iirc it was something else by default and systemd wasn't happy about it.

— Reply to this email directly, view it on GitHub https://github.com/grishka/Smithereen/pull/59#issuecomment-1548644837, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAVC62QEUASN4GKWXTFAVTXGKQS5ANCNFSM6AAAAAAYCYNU7E . You are receiving this because you authored the thread.Message ID: @.***>

kgilpin commented 1 year ago

Whatever the process exit status was going to be, I don't think the shutdown hook should mess with it.

If the process is exiting for a legitimate non-zero reason, then the shutdown hook would not want to mask that by forcing the exit status to zero.

grishka commented 1 year ago

I no longer understand what your changes are supposed to achieve 🤔

grishka commented 1 year ago

My intention with the shutdown hook is to intercept SIGINT and shut down gracefully. I'm not sure there are other ways to do it.

kgilpin commented 1 year ago

Sorry, this branch got some commits that are intended for another branch.

The exit code of a Java program terminated by a SIGINT signal is normally 130. This is the convention in Unix-like systems. When a process is interrupted by a SIGINT signal, the system sets the exit status to 128 plus the signal number. In the case of SIGINT, the signal number is 2, so the exit code becomes 128 + 2 = 130.

If you’re sure you want to change it to 0, did you try System.exit(0)? This will allow other shutdown hooks to run normally - halt is aborting immediately and cutting off other shutdown work that’s in progress.

grishka commented 1 year ago

If you’re sure you want to change it to 0, did you try System.exit(0)?

It will cause a deadlock.