From: Yu Watanabe <watanabe.yu+github@gmail.com>
Date: Wed, 18 Dec 2024 02:14:06 +0000 (+0900)
Subject: ptyfwd: always flush buffer and disconnect before exit
X-Git-Tag: v258-rc1~1817^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=12807b5a49d1fe60434d473afe11ff81a4c92306;p=thirdparty%2Fsystemd.git

ptyfwd: always flush buffer and disconnect before exit

Then, it is not necessary to manually drain PTY forwarder by the user
side. Also, not necessary to free PTY forwarder earlier explicitly to
make it disconnected.
---

diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c
index 1f477e682b4..97a9129ccd7 100644
--- a/src/machine/machinectl.c
+++ b/src/machine/machinectl.c
@@ -1242,8 +1242,6 @@ static int process_forward(sd_event *event, PTYForward **forward, int master, PT
                 (flags & PTY_FORWARD_IGNORE_VHANGUP) &&
                 pty_forward_get_ignore_vhangup(*forward) == 0;
 
-        *forward = pty_forward_free(*forward);
-
         if (!arg_quiet) {
                 if (machine_died)
                         log_info("Machine %s terminated.", name);
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index ee04ef8a2b6..d465cd58e25 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -5740,9 +5740,6 @@ static int run_container(
         if (r < 0)
                 return log_error_errno(r, "Failed to run event loop: %m");
 
-        if (forward)
-                forward = pty_forward_free(forward);
-
         /* Kill if it is not dead yet anyway */
         if (!arg_register && !arg_keep_unit && bus)
                 terminate_scope(bus, arg_machine);
diff --git a/src/run/run.c b/src/run/run.c
index 6a50b40c159..72f8affc2dd 100644
--- a/src/run/run.c
+++ b/src/run/run.c
@@ -1555,16 +1555,9 @@ static int run_context_reconnect(RunContext *c) {
 }
 
 static void run_context_check_done(RunContext *c) {
-        bool done;
-
         assert(c);
 
-        done = STRPTR_IN_SET(c->active_state, "inactive", "failed") && !c->has_job;
-
-        if (c->forward && !pty_forward_is_done(c->forward) && done) /* If the service is gone, it's time to drain the output */
-                done = pty_forward_drain(c->forward);
-
-        if (done)
+        if (STRPTR_IN_SET(c->active_state, "inactive", "failed") && !c->has_job)
                 (void) sd_event_exit(c->event, EXIT_SUCCESS);
 }
 
@@ -2096,11 +2089,6 @@ static int start_transient_service(sd_bus *bus) {
 
                 if (arg_wait && !arg_quiet) {
 
-                        /* Explicitly destroy the PTY forwarder, so that the PTY device is usable again, with its
-                         * original settings (i.e. proper line breaks), so that we can show the summary in a pretty
-                         * way. */
-                        c.forward = pty_forward_free(c.forward);
-
                         if (!isempty(c.result))
                                 log_info("Finished with result: %s", strna(c.result));
 
diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c
index 401ff8fce58..ea365300419 100644
--- a/src/shared/ptyfwd.c
+++ b/src/shared/ptyfwd.c
@@ -58,8 +58,8 @@ struct PTYForward {
         sd_event_source *stdin_event_source;
         sd_event_source *stdout_event_source;
         sd_event_source *master_event_source;
-
         sd_event_source *sigwinch_event_source;
+        sd_event_source *exit_event_source;
 
         struct termios saved_stdin_attr;
         struct termios saved_stdout_attr;
@@ -81,7 +81,6 @@ struct PTYForward {
         bool read_from_master:1;
 
         bool done:1;
-        bool drain:1;
 
         bool last_char_set:1;
         char last_char;
@@ -117,9 +116,9 @@ static void pty_forward_disconnect(PTYForward *f) {
 
         f->stdin_event_source = sd_event_source_unref(f->stdin_event_source);
         f->stdout_event_source = sd_event_source_unref(f->stdout_event_source);
-
         f->master_event_source = sd_event_source_unref(f->master_event_source);
         f->sigwinch_event_source = sd_event_source_unref(f->sigwinch_event_source);
+        f->exit_event_source = sd_event_source_unref(f->exit_event_source);
         f->event = sd_event_unref(f->event);
 
         if (f->output_fd >= 0) {
@@ -751,11 +750,6 @@ 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;
 }
 
@@ -830,6 +824,33 @@ static int on_sigwinch_event(sd_event_source *e, const struct signalfd_siginfo *
         return 0;
 }
 
+static int on_exit_event(sd_event_source *e, void *userdata) {
+        PTYForward *f = ASSERT_PTR(userdata);
+        int r;
+
+        assert(e);
+        assert(e == f->exit_event_source);
+
+        /* Drain the buffer on exit. */
+
+        if (f->done)
+                return 0;
+
+        for (unsigned trial = 0; trial < 1000; trial++) {
+                if (drained(f))
+                        return pty_forward_done(f, 0);
+
+                r = shovel(f);
+                if (r < 0)
+                        return r;
+                if (f->done)
+                        return 0;
+        }
+
+        /* If we could not drain, then propagate recognizable error code. */
+        return pty_forward_done(f, -ELOOP);
+}
+
 int pty_forward_new(
                 sd_event *event,
                 int master,
@@ -990,6 +1011,12 @@ int pty_forward_new(
 
         (void) sd_event_source_set_description(f->sigwinch_event_source, "ptyfwd-sigwinch");
 
+        r = sd_event_add_exit(f->event, &f->exit_event_source, on_exit_event, f);
+        if (r < 0)
+                return r;
+
+        (void) sd_event_source_set_description(f->exit_event_source, "ptyfwd-exit");
+
         *ret = TAKE_PTR(f);
 
         return 0;
@@ -1036,12 +1063,6 @@ bool pty_forward_get_ignore_vhangup(PTYForward *f) {
         return FLAGS_SET(f->flags, PTY_FORWARD_IGNORE_VHANGUP);
 }
 
-bool pty_forward_is_done(PTYForward *f) {
-        assert(f);
-
-        return f->done;
-}
-
 void pty_forward_set_handler(PTYForward *f, PTYForwardHandler cb, void *userdata) {
         assert(f);
 
@@ -1049,20 +1070,6 @@ 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 aae8b0b57ac..615e1056e88 100644
--- a/src/shared/ptyfwd.h
+++ b/src/shared/ptyfwd.h
@@ -31,12 +31,8 @@ PTYForward* pty_forward_free(PTYForward *f);
 int pty_forward_set_ignore_vhangup(PTYForward *f, bool ignore_vhangup);
 bool pty_forward_get_ignore_vhangup(PTYForward *f);
 
-bool pty_forward_is_done(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);