diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index 7843b9aa630..39d3f3aed82 100644 --- a/src/machine/machinectl.c +++ b/src/machine/machinectl.c @@ -1188,35 +1188,29 @@ static int bind_mount(int argc, char *argv[], void *userdata) { } static int on_machine_removed(sd_bus_message *m, void *userdata, sd_bus_error *ret_error) { - PTYForward ** forward = (PTYForward**) userdata; + PTYForward *forward = ASSERT_PTR(userdata); int r; assert(m); - assert(forward); - if (*forward) { - /* If the forwarder is already initialized, tell it to - * exit on the next vhangup(), so that we still flush - * out what might be queued and exit then. */ - - r = pty_forward_set_ignore_vhangup(*forward, false); - if (r >= 0) - return 0; + /* Tell the forwarder to exit on the next vhangup(), so that we still flush out what might be queued + * and exit then. */ + r = pty_forward_set_ignore_vhangup(forward, false); + if (r < 0) { + /* On error, quit immediately. */ log_error_errno(r, "Failed to set ignore_vhangup flag: %m"); + (void) sd_event_exit(sd_bus_get_event(sd_bus_message_get_bus(m)), EXIT_FAILURE); } - /* On error, or when the forwarder is not initialized yet, quit immediately */ - sd_event_exit(sd_bus_get_event(sd_bus_message_get_bus(m)), EXIT_FAILURE); return 0; } -static int process_forward(sd_event *event, PTYForward **forward, int master, PTYForwardFlags flags, const char *name) { - char last_char = 0; - bool machine_died; +static int process_forward(sd_event *event, sd_bus_slot *machine_removed_slot, int master, PTYForwardFlags flags, const char *name) { int r; assert(event); + assert(machine_removed_slot); assert(master >= 0); assert(name); @@ -1231,24 +1225,21 @@ static int process_forward(sd_event *event, PTYForward **forward, int master, PT if (r < 0) return log_error_errno(r, "Failed to enable SIGINT/SITERM handling: %m"); - r = pty_forward_new(event, master, flags, forward); + _cleanup_(pty_forward_freep) PTYForward *forward = NULL; + r = pty_forward_new(event, master, flags, &forward); if (r < 0) return log_error_errno(r, "Failed to create PTY forwarder: %m"); + /* No userdata should not set previously. */ + assert_se(!sd_bus_slot_set_userdata(machine_removed_slot, forward)); + r = sd_event_loop(event); if (r < 0) return log_error_errno(r, "Failed to run event loop: %m"); - pty_forward_get_last_char(*forward, &last_char); - - machine_died = + bool machine_died = (flags & PTY_FORWARD_IGNORE_VHANGUP) && - pty_forward_get_ignore_vhangup(*forward) == 0; - - *forward = pty_forward_free(*forward); - - if (last_char != '\n') - fputc('\n', stdout); + pty_forward_get_ignore_vhangup(forward) == 0; if (!arg_quiet) { if (machine_died) @@ -1300,7 +1291,6 @@ static int parse_machine_uid(const char *spec, const char **machine, char **uid) static int login_machine(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - _cleanup_(pty_forward_freep) PTYForward *forward = NULL; _cleanup_(sd_bus_slot_unrefp) sd_bus_slot *slot = NULL; _cleanup_(sd_event_unrefp) sd_event *event = NULL; int master = -1, r; @@ -1334,7 +1324,7 @@ static int login_machine(int argc, char *argv[], void *userdata) { "member='MachineRemoved'," "arg0='", machine, "'"); - r = sd_bus_add_match_async(bus, &slot, match, on_machine_removed, NULL, &forward); + r = sd_bus_add_match_async(bus, &slot, match, on_machine_removed, NULL, NULL); if (r < 0) return log_error_errno(r, "Failed to request machine removal match: %m"); @@ -1346,13 +1336,12 @@ static int login_machine(int argc, char *argv[], void *userdata) { if (r < 0) return bus_log_parse_error(r); - return process_forward(event, &forward, master, PTY_FORWARD_IGNORE_VHANGUP, machine); + return process_forward(event, slot, master, PTY_FORWARD_IGNORE_VHANGUP, machine); } static int shell_machine(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL, *m = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - _cleanup_(pty_forward_freep) PTYForward *forward = NULL; _cleanup_(sd_bus_slot_unrefp) sd_bus_slot *slot = NULL; _cleanup_(sd_event_unrefp) sd_event *event = NULL; int master = -1, r; @@ -1396,7 +1385,7 @@ static int shell_machine(int argc, char *argv[], void *userdata) { "member='MachineRemoved'," "arg0='", machine, "'"); - r = sd_bus_add_match_async(bus, &slot, match, on_machine_removed, NULL, &forward); + r = sd_bus_add_match_async(bus, &slot, match, on_machine_removed, NULL, NULL); if (r < 0) return log_error_errno(r, "Failed to request machine removal match: %m"); @@ -1426,7 +1415,7 @@ static int shell_machine(int argc, char *argv[], void *userdata) { if (r < 0) return bus_log_parse_error(r); - return process_forward(event, &forward, master, 0, machine); + return process_forward(event, slot, master, /* flags = */ 0, machine); } static int normalize_nspawn_filename(const char *name, char **ret_file) { diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index ec3bb40c3da..d465cd58e25 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -5740,16 +5740,6 @@ static int run_container( if (r < 0) return log_error_errno(r, "Failed to run event loop: %m"); - if (forward) { - char last_char = 0; - - (void) pty_forward_get_last_char(forward, &last_char); - forward = pty_forward_free(forward); - - if (!arg_quiet && last_char != '\n') - putc('\n', stdout); - } - /* 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 e989934c741..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); } @@ -2094,21 +2087,8 @@ static int start_transient_service(sd_bus *bus) { if (r < 0) return log_error_errno(r, "Failed to run event loop: %m"); - if (c.forward) { - char last_char = 0; - - r = pty_forward_get_last_char(c.forward, &last_char); - if (r >= 0 && !arg_quiet && last_char != '\n') - fputc('\n', stdout); - } - 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 00450ca6967..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,14 +81,16 @@ struct PTYForward { bool read_from_master:1; bool done:1; - bool drain:1; bool last_char_set:1; char last_char; + char last_char_safe; char in_buffer[LINE_MAX], *out_buffer; size_t out_buffer_size; size_t in_buffer_full, out_buffer_full; + size_t out_buffer_write_len; /* The length of the output in the buffer except for the trailing + * truncated OSC, CSI, or some (but not all) ESC sequence. */ usec_t escape_timestamp; unsigned escape_counter; @@ -114,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) { @@ -133,6 +135,19 @@ static void pty_forward_disconnect(PTYForward *f) { (void) loop_write(f->output_fd, ANSI_WINDOW_TITLE_POP, SIZE_MAX); } + if (f->last_char_set && f->last_char != '\n') { + const char *s; + + if (isatty_safe(f->output_fd) && f->last_char != '\r') + s = "\r\n"; + else + s = "\n"; + (void) loop_write(f->output_fd, s, SIZE_MAX); + + f->last_char_set = true; + f->last_char = '\n'; + } + if (f->close_output_fd) f->output_fd = safe_close(f->output_fd); } @@ -151,6 +166,7 @@ static void pty_forward_disconnect(PTYForward *f) { f->out_buffer = mfree(f->out_buffer); f->out_buffer_size = 0; f->out_buffer_full = 0; + f->out_buffer_write_len = 0; f->in_buffer_full = 0; f->csi_sequence = mfree(f->csi_sequence); @@ -243,7 +259,7 @@ static bool drained(PTYForward *f) { return true; } -static char *background_color_sequence(PTYForward *f) { +static char* background_color_sequence(PTYForward *f) { assert(f); assert(f->background_color); @@ -277,6 +293,9 @@ static int insert_background_color(PTYForward *f, size_t offset) { assert(f); + if (FLAGS_SET(f->flags, PTY_FORWARD_DUMB_TERMINAL)) + return 0; + if (!f->background_color) return 0; @@ -359,6 +378,9 @@ static int is_csi_background_reset_sequence(const char *seq) { static int insert_background_fix(PTYForward *f, size_t offset) { assert(f); + if (FLAGS_SET(f->flags, PTY_FORWARD_DUMB_TERMINAL)) + return 0; + if (!f->background_color) return 0; @@ -391,6 +413,9 @@ bool shall_set_terminal_title(void) { static int insert_window_title_fix(PTYForward *f, size_t offset) { assert(f); + if (FLAGS_SET(f->flags, PTY_FORWARD_DUMB_TERMINAL)) + return 0; + if (!f->title_prefix) return 0; @@ -414,12 +439,6 @@ static int pty_forward_ansi_process(PTYForward *f, size_t offset) { assert(f); assert(offset <= f->out_buffer_full); - if (!f->background_color && !f->title_prefix) - return 0; - - if (FLAGS_SET(f->flags, PTY_FORWARD_DUMB_TERMINAL)) - return 0; - for (size_t i = offset; i < f->out_buffer_full; i++) { char c = f->out_buffer[i]; @@ -433,8 +452,11 @@ static int pty_forward_ansi_process(PTYForward *f, size_t offset) { if (r < 0) return r; i += r; + f->last_char_safe = c; } else if (c == 0x1B) /* ESC */ f->ansi_color_state = ANSI_COLOR_STATE_ESC; + else if (!char_is_cc(c)) + f->last_char_safe = c; break; case ANSI_COLOR_STATE_ESC: @@ -545,6 +567,9 @@ static int pty_forward_ansi_process(PTYForward *f, size_t offset) { default: assert_not_reached(); } + + if (f->ansi_color_state == ANSI_COLOR_STATE_TEXT) + f->out_buffer_write_len = i + 1; } return 0; @@ -579,7 +604,7 @@ static int do_shovel(PTYForward *f) { } if (f->out_buffer) { - f->out_buffer_full = strlen(f->out_buffer); + f->out_buffer_full = f->out_buffer_write_len = strlen(f->out_buffer); f->out_buffer_size = MALLOC_SIZEOF_SAFE(f->out_buffer); } } @@ -679,9 +704,10 @@ static int do_shovel(PTYForward *f) { } } - if (f->stdout_writable && f->out_buffer_full > 0) { + if (f->stdout_writable && f->out_buffer_write_len > 0) { + assert(f->out_buffer_write_len <= f->out_buffer_full); - k = write(f->output_fd, f->out_buffer, f->out_buffer_full); + k = write(f->output_fd, f->out_buffer, f->out_buffer_write_len); if (k < 0) { if (errno == EAGAIN) @@ -695,14 +721,22 @@ static int do_shovel(PTYForward *f) { } else { - if (k > 0) { - f->last_char = f->out_buffer[k-1]; + if (k > 0 && f->last_char_safe != '\0') { + if ((size_t) k == f->out_buffer_write_len) + /* If we wrote all, then save the last safe character. */ + f->last_char = f->last_char_safe; + else + /* If we wrote partially, then tentatively save the last written character. + * Hopefully, we will write more in the next loop. */ + f->last_char = f->out_buffer[k-1]; + f->last_char_set = true; } - assert(f->out_buffer_full >= (size_t) k); + assert(f->out_buffer_write_len >= (size_t) k); memmove(f->out_buffer, f->out_buffer + k, f->out_buffer_full - k); f->out_buffer_full -= k; + f->out_buffer_write_len -= k; } } } @@ -711,16 +745,11 @@ static int do_shovel(PTYForward *f) { /* Exit the loop if any side hung up and if there's * nothing more to write or nothing we could write. */ - if ((f->out_buffer_full <= 0 || f->stdout_hangup) && + if ((f->out_buffer_write_len <= 0 || f->stdout_hangup) && (f->in_buffer_full <= 0 || f->master_hangup)) 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; } @@ -795,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, @@ -955,12 +1011,18 @@ 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; } -PTYForward *pty_forward_free(PTYForward *f) { +PTYForward* pty_forward_free(PTYForward *f) { if (!f) return NULL; @@ -972,17 +1034,6 @@ PTYForward *pty_forward_free(PTYForward *f) { return mfree(f); } -int pty_forward_get_last_char(PTYForward *f, char *ch) { - assert(f); - assert(ch); - - if (!f->last_char_set) - return -ENXIO; - - *ch = f->last_char; - return 0; -} - int pty_forward_set_ignore_vhangup(PTYForward *f, bool b) { int r; @@ -1012,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); @@ -1025,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 b86027e9bc5..615e1056e88 100644 --- a/src/shared/ptyfwd.h +++ b/src/shared/ptyfwd.h @@ -25,20 +25,14 @@ typedef enum PTYForwardFlags { typedef int (*PTYForwardHandler)(PTYForward *f, int rcode, void *userdata); -int pty_forward_new(sd_event *event, int master, PTYForwardFlags flags, PTYForward **f); -PTYForward *pty_forward_free(PTYForward *f); - -int pty_forward_get_last_char(PTYForward *f, char *ch); +int pty_forward_new(sd_event *event, int master, PTYForwardFlags flags, PTYForward **ret); +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);