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 1f477e682b453598ca6591e1343726a7cfa71331..97a9129ccd7fb00f1b5bee9b2c5ebf2a38ee6074 100644 (file)
--- 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 ee04ef8a2b6fbeaa1501691bd573664d5cac69fb..d465cd58e25f5402ff7ce7b23c659bf2959f5fe9 100644 (file)
--- 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 6a50b40c159e51391b9fe6e9acd8bd6e44a09c50..72f8affc2dde29fb486e2682afd14de538e58aee 100644 (file)
--- 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 401ff8fce58e9e9ed68c066124a414a8fe370e3c..ea365300419172722a234ea810bb049a7cf50dcd 100644 (file)
--- 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 aae8b0b57ac45fcee96294d5cf86d526a271eb85..615e1056e883166db10451e36586a724b28fa5be 100644 (file)
--- 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);