erlang / otp

Erlang/OTP
http://erlang.org
Apache License 2.0
11.32k stars 2.94k forks source link

Reading byte-by-byte fails on Erlang/OTP 26 with binary IO #7286

Closed josevalim closed 1 year ago

josevalim commented 1 year ago

Describe the bug

$ echo "x" | erl -noshell -eval 'io:setopts([binary]), erlang:display(io:get_chars("", 1)), erlang:display(io:get_chars("", 1)).'
<<"x">>
{error,collect_chars}

Expected behavior

In previous Erlang versions it would return <<"\n">> on the second line.

Affected versions

Erlang/OTP 26.

Additional context

Originally reported here: https://github.com/elixir-lang/elixir/issues/12594

josevalim commented 1 year ago

Reverting #6881 fixes the issue.

josevalim commented 1 year ago

Here is a simpler way to reproduce the bug from the previous PR:

$ cat | erl -noshell -eval 'io:setopts([binary]), erlang:display(file:read(standard_io, 3)), erlang:display(file:read(standard_io, 3)).'
fo
{ok,<<102,111,10>>}
eof

I.e. start cat and type only fo and newline. It returns eof but stdin is still open.

josevalim commented 1 year ago

So the root cause of the issue is this line:

https://github.com/erlang/otp/pull/6881/files#diff-5eebd9f99e07c97d6ae3e3d5056e424e5bd945cd766afeb3ad71abae25b08e90R521

When it returns eof, it does not mean stdin closed. It means we reached the end of the buffer. The PR above attempted to change how collect_chars work but unfortunately it introduced regressions. I propose reverting it because I don't think the solution is in collect_chars.

The only solution I can think right now is to introduce some sort of unknown state. If we return eof, we abort when we should not. If we return an empty buffer, then we return empty strings instead of eof. So we need to communicate we don't know the buffer status until we read and then change at least edlin:edit_line and group:cast to handle it. It would be nice if we could add tests for both this PR and #6881 but I am not sure how feasible it is.

garazdawi commented 1 year ago

I've done a solution that fixes the problem in the example you posted in #7384 . Can you check if it also solves your original problem?

josevalim commented 1 year ago

Yes it does, thank you!