From 97509aa2be00b366f10fa4e97506e7b96d514345 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Mon, 23 Feb 2026 12:28:17 +0800 Subject: [PATCH] fix(process): handle poll() interrupted by a signal (#38024) It is indeed possible that a signal may arrive during flush_stream() (e.g. SIGCHLD from another child process), so poll() again on EINTR. Fixes the following Coverity warning: _____________________________________________________________________________________________ *** CID 644345: Error handling issues (CHECKED_RETURN) /src/nvim/event/proc.c: 405 in flush_stream() 399 400 #ifdef __linux__ 401 // On Linux, libuv's polling (which uses epoll) doesn't flush PTY master's pending 402 // work on kernel workqueue, so use an explcit poll() before that. #37982 403 if (proc->type == kProcTypePty && !stream->did_eof) { 404 struct pollfd pollfd = { .fd = ((PtyProc *)proc)->tty_fd, .events = POLLIN }; >>> CID 644345: Error handling issues (CHECKED_RETURN) >>> Calling "poll(&pollfd, 1UL, 0)" without checking return value. This library function may fail and return an error code. 405 poll(&pollfd, 1, 0); 406 } 407 #endif 408 // Poll for data and process the generated events. 409 loop_poll_events(proc->loop, 0); 410 if (stream->s.events) { Another possible error is ENOMEM, which is probably not worth handling. For reference, #23308 previously removed a case of ENOMEM handling, and this PR also removes an outdated mention of that. Also reduce the number of #ifdefs in non-OS-specific files. --- src/nvim/channel.c | 2 -- src/nvim/event/proc.c | 10 +--------- src/nvim/os/fs.c | 2 +- src/nvim/os/pty_proc_unix.c | 23 ++++++++++++++++++++++- src/nvim/os/pty_proc_win.c | 10 ++++++++++ 5 files changed, 34 insertions(+), 13 deletions(-) diff --git a/src/nvim/channel.c b/src/nvim/channel.c index ef41d7eb78..465c678e5d 100644 --- a/src/nvim/channel.c +++ b/src/nvim/channel.c @@ -849,10 +849,8 @@ static void term_resize(uint16_t width, uint16_t height, void *data) static void term_resume(void *data) { -#ifdef UNIX Channel *chan = data; pty_proc_resume(&chan->stream.pty); -#endif } static inline void term_delayed_free(void **argv) diff --git a/src/nvim/event/proc.c b/src/nvim/event/proc.c index 2413ab68d4..009b8b2a7e 100644 --- a/src/nvim/event/proc.c +++ b/src/nvim/event/proc.c @@ -3,9 +3,6 @@ #include #include #include -#ifdef __linux__ -# include -#endif #include "klib/kvec.h" #include "nvim/channel.h" @@ -397,14 +394,9 @@ static void flush_stream(Proc *proc, RStream *stream) // Remember number of bytes before polling. size_t num_bytes = stream->num_bytes; -#ifdef __linux__ - // On Linux, libuv's polling (which uses epoll) doesn't flush PTY master's pending - // work on kernel workqueue, so use an explcit poll() before that. #37982 if (proc->type == kProcTypePty && !stream->did_eof) { - struct pollfd pollfd = { .fd = ((PtyProc *)proc)->tty_fd, .events = POLLIN }; - poll(&pollfd, 1, 0); + pty_proc_flush_master((PtyProc *)proc); } -#endif // Poll for data and process the generated events. loop_poll_events(proc->loop, 0); if (stream->s.events) { diff --git a/src/nvim/os/fs.c b/src/nvim/os/fs.c index ecd013c4e2..93a3750a7b 100644 --- a/src/nvim/os/fs.c +++ b/src/nvim/os/fs.c @@ -563,7 +563,7 @@ int os_open_stdin_fd(void) /// Read from a file /// -/// Handles EINTR and ENOMEM, but not other errors. +/// Handles EINTR, but not other errors. /// /// @param[in] fd File descriptor to read from. /// @param[out] ret_eof Is set to true if EOF was encountered, otherwise set diff --git a/src/nvim/os/pty_proc_unix.c b/src/nvim/os/pty_proc_unix.c index cc5cd899fb..4b6bc9c5cb 100644 --- a/src/nvim/os/pty_proc_unix.c +++ b/src/nvim/os/pty_proc_unix.c @@ -31,6 +31,9 @@ int forkpty(int *, char *, const struct termios *, const struct winsize *); #ifdef __APPLE__ # include #endif +#ifdef __linux__ +# include +#endif #include "auto/config.h" #include "klib/kvec.h" @@ -240,12 +243,29 @@ void pty_proc_resize(PtyProc *ptyproc, uint16_t width, uint16_t height) } void pty_proc_resume(PtyProc *ptyproc) + FUNC_ATTR_NONNULL_ALL { // Send SIGCONT to the entire process group, as some shells (e.g. fish) don't // propagate SIGCONT to suspended child processes. killpg(((Proc *)ptyproc)->pid, SIGCONT); } +/// On Linux, libuv's polling (which uses epoll) doesn't flush PTY master's pending +/// work on kernel workqueue, so use an explcit poll() before that. #37982 +/// Note that poll() only flushes pending work if no data is immediately available, +/// so this function is needed before every libuv poll in flush_stream(). +void pty_proc_flush_master(PtyProc *ptyproc) + FUNC_ATTR_NONNULL_ALL +{ +#ifdef __linux__ + struct pollfd pollfd = { .fd = ptyproc->tty_fd, .events = POLLIN }; + int n = 0; + do { + n = poll(&pollfd, 1, 0); + } while (n < 0 && errno == EINTR); +#endif +} + void pty_proc_close(PtyProc *ptyproc) FUNC_ATTR_NONNULL_ALL { @@ -256,7 +276,8 @@ void pty_proc_close(PtyProc *ptyproc) } } -void pty_proc_close_master(PtyProc *ptyproc) FUNC_ATTR_NONNULL_ALL +void pty_proc_close_master(PtyProc *ptyproc) + FUNC_ATTR_NONNULL_ALL { if (ptyproc->tty_fd >= 0) { close(ptyproc->tty_fd); diff --git a/src/nvim/os/pty_proc_win.c b/src/nvim/os/pty_proc_win.c index 3c23c0f4df..2930f55f95 100644 --- a/src/nvim/os/pty_proc_win.c +++ b/src/nvim/os/pty_proc_win.c @@ -180,6 +180,16 @@ void pty_proc_resize(PtyProc *ptyproc, uint16_t width, uint16_t height) os_conpty_set_size(ptyproc->conpty, width, height); } +void pty_proc_resume(PtyProc *ptyproc) + FUNC_ATTR_NONNULL_ALL +{ +} + +void pty_proc_flush_master(PtyProc *ptyproc) + FUNC_ATTR_NONNULL_ALL +{ +} + void pty_proc_close(PtyProc *ptyproc) FUNC_ATTR_NONNULL_ALL {