ptyfwd: several cleanups (#35663)

This commit is contained in:
Yu Watanabe
2024-12-19 00:35:42 +09:00
committed by GitHub
5 changed files with 108 additions and 124 deletions

View File

@@ -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) {

View File

@@ -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);

View File

@@ -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));

View File

@@ -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;

View File

@@ -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);