Skip to content
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

ConsoleReader should be blocking in sbt 1.4+ since System.in already is non-blocking #11061

Merged
merged 1 commit into from Dec 21, 2021

Conversation

mkurz
Copy link
Member

@mkurz mkurz commented Dec 19, 2021

Fixes #11032

In #9420 we made the run command interruptible by adding a couple of sleep statements, so that when hitting CTRL+C sbt had the chance to interrupt the thread.
To fix #10864 Greg now switched from FileDescriptor.in to System.in in #10890, which totally make sense, however it isn't that easy. Starting with sbt 1.4 System.in is non-blocking, therefore the patch introduced in #9420 makes the sbt console hang when waiting for an input. The problem is that read() recursively calls itself in the same thread that is supposed to read data from System.in, but because of the endless loop it never has the chance to do so.
Anyway, this comment confirms exactly what I debugged:

They shouldn't need to create a non blocking input stream reader because sbt makes System.in non-blocking starting with 1.4.0.

To solve this we create the exact same (legacy) ConsoleReader when using sbt <=1.3, that already was used before Greg's patch, and yes it still uses FileDescriptor.in, but that does not matter because until sbt 1.4.x that was totally ok, because there was not client mode yet.
For sbt 1.4+ we now create a blocking ConsoleReader which just wraps System.in/System.out, basically the solution Greg referred to in his pull request.

@mkurz
Copy link
Member Author

mkurz commented Dec 19, 2021

@Mergifyio backport master

@mergify
Copy link
Contributor

mergify bot commented Dec 19, 2021

backport master

🟠 Waiting for conditions to match

  • merged [:pushpin: backport requirement]

@mkurz
Copy link
Member Author

mkurz commented Dec 19, 2021

BTW: I tested this with sbt

  • 1.2.8
  • 1.3.13
  • 1.4.0
  • 1.5.7
  • 1.6.0-RC2

@mkurz
Copy link
Member Author

mkurz commented Dec 20, 2021

@Mergifyio backport 2.7.x

@mergify
Copy link
Contributor

mergify bot commented Dec 20, 2021

backport 2.7.x

🟠 Waiting for conditions to match

  • merged [:pushpin: backport requirement]

@mkurz mkurz requested a review from gmethvin December 20, 2021 22:44
Copy link
Member

@gmethvin gmethvin left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for fixing this

@mergify
Copy link
Contributor

mergify bot commented Dec 21, 2021

backport master

✅ Backports have been created

@mergify
Copy link
Contributor

mergify bot commented Dec 21, 2021

backport 2.7.x

✅ Backports have been created

@mkurz
Copy link
Member Author

mkurz commented Dec 21, 2021

Thanks Greg!

@mkurz mkurz deleted the sbt_enter_fix branch December 21, 2021 11:00
mergify bot added a commit that referenced this pull request Dec 21, 2021
ConsoleReader should be blocking in sbt 1.4+ since System.in already is non-blocking (backport #11061)
mergify bot added a commit that referenced this pull request Dec 21, 2021
ConsoleReader should be blocking in sbt 1.4+ since System.in already is non-blocking (backport #11061)
@mkurz mkurz modified the milestones: 2.8.12, 2.8.13 Jan 12, 2022
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.

None yet

2 participants