std::io::stdin() is causing significant trouble, losing data to its buffer

[chris-morgan]

Currently (for three months, since #12422), stdin() returns a buffered reader.

This is causing lots of trouble with people that write

let a = io::stdin().read_line();
let b = io::stdin().read_line();

The first read will typically consume all the data from stdin, and so b ends up None, the rest of the first stdin() call having mysteriously vanished.

Documentation that you should only call stdin() once is not sufficient. It needs to be either a compile error in some way (once fn stdin()!?) or calling stdin() multiple times must be safe. (Causing task failure, “you’ve already called stdin()!”, would probably not be considered acceptable.)

One suggestion is to switch it to using task-local storage. This is imperfect (reading stdin from multiple tasks is broken), but generally acceptable.

Having stdin/stdout/stderr as true globals in some way is another possibility.

Yet another suggestion, aired by @o11c, is passing stdin, stdout and stderr as arguments to main. (There are certainly significant difficulties with that scheme.)

This needs to be fixed urgently. It appears to be tripping up a significant number of people and is very much non-obvious.

[o11c]

With a little further thought, stderr needs to be accessible to things like fail!. Of course, it's not necssarily the case that fail! has to use the same stderr ... but it might be best to exclude stderr. Actually, I'm not convinced that stderr should entirely be an ordinary stream.

However, I do still defend the notion of passing stdout as an argument to main, even though it doesn't have as many problems as stdin does. It just plain leads to better design. In C or C++, how many dump methods are there that are hard-coded to write to stdout because there is a family of functions that uses it implicitly? Sometimes stderr may be more appropriate, but in C stderr is a second-class citiizen, and often it would be nice to be able to specify an arbitrary file.

By "argument to main" I think there should be a single struct that contains the two standard streams and whatever else makes sense to put there. (personally, I would actually prefer that os::args and os::args also be members, but I'm not going to fight for that) That same struct should maybe be reused when creating tasks, but I'm unsure of what kind of lifetime it and its members will need to have.

[seanmonstar]

it looks like there's already a with_local_stdout: https://github.com/mozilla/rust/blob/master/src/libstd/io/stdio.rs#L207

Would a solution here be that stdin(), stdout(), and stderr() all use a similar with_local_std<io>?

[sfackler]

with_local_foo isn't a great solution since it's unsafe. A better solution would probably be to have the BufferedWriter/Reader be stored in the task's internals and stdin() etc would return smart pointers back to that Reader/Writer. You'd still run into issues if you try to read from stdin in multiple tasks, so it might need to be a global synchronized singleton which would be rather unfortunate.

[o11c]

I'd rather see stdin() return an Option, where it returns None in all other tasks but the main one if you didn't specify a stdin when starting the task

[ben0x539]

To begin with, we could rename it to create_buffered_stdin() or something similarly "you shouldn't call this for every line of input"-sounding. If I still mess it up, losing my input isn't really unsafe, so...

Even if we change the standard streams to be arguments to main, chances are people who feel like using them deep within some code without having them passed down all the way from main will just go "gee, rust is making this too complicated" and call the libc stdio functions (and that will conflict with whatever we do, unless we go the C++ route of building our standard streams on top of stdio).

[aturon]

cc me

[sfackler]

It seems like stdin should really return a Reader wrapping a Arc<Mutex<BufferedReader<StdReader>>>.

[sfackler]

So it seems like there are two options: We can use thread-local BufferedReader<StdReader>s or a global singleton Reader wrapping an Arc<Mutex<BufferedReader<StdReader>>>. The first option is easier, but it runs into problems if multiple threads call stdin(). The second option has some weird side effects. For example, we can't actually implement Buffer for it, since we can't safely implement fill_buf and consume doesn't make sense. We could manually implement read_line, read_until, read_char and BufferPrelude. I'm working on the second option right now since it seems like there will be less surprises to users.