Skip to content

Shutdown the runtime only when unforked #4234

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

Merged
djspiewak merged 1 commit into from
Jan 11, 2025
Merged

Shutdown the runtime only when unforked #4234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 2 additions & 3 deletions core/jvm/src/main/scala/cats/effect/IOApp.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,6 @@ trait IOApp {
IOApp.this.reportFailure(t).unsafeRunAndForgetWithoutCallback()(runtime)

case t =>
runtime.shutdown()
queue.clear()
queue.put(t)
Comment on lines 219 to 221

@armanbilge armanbilge Jan 11, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We don't have access to isForked here. But, I think this particular case doesn't matter, because t is a fatal exception, and point of fatal exceptions in that they may leave the JVM in an inconsistent state (such as stuff not being shutdown/closed). Furthermore it may never have been safe to attempt shutdown here anyway—we don't entirely control this shutdown code, so we couldn't be certain it's not allocating.

}
Expand Down Expand Up @@ -504,7 +503,7 @@ trait IOApp {

// Clean up after ourselves, relevant for running IOApps in sbt,
// otherwise scheduler threads will accumulate over time.
runtime.shutdown()
if (!isForked) runtime.shutdown()
}

val hook = new Thread(() => handleShutdown())
Expand All @@ -527,7 +526,7 @@ trait IOApp {
case ec: ExitCode =>
// Clean up after ourselves, relevant for running IOApps in sbt,
// otherwise scheduler threads will accumulate over time.
runtime.shutdown()
if (!isForked) runtime.shutdown()
if (ec == ExitCode.Success) {
// Return naturally from main. This allows any non-daemon
// threads to gracefully complete their work, and managed
Expand Down
3 changes: 1 addition & 2 deletions tests/jvm/src/main/scala/catseffect/examplesplatform.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ package examples {
override protected def runtimeConfig =
super.runtimeConfig.copy(shutdownHookTimeout = Duration.Zero)

val run: IO[Unit] =
IO.blocking(System.exit(0)).uncancelable
val run: IO[Unit] = IO(System.exit(0))

@armanbilge armanbilge Jan 11, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Restores this test to the original state*, now that we fixed this gotcha.

* The original test had uncancelable and I'm not sure why, since I don't think it does anything.

}

object FatalErrorUnsafeRun extends IOApp {
Expand Down