From: Yu Watanabe <watanabe.yu+github@gmail.com>
Date: Wed, 25 Dec 2024 08:40:04 +0000 (+0900)
Subject: ptyfwd,run: process remaining outputs in IO event sources
X-Git-Tag: v258-rc1~1778^2~1
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=7647378b74a3a701bfcfea69e5b8514f32efad4b;p=thirdparty%2Fsystemd.git

ptyfwd,run: process remaining outputs in IO event sources

This partially reverts 12807b5a49d1fe60434d473afe11ff81a4c92306.

Otherwise, reading or writing a fd in on_exit_event() handler may return
EBUSY, and the event loop may finish with -ELOOP.

Also, this makes drained() returns true if the PTY forwarder is already
disconnected, for safety. Hence, it is not necessary to re-introduce
pty_forward_is_done().
---

diff --git a/src/run/run.c b/src/run/run.c
index 72f8affc2dd..28953a6514b 100644
--- a/src/run/run.c
+++ b/src/run/run.c
@@ -1557,7 +1557,12 @@ static int run_context_reconnect(RunContext *c) {
 static void run_context_check_done(RunContext *c) {
         assert(c);
 
-        if (STRPTR_IN_SET(c->active_state, "inactive", "failed") && !c->has_job)
+        bool done = STRPTR_IN_SET(c->active_state, "inactive", "failed") && !c->has_job;
+
+        if (done && c->forward) /* If the service is gone, it's time to drain the output */
+                done = pty_forward_drain(c->forward);
+
+        if (done)
                 (void) sd_event_exit(c->event, EXIT_SUCCESS);
 }
 
diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c
index e66c4edf581..901a73dafde 100644
--- a/src/shared/ptyfwd.c
+++ b/src/shared/ptyfwd.c
@@ -81,6 +81,7 @@ struct PTYForward {
         bool read_from_master:1;
 
         bool done:1;
+        bool drain:1;
 
         bool last_char_set:1;
         char last_char;
@@ -240,6 +241,9 @@ static bool drained(PTYForward *f) {
 
         assert(f);
 
+        if (f->done)
+                return true;
+
         if (f->out_buffer_full > 0)
                 return false;
 
@@ -759,6 +763,11 @@ static int do_shovel(PTYForward *f) {
                         return pty_forward_done(f, 0);
         }
 
+        /* If we were asked to drain, and there's nothing more to handle from the master, then call the callback
+         * too. */
+        if (f->drain && drained(f))
+                return pty_forward_done(f, 0);
+
         return 0;
 }
 
@@ -1086,6 +1095,20 @@ void pty_forward_set_handler(PTYForward *f, PTYForwardHandler cb, void *userdata
         f->userdata = userdata;
 }
 
+bool 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
+         */
+
+        f->drain = true;
+        return drained(f);
+}
+
 int pty_forward_set_priority(PTYForward *f, int64_t priority) {
         int r;
 
diff --git a/src/shared/ptyfwd.h b/src/shared/ptyfwd.h
index 615e1056e88..a6e729d7bc6 100644
--- a/src/shared/ptyfwd.h
+++ b/src/shared/ptyfwd.h
@@ -33,6 +33,8 @@ bool pty_forward_get_ignore_vhangup(PTYForward *f);
 
 void pty_forward_set_handler(PTYForward *f, PTYForwardHandler handler, void *userdata);
 
+bool pty_forward_drain(PTYForward *f);
+
 int pty_forward_set_priority(PTYForward *f, int64_t priority);
 
 int pty_forward_set_width_height(PTYForward *f, unsigned width, unsigned height);