-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add simple stdin support #130
base: main
Are you sure you want to change the base?
Conversation
|
||
public interface IInputSource | ||
{ | ||
IEnumerable<string> GetInput(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see what you're going for here.
My one basically using IObservable... IObservable is the mirror-image of IEnumerable so you end up with the same approach, just synchronous instead of async.
That said, while sync is much simpler, I'm not sure it's safe or correct 🤔 Will be fun to chat through it.
{ | ||
try | ||
{ | ||
foreach (var val in stdInSource!.GetInput()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the first iteration of my stdin one, I effectively wrote this; I just had an array of strings for stdin, and I wrote them all in a foreach loop exactly like this one.
I found though, that when I ran the test that asks for more than one line of input, it didn't work. What appeared to happen was that the first line of text would get sent to the first prompt, the second line of text would get dropped because the shell script hadn't yet asked for any input, and then shortly later when it did ask, it was too late.
I see you've brought that test across in this PR. Does it work for you? If so my hypothesis above must have been wrong in some way.
Anyway, while this should work well for simple programs, there are two things which give me pause
- We are single-threaded in the path of launching the process. If someone were to set up a stdin string for a program that wasn't expecting it, it might never read from stdin and our write would block forever. Our whole code-path would stall.
- Related to that: If our input enumerable takes a long time to enumerate, we'd also stall
- This approach works for small data sizes only because C# Streams have inbuilt buffering. If you exceed the buffer size, then writes will block until the other end starts to consume the data. Stall
- As I discovered, some programs exit when stdin is closed.
octopus.server run
is such a thing, and ross needed to pass in a dummy stdin stream that never closes, to stop it shutting itself down immediately. With an IEnumerable-based stream like this, how could we pull that off?
No description provided.