GNOME Bugzilla – Bug 794591
gst-play-1.0 leaves stdin in non-blocking mode
Last modified: 2018-03-26 12:27:27 UTC
Hi, I noticed that *after* executing "gst-play-1.0 SOMEFILE" in bash, interactive cli programs that handle stdin in a special way start to misbehave. One simple example is: read SOMEVAR which fails with the following error: bash: read: read error: 0: Resource temporarily unavailable Another command which fails is: git add -p where the interactive prompt to choose the action stops working. The same effect of running gst-play-1.0 can be simulated with this command: python3 -c $'import os\nos.set_blocking(0, False)' so it looks like gst-play-1.0 is not resetting the stdin blocking status correctly. This is my environment: Debian Unstable GStreamer Core Library version 1.14.0 GNU bash, version 4.4.18(1)-release (x86_64-pc-linux-gnu) The issue does not seem to happen to everybody, Jan Schmidt told me on IRC: <thaytan> ao2, definitely doesn't happen on Fedora with bash 4.4.19 (or any other version in the last few years) The issue does not occur with dash (the Debian POSIX shell), which seems to reset the blocking status automatically and warns with the following message: sh: turning off NDELAY mode However the problem occurs every time here with bash. I took a look at the code and I am going to send a proof-of-concept patch which fixes the issue. Thanks, Antonio
Created attachment 369997 [details] [review] tools: play: restore STDIN blocking status when exiting When gst_play_kb_set_key_handler() gets called from restore_terminal() it resets the terminal flags but not the STDIN blocking status. This can result in broken behavior for cli command executed in the same terminal after gst-play-1.0 exited. Restore the STDIN blocking status to fix that.
In the final patch I could save the stdin flags in a global variable and restore them as they were instead of forcing blocking mode unconditionally at exit. I mean just like it's done for term_settings, let me know if that is OK. Thanks, Antonio
Nice catch! I've noticed these errors too (on debian sid with gst-uninstalled), but it just hadn't occured to me that it could be gst-play's fault and outlive any application. Do we actually need to set stdin to non-blocking in the first place? We hook into the glib event loop to pick up input, so I don't know if there was a reason for that or not.
(In reply to Tim-Philipp Müller from comment #3) [...] > Do we actually need to set stdin to non-blocking in the first place? We hook > into the glib event loop to pick up input, so I don't know if there was a > reason for that or not. I verified that just removing the code about O_NONBLOCK breaks interactive mode. I don't know glib internals but, assuming it select/polls the fd, the terminal still needs to be configured so that the data gets read() as soon as it arrives. Clearing ICANON is not enough for that to happen. Setting the fd to non-block achieves the desired effect. However from man termios it turns out the proper way to achieve the polling read() with the terminal in raw mode is to set c_cc[VMIN] and c_cc[VTIME] to 0 in struct termios. Also in Advanced Programming in the Unix Environment 2nd edition by Richard Stevens, Section 4.5.9, it is strongly recommended to set VMIN and VTIME explicitly when clearing ICANON: The subscripts VMIN and VTIME of the c_cc array hold MIN and TIME. As those positions may be the same ones used by VEOF and VEOL, make sure you set MIN and TIME explicitly or you might get whatever the current EOF and EOL characters work out to, which will cause very strange results. If you ever find a process getting input on every fourth character typed, this is probably what’s happened. (Ctrl-d, the usual EOF character, is 4.) So let's just do that and with VMIN and VTIME set to 0, using O_NONBLOCK is not needed anymore. As a side note, using O_NONBLOCK is even discouraged in some cases, from the same section of the book: The standards don’t specify exactly what happens if O_NONBLOCK is set and MIN and/or TIME are nonzero. O_NONBLOCK may have precedence (causing an immediate return if no characters are available), or MIN/TIME may have precedence. To avoid this uncertainty, you should not set O_NONBLOCK when ICANON is clear. This does not apply here because we'll just set the values to 0, but I take this as a hint that O_NONBLOCK is not the right mechanism for terminals, even if sometimes it happens to have the desired effect. New patch coming up. Ciao, Antonio
Created attachment 370020 [details] [review] tools: play: fix leaving STDIN in non-blocking mode after exit gst-play-1.0 sets STDIN to non-blocking mode to have the input characters read as soon as they arrive. However, when gst_play_kb_set_key_handler() gets called from restore_terminal() it forgets to restore the STDIN blocking status. This can result in broken behavior for cli command executed in the same terminal after gst-play-1.0 exited. It turns out that putting STDIN in non-blocking mode is not even the proper way to achieve the desired effect, instead VMIN and VTIME in struct termios should be set to 0. Let's do that, and don't mess with the STDIN blocking mode now that it's not necessary.
Thanks! commit cfc1be0d61e7c4c79afbc16873cd3bc38fa55f79 Author: Antonio Ospite <ao2@ao2.it> Date: Thu Mar 22 11:12:20 2018 +0100 tools: play: fix leaving STDIN in non-blocking mode after exit gst-play-1.0 sets STDIN to non-blocking mode to have the input characters read as soon as they arrive. However, when gst_play_kb_set_key_handler() gets called from restore_terminal() it forgets to restore the STDIN blocking status. This can result in broken behavior for cli command executed in the same terminal after gst-play-1.0 exited. It turns out that putting STDIN in non-blocking mode is not even the proper way to achieve the desired effect, instead VMIN and VTIME in struct termios should be set to 0. Let's do that, and don't mess with the STDIN blocking mode now that it's not necessary. https://bugzilla.gnome.org/show_bug.cgi?id=794591