cathode icon indicating copy to clipboard operation
cathode copied to clipboard

Cancellation on ReadAsync causes next ReadAsync to skip a byte (raw mode)

Open scottbilas opened this issue 10 months ago • 9 comments

I'm unsure if this is a bug or I'm using the API wrong, but a canceled ReadAsync causes the next ReadAsync to ignore the next stdin byte, and then behaves normally after that.

To repro, run the below code. Type 'a', wait to allow the cancel, then type 'b' then 'c'. You will see a "c" for step 3, but it should show a "b".

Is there some state that I need to clear after a ReadAsync cancellation?

using static Vezel.Cathode.Terminal;

EnableRawMode();

var buf = new byte[1];

// read key press, works ok
await OutAsync("1: ");
await ReadAsync(buf);
await OutLineAsync((char)buf[0]);

// do a timeout cancel
try
{
    var cancel = new CancellationTokenSource();
    cancel.CancelAfter(100);

    await OutAsync("2: ");
    await ReadAsync(buf, cancel.Token);
    await OutLineAsync((char)buf[0]);
}
catch (OperationCanceledException)
{
    await OutLineAsync("cancel");
}

// this one requires two keypresses
await OutAsync("3: ");
await ReadAsync(buf);
await OutLineAsync((char)buf[0]);

scottbilas avatar Apr 28 '24 20:04 scottbilas

What's the OS and terminal emulator?

alexrp avatar Apr 28 '24 20:04 alexrp

To repro, run the below code. Type 'a', wait to allow the cancel, then type 'b' then 'c'.

~~AFAICS, the repro code will only allow two inputs due to the cancellation?~~ I guess it's because I'm trying on Linux where it works properly. So I assume the bug is Windows-specific?

alexrp avatar Apr 28 '24 20:04 alexrp

This is on Windows in the Windows Terminal.

If I switch to WSL in the same terminal, it works as I'd expect. I also tested Mac, same. So it indeed looks like a bug just on Windows. I can work around it for now by setting up a separate task to read without a timeout into an additional pipe and putting the timeout on reading from that.

Separately, I noticed that on Mac/linux, OutLine only does \n but not also \r. I'd expect even on raw mode a higher level function like OutLine to do the "full newline", otherwise I'm not sure what the value of OutLine would be..behavior is surprising anyway. (Feels like the same sort of situation as the \n issue we worked through previously.)

scottbilas avatar Apr 29 '24 05:04 scottbilas

This is on Windows in the Windows Terminal.

If I switch to WSL in the same terminal, it works as I'd expect. I also tested Mac, same. So it indeed looks like a bug just on Windows. I can work around it for now by setting up a separate task to read without a timeout into an additional pipe and putting the timeout on reading from that.

Ok, I'll investigate next time I boot into Windows.

Separately, I noticed that on Mac/linux, OutLine only does \n but not also \r. I'd expect even on raw mode a higher level function like OutLine to do the "full newline", otherwise I'm not sure what the value of OutLine would be..behavior is surprising anyway. (Feels like the same sort of situation as the \n issue we worked through previously.)

OutLine() and friends use Environment.NewLine, similar to the equivalent methods on System.Console. ReadLine() can't really be made workable in raw mode; it's considered valid only in cooked mode. So while OutLine() could be made to work by just always using \r\n, it would be inconsistent with ReadLine() and so be somewhat misleading, so I didn't. The right thing to do here is probably for these methods to throw in raw mode to clarify that they're higher-level cooked mode helpers.

alexrp avatar Apr 29 '24 05:04 alexrp

The right thing to do here is probably for these methods to throw in raw mode to clarify that they're higher-level cooked mode helpers.

Great idea 👍🏽

scottbilas avatar Apr 29 '24 07:04 scottbilas

Ok, I can reproduce it. The behavior here is not encouraging.

I modified the repro to print the byte as an integer instead. If I press a, wait for cancellation, and then ø (Danish keyboard), I get:

❯ dotnet run
1: 0x61
2: cancel
3: 0xb8

Whereas running Cathode's raw sample and pressing a + ø prints:

❯ dotnet run --no-build
Entering raw mode.

0x61
0xc3
0xb8
...

So, something is indeed dropping a byte, even one that's part of a single code point, resulting in garbled input.

alexrp avatar Apr 30 '24 14:04 alexrp

Bad news:

❯ dotnet run
1: result=1, error=0, progress=1
0x61
2: result=0, error=995, progress=0
cancel
3: result=1, error=0, progress=1
0xb8

Those are the results of every ReadFile() call. The first two calls are as expected. The third call indicates that the 0xc3 byte never even reaches us. It's just lost.

I don't know if there's actually anything we can do about this. I'm tempted to say that we are, yet again, blocked by https://github.com/microsoft/terminal/issues/12143...

alexrp avatar Apr 30 '24 15:04 alexrp

It might be interesting to note that, if I modify the repro to not perform a third read, pressing ø after the program exits prints the ø properly in the shell. Whether that actually means anything useful, I'm not sure.

alexrp avatar Apr 30 '24 15:04 alexrp

@scottbilas by the way, can I get you to upvote https://github.com/microsoft/terminal/issues/12143? I need to somehow convince the team that it's worth spending time on fixing, and they seem to use upvotes a lot in their prioritization.

alexrp avatar Apr 30 '24 15:04 alexrp