Skip to content

fix(suspendapp): avoid deadlock and align JVM behaviour on system.exit#3930

Open
tKe wants to merge 1 commit into
mainfrom
tKe/suspend-app-system-exit
Open

fix(suspendapp): avoid deadlock and align JVM behaviour on system.exit#3930
tKe wants to merge 1 commit into
mainfrom
tKe/suspend-app-system-exit

Conversation

@tKe

@tKe tKe commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

fixes #3846

This fix essentially sidesteps the deadlock by not attempting to cancel/graceful-shutdown when the hook is invoked for any reason other than SIGINT/SIGTERM.

The rationale is that System.exit equivalents on other platforms doesn't trigger cancellation of the SuspendApp, and the other hook invocation cause (last non-daemon thread terminated) is not expected through the normal usage of SuspendApp (that is, as the entry point on the main thread for a service/application).

Instead, this fixes the Signal handling, captures the signal, and limits triggering cancellation to only when signalled.

@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Kover Report

File Coverage [0.00%]
arrow-libs/suspendapp/suspendapp/src/jvmMain/kotlin/arrow/continuations/Enviroment.jvm.kt 0.00%
Total Project Coverage 47.78%

@tKe tKe requested review from nomisRev and serras June 27, 2026 09:11
@serras

serras commented Jun 27, 2026

Copy link
Copy Markdown
Member

@tKe could you rebase after the merging of #3917? In fact, is there any relation between both PRs that need to be taken into account?


override fun onSigTerm(block: suspend (code: Int) -> Unit): Unit =
addSignalHandler("SIGTERM") { block(15) }
addSignalHandler("TERM", block)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the change from SIGTERM to TERM needed?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sun.misc.Signal takes the signal name sans SIG prefix.
The example from the JavaDoc is:

Signal objects are created based on their names. For example:

new Signal("INT");

Previously these were throwing IllegalArgumentException on construction that were being caught and ignored.

@tKe

tKe commented Jun 27, 2026

Copy link
Copy Markdown
Collaborator Author

@serras rebasing this, which includes job name fixes for the js tests, highlighted that the exitApp change has introduced a regression in then wasm/js signal handling as it now reports an exitCode of 255 not 128+sig. I'll include a fix in this rebase.

@tKe tKe force-pushed the tKe/suspend-app-system-exit branch from 2272659 to 3b13f57 Compare June 27, 2026 10:48
@tKe tKe requested a review from serras June 27, 2026 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] SuspendApp freezes app on System.exit

2 participants