ptyfwd,run: make pty_forward_drain() trigger defer event to call shovel()

drained() checks PTYForward.master_readable flag, but it may be
tentatively unset due to a tentative error like EAGAIN in the previous
IO event. Let's try to call shovel() one more time, which re-read the
master and call drained() at the end. Otherwise, we may lost some data.

diff --git a/src/run/run.c b/src/run/run.c
index 69d0490e57d33a15252fe9f8e3da7584635c1614..99aad5df776d3be588ae69b012d2d7af82cbbc1f 100644 (file)
--- a/src/run/run.c
+++ b/src/run/run.c
@@ -1818,17 +1818,24 @@ static int run_context_check_started(RunContext *c) {
 }
 
 static void run_context_check_done(RunContext *c) {
+        int r;
+
         assert(c);
 
-        bool done = STRPTR_IN_SET(c->active_state, "inactive", "failed") &&
-                !c->start_job &&   /* our start job */
-                !c->job;           /* any other job */
+        if (!STRPTR_IN_SET(c->active_state, "inactive", "failed") ||
+            c->start_job ||   /* our start job */
+            c->job)           /* any other job */
+                return;
 
-        if (done && c->forward) /* If the service is gone, it's time to drain the output */
-                done = pty_forward_drain(c->forward);
+        if (!c->forward)
+                return (void) sd_event_exit(c->event, EXIT_SUCCESS);
 
-        if (done)
-                (void) sd_event_exit(c->event, EXIT_SUCCESS);
+        /* If the service is gone, it's time to drain the output */
+        r = pty_forward_drain(c->forward);
+        if (r < 0) {
+                log_error_errno(r, "Failed to drain PTY forwarder: %m");
+                return (void) sd_event_exit(c->event, EXIT_FAILURE);
+        }
 }
 
 static int map_job(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) {
@@ -1983,6 +1990,8 @@ static int pty_forward_handler(PTYForward *f, int rcode, void *userdata) {
                 return log_error_errno(rcode, "Error on PTY forwarding logic: %m");
         }
 
+        c->forward = pty_forward_free(c->forward);
+
         run_context_check_done(c);
         return 0;
 }

diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c
index bc312f17019c52d5a486e2653e66da5680efcfc3..2b1154f5fa138383f29433eca28b7e97fc74c997 100644 (file)
--- a/src/shared/ptyfwd.c
+++ b/src/shared/ptyfwd.c
@@ -1135,18 +1135,14 @@ void pty_forward_set_hotkey_handler(PTYForward *f, PTYForwardHotkeyHandler cb, v
         f->hotkey_userdata = userdata;
 }
 
-bool pty_forward_drain(PTYForward *f) {
+int pty_forward_drain(PTYForward *f) {
         assert(f);
 
-        /* Starts draining the forwarder. Specifically:
-         *
-         * - Returns true if there are no unprocessed bytes from the pty, false otherwise
-         *
-         * - Makes sure the handler function is called the next time the number of unprocessed bytes hits zero
-         */
+        /* Starts draining the forwarder. This makes sure the handler function is called the next time the
+         * number of unprocessed bytes hits zero. */
 
         f->drain = true;
-        return drained(f);
+        return pty_forward_add_defer(f);
 }
 
 int pty_forward_set_priority(PTYForward *f, int64_t priority) {

diff --git a/src/shared/ptyfwd.h b/src/shared/ptyfwd.h
index eb6d2807c088fcd01c20c3e27d2eed58f3b64d24..1983a1cd16f9c62eec1bb4f7f51f2d41874cadc0 100644 (file)
--- a/src/shared/ptyfwd.h
+++ b/src/shared/ptyfwd.h
@@ -34,7 +34,7 @@ bool pty_forward_vhangup_honored(const PTYForward *f);
 void pty_forward_set_hangup_handler(PTYForward *f, PTYForwardHangupHandler handler, void *userdata);
 void pty_forward_set_hotkey_handler(PTYForward *f, PTYForwardHotkeyHandler handler, void *userdata);
 
-bool pty_forward_drain(PTYForward *f);
+int pty_forward_drain(PTYForward *f);
 
 int pty_forward_set_priority(PTYForward *f, int64_t priority);