Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tKe wants to merge 2 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3b13f57
fix(suspendapp): avoid deadlock and align JVM behaviour on system.exit
tKe Jun 26, 2026
fc06804
Add error logs for signal handler errors and hook skipping cancellation
tKe Jul 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 22 additions & 9 deletions arrow-libs/suspendapp/suspendapp-test-app/src/commonMain/kotlin/app.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,24 @@ import arrow.continuations.SuspendApp
import arrow.continuations.SuspendAppScope
import arrow.continuations.exitApp
import arrow.fx.coroutines.resourceScope
import kotlin.time.Duration
import kotlin.time.Duration.Companion.milliseconds
import kotlin.time.Duration.Companion.seconds
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.awaitCancellation
import kotlinx.coroutines.awaitCancellation
import kotlinx.coroutines.async
import kotlinx.coroutines.delay
import kotlinx.coroutines.isActive
import kotlinx.coroutines.launch
import kotlin.time.Duration
import kotlin.time.Duration.Companion.seconds

fun interface Work {
suspend fun SuspendAppScope.work()
}

sealed interface Mode : Work
data class Delay(val duration: Duration = 3.seconds) : Mode, Work by Work({ delay(duration) })
data object Wait : Mode, Work by Work({
while (isActive) {
delay(1000)
}
})

data object Wait : Mode, Work by Work({ awaitCancellation() })
data object Fail : Mode, Work by Work({ error("BOOM!") })
data object ChildFail : Mode, Work by Work({ launch { error("boom.") } })
data object ExitApp : Mode, Work by Work({ exitApp(42) })
Expand All @@ -30,6 +28,17 @@ data object ChildLaunchExitApp : Mode, Work by Work({
launch { exitApp(24) }
awaitCancellation()
})
data object ExitProcess : Mode, Work by Work({
launch { exitProcess(42) }
awaitCancellation()
})

data object Timeout : Mode, Work by Work({
resourceScope {
onRelease { delay(6.seconds) }
awaitCancellation()
}
})

fun app(mode: String?) = app(
when (mode) {
Expand All @@ -40,13 +49,15 @@ fun app(mode: String?) = app(
"exitapp" -> ExitApp
"childexitapp" -> ChildExitApp
"childlaunchexitapp" -> ChildLaunchExitApp
"exit" -> ExitProcess
"timeout" -> Timeout
else -> Delay()
}
)

fun app(work: Work) {
println("pre-suspendapp")
SuspendApp(timeout = 10.seconds) {
SuspendApp(timeout = 5.seconds) {
println("Running $work")
resourceScope {
onRelease {
Expand All @@ -59,3 +70,5 @@ fun app(work: Work) {
}
println("post-suspendapp")
}

expect fun exitProcess(code: Int): Nothing
5 changes: 5 additions & 0 deletions arrow-libs/suspendapp/suspendapp-test-app/src/jsMain/kotlin/app.js.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@

actual fun exitProcess(code: Int): Nothing {
js("process.exit(code);")
error("non-exiting process.exit")
}
3 changes: 3 additions & 0 deletions arrow-libs/suspendapp/suspendapp-test-app/src/jvmMain/kotlin/app.jvm.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import kotlin.system.exitProcess

actual fun exitProcess(code: Int): Nothing = exitProcess(code)
3 changes: 3 additions & 0 deletions arrow-libs/suspendapp/suspendapp-test-app/src/nativeMain/kotlin/app.native.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import kotlin.system.exitProcess

actual fun exitProcess(code: Int): Nothing = exitProcess(code)
11 changes: 11 additions & 0 deletions arrow-libs/suspendapp/suspendapp-test-app/src/wasmJsMain/kotlin/app.wasmJs.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
@file:OptIn(ExperimentalWasmJsInterop::class)

actual fun exitProcess(code: Int): Nothing {
jsExit(code)
error("did not exit")
}

@Suppress("unused")
private fun jsExit(code: Int) {
js("process.exit(code);")
}
2 changes: 1 addition & 1 deletion arrow-libs/suspendapp/suspendapp-test-runner/src/test/kotlin/JsSpec.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ open class JsSpec : SuspendAppTest() {
.apply { environment()["TASK"] = mode }

companion object {
val config = JsTestConfig("jsNodeRun")
val config = JsTestConfig("jsNodeProductionRun")

@JvmStatic
fun enabled() = config.validConfig()
Expand Down
37 changes: 32 additions & 5 deletions arrow-libs/suspendapp/suspendapp-test-runner/src/test/kotlin/SuspendAppTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import io.kotest.inspectors.shouldForAtLeastOne
import io.kotest.inspectors.shouldForNone
import io.kotest.inspectors.shouldForOne
import io.kotest.matchers.collections.shouldBeIn
import io.kotest.matchers.shouldBe
import io.kotest.matchers.shouldNotBe
import io.kotest.matchers.string.shouldContain
import io.kotest.matchers.string.shouldEndWith
import kotlinx.coroutines.delay
import kotlinx.coroutines.test.runTest
Expand All @@ -19,15 +24,25 @@ abstract class SuspendAppTest : ProcessProvider {
val (process, output) = execute("fail")
process.exitValue() shouldBe 255
output.shouldForOne { it.line shouldBe "resource clean complete" }
.shouldForOne { it.line shouldEndWith "IllegalStateException: BOOM!" }
.shouldForAtLeastOne { it.line shouldEndWith "IllegalStateException: BOOM!" }
}

@Test
fun exit() = runTest {
val (process, output) = execute("exit")
process.exitValue() shouldBe 42
output
.filter { it.source == "stdout" }
.shouldForNone { it.line shouldBe "resource clean complete" }
.last().line shouldBe "Running ExitProcess"
}

@Test
fun childFailure() = runTest {
val (process, output) = execute("childfail")
process.exitValue() shouldBe 255
output.shouldForOne { it.line shouldBe "resource clean complete" }
.shouldForOne { it.line shouldEndWith "IllegalStateException: boom." }
.shouldForAtLeastOne { it.line shouldEndWith "IllegalStateException: boom." }
}

@Test
Expand Down Expand Up @@ -57,9 +72,21 @@ abstract class SuspendAppTest : ProcessProvider {
@Test
fun waitAndSignalSigint() = waitAndSignal(Signal.SIGINT)

private fun waitAndSignal(signal: Signal) = runTest {
val (process, output) = execute("wait") {
delay(1.seconds)
@Test
fun waitAndTimeout() = runTest {
val (process, output) = execute("timeout") {
delay(0.5.seconds)
sendSignal(Signal.SIGTERM)
}
// TODO: inconsistent exit codes across platforms, for now just check it's not a success

@nomisRev nomisRev Jul 3, 2026 •
edited
Loading

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.

Seems like an inconsistency introduced in this PR? See other comment https://github.com/arrow-kt/arrow/pull/3930/changes#r3519183429.

@tKe tKe Jul 3, 2026

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.

Currently, I believe the inconsistency is on JVM still exiting with the correct signalled exit code (128+sig) but the other platforms are exiting with exit code 255 due to the timeout exception.

JvmSpec:waitAndTimeout exitCode == 143
JsSpec:waitAndTimeout exitCode == 255
WasmJsSpec:waitAndTimeout exitCode == 255
NativeSpec:waitAndTimeout exitCode == 255

process.exitValue() shouldNotBe 0
output.shouldForAtLeastOne { it.line shouldContain "Timed out waiting for 5000 ms" }
.shouldForNone { it.line shouldContain "resource clean complete" }
}

private fun waitAndSignal(signal: Signal, mode: String = "wait") = runTest {
val (process, output) = execute(mode) {
delay(0.5.seconds)
sendSignal(signal)
}
process.exitValue() shouldBe signal.code + 128
Expand Down
2 changes: 1 addition & 1 deletion arrow-libs/suspendapp/suspendapp-test-runner/src/test/kotlin/WasmJsSpec.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ class WasmJsSpec : JsSpec() {
override val config: JsTestConfig get() = Companion.config

companion object {
val config = JsTestConfig("wasmJsNodeRun")
val config = JsTestConfig("wasmJsNodeProductionRun")

@JvmStatic
fun enabled() = config.validConfig()
Expand Down
74 changes: 59 additions & 15 deletions arrow-libs/suspendapp/suspendapp/src/jvmMain/kotlin/arrow/continuations/Enviroment.jvm.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package arrow.continuations

import arrow.AutoCloseScope
import java.util.concurrent.atomic.AtomicBoolean
import java.util.concurrent.atomic.AtomicInteger
import kotlin.coroutines.CoroutineContext
import kotlin.system.exitProcess
import kotlinx.coroutines.CoroutineScope
Expand All @@ -14,42 +15,85 @@ internal actual fun AutoCloseScope.process(): Process = JvmProcess
private object JvmProcess : Process {
override fun onShutdown(block: suspend () -> Unit): () -> Unit {
val isShutdown = AtomicBoolean(false)
val lastSignal = AtomicInteger(-1)
onSigInt(lastSignal::set)
onSigTerm(lastSignal::set)

fun shutdown() {
if (!isShutdown.getAndSet(true))
runBlocking {
// We don't call exit from ShutdownHook on JVM
try {
block()
} catch (e: Throwable) {
e.printStackTrace()
if (!isShutdown.getAndSet(true)) {
/*
`SuspendApp` is intended to be invoked from the main entrypoint (a non-daemon thread) and, in that normal usage,
will unregister this hook before main completes.

This hook can run when:
1) JVM receives SIGINT/SIGTERM
2) last non-daemon thread exits
3) `System.exit` is called

We run graceful shutdown only for (1), detected by a captured signal.

Regarding (2):
- In the expected usage, a non-daemon caller thread (usually main) remains alive while
`SuspendApp` is running.
- So if this hook is invoked because the last non-daemon thread exited, that likely reflects an
unexpected invocation pattern (for example, daemon-thread-driven usage).
- We choose not to provide graceful-shutdown guarantees for that edge case.

Regarding (3) and deadlock risk:
- `System.exit` may be called from code running in the `SuspendApp` coroutine scope.
- `System.exit` will never return to that caller.
- If this hook attempts graceful termination (`block()`), it can end up waiting for the
`SuspendApp` scope to complete (join/drain), while that scope is waiting on a call path that
includes the non-returning `System.exit`.
- Since that wait cannot resolve, the shutdown hook will deadlock.

Other platform behaviour:
- Other targets do not have a JVM-style shutdown hook with equivalent process-exit semantics,
and currently do not attempt graceful shutdown either, except when signalled.

So, graceful shutdown is gated solely on SIGINT/SIGTERM capture, both to align behaviour with
other platforms and to avoid the potential deadlock of an explicit `System.exit`.
*/
if (lastSignal.get() != -1) {
Comment thread
tKe marked this conversation as resolved.
runBlocking {
// We don't call exit from ShutdownHook on JVM
try {
block()
} catch (e: Throwable) {
e.printStackTrace()
}
}
}
}
}

val hook = Thread(::shutdown, "Arrow-kt SuspendApp JVM ShutdownHook")
Runtime.getRuntime().addShutdownHook(hook)
return {
if (!isShutdown.get()) {
Runtime.getRuntime().removeShutdownHook(hook)
try {
Runtime.getRuntime().removeShutdownHook(hook)
} catch (_: IllegalStateException) {

@nomisRev nomisRev Jul 3, 2026

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.

Does this occur? It should be gated by isShutdown, right?

@tKe tKe Jul 3, 2026

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.

Unlikely though it is, there's the possibility the runtime shutdown process has been started but our shutdown hook thread has not yet started or set the isShutdown flag. Attempting to remove hooks once the shutdown process has started will fail.

// Shutdown hook already running, ignore
}
}
}
}

override fun onSigTerm(block: suspend (code: Int) -> Unit): Unit =
addSignalHandler("SIGTERM") { block(15) }
addSignalHandler("TERM", block)
Comment thread
tKe marked this conversation as resolved.

override fun onSigInt(block: suspend (code: Int) -> Unit): Unit =
addSignalHandler("SIGINT") { block(2) }
addSignalHandler("INT", block)

private fun addSignalHandler(signal: String, action: suspend () -> Unit): Unit =
private fun addSignalHandler(signal: String, action: suspend (code: Int) -> Unit): Unit =
try {
var handle: SignalHandler? = null
@Suppress("ASSIGNED_VALUE_IS_NEVER_READ")
handle =
Signal.handle(Signal(signal)) { prev ->
runBlocking { action() }
Signal.handle(Signal(signal)) { sig ->
runBlocking { action(sig.number) }

@nomisRev nomisRev Jul 3, 2026 •
edited
Loading

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.

This violates the "exit code formula" Exit Code=128+Signal Number.

The exit code formula Exit Code=128+Signal Number is not being respected here since now we set 2 for SIGINT, and 15 for SIGTERM. This seems consistently implemented in the other platforms.

@tKe tKe Jul 3, 2026

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.

This is consistent with the other platforms where the onSigInt/onSigTerm blocks receive the signal number (i.e. 2 / 15). We could name the parameter of the lambda for clarity and avoid using unqualified code in most places but I didn't want to make too many tweaks outside of the main area of impact.

The other platforms handlers tend to do the sig+128 maths to convert an exit code in the block passed to onSigInt/onSigTerm.
i.e. { code -> exitAfter(128 + code) { block() } }

For the JVM, we let the JVM handle the exit code calculation based on the default signal handler so we needn't do the calculation here (although we could theoretically call System.halt we don't know if other shutdown hooks have finished or not, nor have control of the ordering)

if (handle != SignalHandler.SIG_DFL && handle != SignalHandler.SIG_IGN) {
handle?.handle(prev)
handle?.handle(sig)
}
}
} catch (_: Throwable) {
Expand Down
Loading