Skip to content

Better handling of ctrl-c while the compiler is running #9429

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.

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
merged 1 commit into from
Jul 27, 2020

Conversation

smarter
Copy link
Member

@smarter smarter commented Jul 24, 2020

Previously, the compiler tried its best to continue working after
ctrl-c, which usually resulted in a flood of error messages. This commit
takes inspiration from scala/scala#6479 to
handle ClosedByInterruptException and InterruptedException gracefully
and avoid this.

Note that the Scala 2 PR goes further: it checks Thread.interrupted()
to set a global cancelled flag (Should we do the same with our
Run#isCancelled ?) and it also catches InterruptedException at the
top-level so that ctrl-c does not end up printing a stacktrace, but this
is beyond what I have the time to look at currently.

@smarter smarter requested a review from odersky July 24, 2020 17:04
Previously, the compiler tried its best to continue working after
ctrl-c, which usually resulted in a flood of error messages. This commit
takes inspiration from scala/scala#6479 to
handle ClosedByInterruptException and InterruptedException gracefully
and avoid this.

Note that the Scala 2 PR goes further: it checks `Thread.interrupted()`
to set a global `cancelled` flag (Should we do the same with our
`Run#isCancelled` ?) and it also catches InterruptedException at the
top-level so that ctrl-c does not end up printing a stacktrace, but this
is beyond what I have the time to look at currently.
@odersky
Copy link
Contributor

odersky commented Jul 27, 2020

Note that the Scala 2 PR goes further: it checks Thread.interrupted()
to set a global cancelled flag (Should we do the same with our
Run#isCancelled ?)

The idea of Run#isCancelled is that it provides a way to exit gracefully, without throwing exceptions. For ^C we might not have to do that and the mechanism in the PR could be enough. It's a welcome improvement for sure.

@odersky odersky assigned smarter and unassigned odersky Jul 27, 2020
@smarter
Copy link
Member Author

smarter commented Jul 27, 2020

For ^C we might not have to do that and the mechanism in the PR could be enough.

My understanding is that ^C from sbt does not immediately causes the thread to stop with an exception, instead it sets the interrupted flag on the Thread, this flag is checked by some methods (like the one doing IO) to throw InterruptedException, but to do things well, we should check it ourselves too, from scala/scala#6479:

A common means of cancelling a task is to shutdown the thread pool
executing it. That's what SBT's CTRL-C handler does, for example.
Typically, thread pools call Thread.interrupt() to cooperatively stop the
workload. We need to do our part by checking interrupted() from time to
time, and translating this into an exception that will stop compilation.

We also need a way to communicate this to the caller of the compiler. Because compilation returns a Reporter, scalac sets a Reporter#cancelled flag which is then checked by the sbt compiler bridge. We probably want to do the same, but then we should also see if we can get rid of Run#isCancelled since these two things would be way too similar.

@smarter smarter merged commit 240a864 into scala:master Jul 27, 2020
@smarter smarter deleted the better-ctrl-c branch July 27, 2020 14:11
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.

2 participants