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
windows: unexpected abort on fast paging with color content #378
Comments
I would think that if the char is not intr_char, it should be ungotten by calling ungetcc_back() like check_poll() does in the non-MSDOS case. However it does seem strange that the char is 0. That could be the first byte of an extended key (pending_scancode) or it could be that the WideCharToMultiByte in WIN32getch returned 0. I'm not sure if ungetting the char would help in either of those cases. @adoxa Jason, any thoughts? BTW, just to confirm, when you say that less "aborts", you mean that the scrolling stops, not that less terminates, correct? |
No. It exits to the shell, with exit code 0.
It could be, but it happens since v570 (specifically, since the ^X handler was added), while the windows unicode hander was added at v605. Also, it would help if someone other than me can reproduce the issue too. |
Here are some interesting data points:
if (ctldisp != OPT_ONPLUS || (vt_enabled && sgr_mode))
WIN32textout(obuf, ob - obuf);
else when running on windows 10 Combined, on win10 with But the issue remains. Going further, commenting out the But even with the output completely disabled, the issue still happens (holding down page-down aborts
So I think this hypothesis doesn't hold up. Interestingly, still with the output completely disabled, it can also happen (but harder to reproduce) with non-highlighted content, e.g. with I still think there could be some timing issue, but I no longer thinks it's because the rendering is too slow.
Indeed, capturing the value returned by I did check how many times this I did manage to figure out why
pending_scancode = (ascii == 0x00);
} while (pending_scancode &&
(currentKey.scan == PCK_CAPS_LOCK || currentKey.scan == PCK_NUM_LOCK));
Hopefully someone which understands the code better could figure out what the actual bug is and how to fix it. |
My hunch is that there are two places which try to read a key, and they're interleaved and racy:
The fact that Some comment suggests that when the scan code is read, it needs two reads, so my money is on a race where these two parts gets split between the two places which try to read a key, and one of them freaks out (probably the main place which reads the key - not the |
81 is 'Q', which is a less command to quit. That is probably related to why it quits after the zero byte is consumed. I think the solution is indeed to unget the 0 byte, but in a way that WIN32getch will see it. ungetcc_back doesn't suffice to do that. I think a new unget mechanism may be needed here. |
Huh, 81 is Q in ASCII, but 81 is also page down at #define PCK_PAGEDOWN '\121' Right? So I'm holding page down, but it ends up interpreted as Q? How does it choose whether to interpret 81 as 81-the-ASCII-value-Q or 81-the-scan-code-page-down? |
See the comment at line 2949 in WIN32getch. Pressing PageDown will send the two byte sequence 00,81, while pressing Q just sends 81. If you press PageDown but the 00 gets consumed unexpectedly and only the 81 is read, it will get interpreted as a 'Q'. Actually I'm not sure I completelly understand the sequence of events; I think there might be some interaction between win32_kbhit and WIN32getch involved. I don't have a Windows build environment to investigate this right now. |
Thanks.
Well, I can test some patches/printouts if you want. |
The Windows version of iread removed the character, causing an extended sequence to be incorrectly split. Add an unget function to put the character back. Resolves gwsw#378.
|
The Windows version of iread removed the character, causing an extended sequence to be incorrectly split. Add an unget function to put the character back. Resolves #378.
Thanks. |
Tested
less
versions: latest official windows build (v633), but also my own builds since v570 (specifically, since 26e91bd "Allow ^X to exit F mode on Windows") upto and including v635.Tested windows versions: Windows 10 and Windows 7.
Steps to reproduce:
cmd.exe
).less -R file
wherefile
is a big file with color escape sequences.Expected result:
It pages till the end of the file.
Actual result:
less
aborts sooner rather than later. For me that's typically after less than 10 pages.Here's a sample C file which was highlighted using GNU source-highlite, and reproduces the issue for me:
tccgen.c.zip
Additional info:
The offending code seems to be this at
os.c
, which was originally added at 26e91bd:If I disable this code completely, e.g. by replacing
WIN32C
withWIN32C_XXXBUG
, then the issue goes away.When not disabled, and when it aborts,
win32_kbhit()
is true, andWIN32getch()
returns0
, which apparently is notintr_char
(which seem to be 24 by default), so the code block ofsigs |= S_INTERRUPT;
etc is NOT entered, and yetless
aborts.The issue is more likely to happen with a big console window.
I'm guessing that the cause is that
less
renders slower than requested (high PGDN input rate, big console, slow-to-render colors), and something gets messed up, but I didn't dig deep enough to figure out what.Another option is that it needs
ungetc()
to revert theWIN32getch()
call, but I couldn't figure out how (i.e. which function to use to unget it). But also,WIN32getch()
returning 0 also seems a bit unexpected...The text was updated successfully, but these errors were encountered: