-
Notifications
You must be signed in to change notification settings - Fork 62
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
100% CPU in a $EDITOR command due to the O_NONBLOCK set on STDIN_FILENO #396
Comments
Thanks for reporting this. I just tried this with neovim and it doesn't seem to have this issue. The reason why we make stdin non-blocking is to work around a WSL bug: microsoft/WSL#3507 (reported in tiny as #269) We do it here: tiny/crates/term_input/src/lib.rs Lines 244 to 265 in 847dfff
I discovered this issue in 2020, I wonder if the bug is fixed in recent versions of WSL. Maybe we can just remove this code. I'm on vacation now and don't have access to a Windows machine, so this will have to wait until next week. Assuming the problem with WSL still exists, I agree that we may want to turn it off before running
Do they restore the mode on exit, or just update it on startup and then leave it with nonblock unset? |
My patch (for my ex-vi) tries to restore the O_NONBLOCK at exit. https://github.com/thrig/vi/commit/0aef961b32a7e5fd1a4861dda3883090255b97ed ... unless the process crashes, in which case O_NONBLOCK will not be restored. It is almost certain that the OpenBSD folks will not accept the patch, though, as stdin is expected to be blocking; otherwise, one would need to add such a check to main() of practically every utility on unix, and likewise attempt to restore the flags, which again may not happen if the process crashes, or does an exec over to something else. Can you set O_NONBLOCK only for WSL hosts? Or to have a compile flag to set the desired state? |
We can do that, but I'm not sure if there's a reliable way of checking this. I found this issue microsoft/WSL#423 which shows a few ways of checking this, but I don't know if any of them are (1) reliable (2) work across versions we want to support (I don't even know how many versions exists and which ones we want to support).
I don't want any more compile flags. We already have a few and it becomes difficult to test every combination of flags as the number increases. It also means more stuff to document, more release binaries etc. We can transparently fix this issue, I just can't do it until next week. PRs are always welcome. |
Not sure if this is in your code or a library, but setting O_NONBLOCK on STDIN_FILENO and then forking out to an $EDITOR that assumes STDIN_FILENO is blocking can case 100% CPU usage in the $EDITOR. It may be prudent to default to turning off O_NONBLOCK across the fork/exec of the $EDITOR, and then to ensure that $EDITOR has not fiddled around with the blocking of the shared STDIN_FILENO afterwards.
(This was on OpenBSD 7.2 using ex(1) as the $EDITOR; I also have mailed a patch to the OpenBSD folks to turn off O_NONBLOCK in ex and vi.)
The text was updated successfully, but these errors were encountered: