Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #4184
The basic problem is that we are spawning an
PumpedProcessInput
from the Mill server's redirectedstdin
to the stdin of any subprocess we spawn, but thePumpedProcessInput
'sInputPumper
thread only terminates on the source stream closing, and we never close the redirectedstdin
of the Mill server.Reading from the shared
stdin
from multiple tasks/threads concurrently doesn't really make sense, so we should probably not usePumpedProcessInput
at all and just pass in aDummyInputStream
with zero data. Interactive use cases like consoles and repl are handled by theos.InheritRaw
codepath anyway and so won't be affected.stdout
/stderr
aren't affected, because having multiple tasks/threads writing lines to the same output stream is probably fine (although we could probably do a better job at ensuring it is done at a line-by-line granularity without splitting of lines between threads/tasks)Tested manually, ran the commands in the linked issue and verified that without this PR the leaked thread can be seen in
jstack
, but after this PR the leaked thread is no longer presentThis might also fix some of the cases where enter-on-rerun for
--watch
stops worked, perhaps because a leakedInputPumper
is grabbing the\n
character before it can be read by theWatching.scala
logic