diff options
author | Justin M. Keyes <justinkz@gmail.com> | 2015-05-21 09:45:46 -0400 |
---|---|---|
committer | Justin M. Keyes <justinkz@gmail.com> | 2015-05-27 09:32:31 -0400 |
commit | 8a782f1699e2a59a3f3e91f6d7c35a3312b82b41 (patch) | |
tree | f667c61d9f1def88d30a91df5db97c1aa971e30a /src | |
parent | 5a9ad68b258f33ebd7fa0a5da47b308f50f1e5e7 (diff) | |
download | rneovim-8a782f1699e2a59a3f3e91f6d7c35a3312b82b41.tar.gz rneovim-8a782f1699e2a59a3f3e91f6d7c35a3312b82b41.tar.bz2 rneovim-8a782f1699e2a59a3f3e91f6d7c35a3312b82b41.zip |
input: set input stream to blocking on exit
If stdin is non-blocking, many tools (e.g. cat(1), read(1)) which assume
that stdin is blocking, will break in odd ways:
read: read error: 0: Resource temporarily unavailable
cat: -: Resource temporarily unavailable
rm: error closing file
libuv puts stdin in nonblocking mode, and leaves it that way at exit
(this is apparently by design). So, before this commit, this always
works (because the shell clobbers O_NONBLOCK):
$ nvim --cmd q
$ read
...but these forms do _not_ work:
$ nvim --cmd q && read
$ echo foo | nvim --cmd q && read
$ nvim && read
After this commit, all of the above forms work.
Background:
https://github.com/fish-shell/fish-shell/commit/437b4397b9cf273922ce7b414bf6626845f15ad0#diff-41f4d294430cd8c36538999d62681ae2
https://github.com/fish-shell/fish-shell/issues/176#issuecomment-15800155
- bash (and other shells: zsh, tcsh, fish), upon returning to the
foreground, always sets fd 0 back to blocking mode. This practice only
applies to stdin, _not_ stdout or stderr (in practice these fds may be
affected anyways).
- bash/zsh/tcsh/fish do _not_ restore the non-blocking status of stdin
when _resuming a job_.
- We do _not_ save/restore the original flags visible to
fcntl(F_[SG]ETFL), because (counterintuitively) that isn't expected.
Helped-by: oni-link <knil.ino@gmail.com>
Closes #2086
Closes #2377
---
Note: The following implementation of stream_set_blocking() was
discarded, because it resulted in a failed libuv assertion[1]:
int stream_set_blocking(int fd, bool blocking)
{
uv_pipe_t stream;
uv_pipe_init(uv_default_loop(), &stream, 0);
uv_pipe_open(&stream, fd);
int retval = uv_stream_set_blocking((uv_stream_t *)&stream, blocking);
uv_close((uv_handle_t *)&stream, NULL);
return retval;
}
[1] .deps/build/src/libuv/src/unix/core.c:833: uv__io_stop: Assertion `loop->watchers[w->fd] == w' failed.
Diffstat (limited to 'src')
-rw-r--r-- | src/nvim/globals.h | 3 | ||||
-rw-r--r-- | src/nvim/if_cscope.c | 1 | ||||
-rw-r--r-- | src/nvim/main.c | 16 | ||||
-rw-r--r-- | src/nvim/misc1.c | 1 | ||||
-rw-r--r-- | src/nvim/os/input.c | 6 | ||||
-rw-r--r-- | src/nvim/os/os.h | 1 | ||||
-rw-r--r-- | src/nvim/os/rstream.c | 5 | ||||
-rw-r--r-- | src/nvim/os/stream.c | 26 |
8 files changed, 50 insertions, 9 deletions
diff --git a/src/nvim/globals.h b/src/nvim/globals.h index 1d93900a94..f012358404 100644 --- a/src/nvim/globals.h +++ b/src/nvim/globals.h @@ -1217,6 +1217,9 @@ EXTERN char *ignoredp; // If a msgpack-rpc channel should be started over stdin/stdout EXTERN bool embedded_mode INIT(= false); +/// TTY from which input was gathered. +EXTERN int global_input_fd INIT(= 0); + /// Used to track the status of external functions. /// Currently only used for iconv(). typedef enum { diff --git a/src/nvim/if_cscope.c b/src/nvim/if_cscope.c index 407709866c..b3d61038c3 100644 --- a/src/nvim/if_cscope.c +++ b/src/nvim/if_cscope.c @@ -820,6 +820,7 @@ err_closing: if (execl("/bin/sh", "sh", "-c", cmd, (char *)NULL) == -1) PERROR(_("cs_create_connection exec failed")); + stream_set_blocking(global_input_fd, true); //normalize stream (#2598) exit(127); /* NOTREACHED */ default: /* parent. */ diff --git a/src/nvim/main.c b/src/nvim/main.c index 5f0372f30e..6ae96a7c69 100644 --- a/src/nvim/main.c +++ b/src/nvim/main.c @@ -271,17 +271,15 @@ int main(int argc, char **argv) || params.output_isatty || params.err_isatty); if (reading_input) { - // Its possible that one of the startup commands(arguments, sourced scripts - // or plugins) will prompt the user, so start reading from a tty stream - // now. - int fd = fileno(stdin); + // One of the startup commands (arguments, sourced scripts or plugins) may + // prompt the user, so start reading from a tty now. + global_input_fd = fileno(stdin); if (!params.input_isatty || params.edit_type == EDIT_STDIN) { - // use stderr or stdout since stdin is not a tty and/or could be used to - // read the file we'll edit when the "-" argument is given(eg: cat file | - // nvim -) - fd = params.err_isatty ? fileno(stderr) : fileno(stdout); + // Use stderr or stdout since stdin is not a tty and/or could be used to + // read the "-" file (eg: cat file | nvim -) + global_input_fd = params.err_isatty ? fileno(stderr) : fileno(stdout); } - input_start_stdin(fd); + input_start_stdin(global_input_fd); } // open terminals when opening files that start with term:// diff --git a/src/nvim/misc1.c b/src/nvim/misc1.c index 3ae3d6e30e..13e5f273e0 100644 --- a/src/nvim/misc1.c +++ b/src/nvim/misc1.c @@ -2699,6 +2699,7 @@ void preserve_exit(void) } ml_close_all(false); // close all memfiles, without deleting + stream_set_blocking(global_input_fd, true); //normalize stream (#2598) mch_errmsg("Vim: Finished.\n"); diff --git a/src/nvim/os/input.c b/src/nvim/os/input.c index 486171b48a..5dcaee8304 100644 --- a/src/nvim/os/input.c +++ b/src/nvim/os/input.c @@ -8,6 +8,7 @@ #include "nvim/api/private/defs.h" #include "nvim/os/input.h" #include "nvim/os/event.h" +#include "nvim/os/os.h" #include "nvim/os/rstream_defs.h" #include "nvim/os/rstream.h" #include "nvim/ascii.h" @@ -61,9 +62,14 @@ void input_start_stdin(int fd) void input_stop_stdin(void) { if (!read_stream) { + // In some cases (i.e. "nvim && read") where we do not explicitly play with + // std{in,out,err}, some other module/library nevertheless sets the stream + // to non-blocking; we still must "fix" the stream (#2598) in those cases. + stream_set_blocking(global_input_fd, true); // normalize stream (#2598) return; } + rstream_set_blocking(read_stream, true); // normalize stream (#2598) rstream_stop(read_stream); rstream_free(read_stream); read_stream = NULL; diff --git a/src/nvim/os/os.h b/src/nvim/os/os.h index 69bd1ff4fd..3dd099890c 100644 --- a/src/nvim/os/os.h +++ b/src/nvim/os/os.h @@ -12,6 +12,7 @@ # include "os/mem.h.generated.h" # include "os/env.h.generated.h" # include "os/users.h.generated.h" +# include "os/stream.h.generated.h" #endif #endif // NVIM_OS_OS_H diff --git a/src/nvim/os/rstream.c b/src/nvim/os/rstream.c index 702f282d53..55bf89d768 100644 --- a/src/nvim/os/rstream.c +++ b/src/nvim/os/rstream.c @@ -297,6 +297,11 @@ void rstream_stop(RStream *rstream) } } +int rstream_set_blocking(RStream *rstream, bool blocking) +{ + return uv_stream_set_blocking(rstream->stream, blocking); +} + /// Returns the number of bytes ready for consumption in `rstream` size_t rstream_pending(RStream *rstream) { diff --git a/src/nvim/os/stream.c b/src/nvim/os/stream.c new file mode 100644 index 0000000000..35cb41081d --- /dev/null +++ b/src/nvim/os/stream.c @@ -0,0 +1,26 @@ +// Functions for working with stdio streams (as opposed to RStream/WStream). + +#include <stdio.h> +#include <stdbool.h> + +#include <uv.h> + +#ifdef INCLUDE_GENERATED_DECLARATIONS +# include "os/stream.c.generated.h" +#endif + +/// Sets the stream associated with `fd` to "blocking" mode. +/// +/// @return `0` on success, or `-errno` on failure. +int stream_set_blocking(int fd, bool blocking) +{ + int flags = fcntl(fd, F_GETFL, 0); + int err = 0; + if (!blocking && !(flags & O_NONBLOCK)) { + err = fcntl(fd, F_SETFL, flags | O_NONBLOCK); + } else if (blocking && (flags & O_NONBLOCK)) { + err = fcntl(fd, F_SETFL, flags & ~O_NONBLOCK); + } + return err == -1 ? -errno : 0; +} + |