From e22e69a31edd3089514436174f97d4d8d96ed194 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 5 Dec 2017 18:28:56 +0100 Subject: [PATCH 1/2] ptyfwd: before deciding that a pty is fully drained, ask the kernel again Apparently there's no guarantee that EPOLLIN is immediately propagated from a pty slave to the master when data is written to it, hence it's not sufficient to check EPOLLIN to decide whether the pty device is drained. Let's fix this by asking the kernel directly through SIOCINQ + SIOCOUTQ, if there's anything buffered left. Fixes: #7531 --- src/shared/ptyfwd.c | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c index 487a013148b..3cc2e187cc4 100644 --- a/src/shared/ptyfwd.c +++ b/src/shared/ptyfwd.c @@ -171,6 +171,30 @@ static bool ignore_vhangup(PTYForward *f) { return false; } +static bool drained(PTYForward *f) { + int q = 0; + + assert(f); + + if (f->out_buffer_full > 0) + return false; + + if (f->master_readable) + return false; + + if (ioctl(f->master, TIOCINQ, &q) < 0) + log_debug_errno(errno, "TIOCINQ failed on master: %m"); + else if (q > 0) + return false; + + if (ioctl(f->master, TIOCOUTQ, &q) < 0) + log_debug_errno(errno, "TIOCOUTQ failed on master: %m"); + else if (q > 0) + return false; + + return true; +} + static int shovel(PTYForward *f) { ssize_t k; @@ -306,7 +330,7 @@ static int shovel(PTYForward *f) { /* If we were asked to drain, and there's nothing more to handle from the master, then call the callback * too. */ - if (f->drain && f->out_buffer_full == 0 && !f->master_readable) + if (f->drain && drained(f)) return pty_forward_done(f, 0); return 0; @@ -547,6 +571,5 @@ bool pty_forward_drain(PTYForward *f) { */ f->drain = true; - - return f->out_buffer_full == 0 && !f->master_readable; + return drained(f); } From d147457cc907ace9bfde8fc08a27616b864973d1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 5 Dec 2017 18:31:32 +0100 Subject: [PATCH 2/2] run: run pty forwarder at higher event priority than the bus We want any tty I/O to happen before we look at service messages, hence let's set priorities on them, and give tty I/O a higher priority. --- src/run/run.c | 5 ++++- src/shared/ptyfwd.c | 23 +++++++++++++++++++++++ src/shared/ptyfwd.h | 2 ++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/run/run.c b/src/run/run.c index db833c29035..bd9a6b2c816 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -1085,6 +1085,9 @@ static int start_transient_service( return log_error_errno(r, "Failed to create PTY forwarder: %m"); pty_forward_set_handler(c.forward, pty_forward_handler, &c); + + /* Make sure to process any TTY events before we process bus events */ + (void) pty_forward_set_priority(c.forward, SD_EVENT_PRIORITY_IMPORTANT); } path = unit_dbus_path_from_name(service); @@ -1100,7 +1103,7 @@ static int start_transient_service( if (r < 0) return log_error_errno(r, "Failed to add properties changed signal."); - r = sd_bus_attach_event(bus, c.event, 0); + r = sd_bus_attach_event(bus, c.event, SD_EVENT_PRIORITY_NORMAL); if (r < 0) return log_error_errno(r, "Failed to attach bus to event loop."); diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c index 3cc2e187cc4..94a4dd513f9 100644 --- a/src/shared/ptyfwd.c +++ b/src/shared/ptyfwd.c @@ -573,3 +573,26 @@ bool pty_forward_drain(PTYForward *f) { f->drain = true; return drained(f); } + +int pty_forward_set_priority(PTYForward *f, int64_t priority) { + int r; + assert(f); + + r = sd_event_source_set_priority(f->stdin_event_source, priority); + if (r < 0) + return r; + + r = sd_event_source_set_priority(f->stdout_event_source, priority); + if (r < 0) + return r; + + r = sd_event_source_set_priority(f->master_event_source, priority); + if (r < 0) + return r; + + r = sd_event_source_set_priority(f->sigwinch_event_source, priority); + if (r < 0) + return r; + + return 0; +} diff --git a/src/shared/ptyfwd.h b/src/shared/ptyfwd.h index ee04fca2c0b..6a0e0c6a2b9 100644 --- a/src/shared/ptyfwd.h +++ b/src/shared/ptyfwd.h @@ -54,4 +54,6 @@ void pty_forward_set_handler(PTYForward *f, PTYForwardHandler handler, void *use bool pty_forward_drain(PTYForward *f); +int pty_forward_set_priority(PTYForward *f, int64_t priority); + DEFINE_TRIVIAL_CLEANUP_FUNC(PTYForward*, pty_forward_free);