mirror of
https://github.com/systemd/systemd-stable.git
synced 2025-01-11 05:17:44 +03:00
Merge pull request #4973 from poettering/run-race
run: fix race for "systemd-run --wait"
This commit is contained in:
commit
b4a8c5ddb1
@ -2226,11 +2226,16 @@ static int process_signal(sd_event *e, struct signal_data *d, uint32_t events) {
|
||||
}
|
||||
|
||||
static int source_dispatch(sd_event_source *s) {
|
||||
EventSourceType saved_type;
|
||||
int r = 0;
|
||||
|
||||
assert(s);
|
||||
assert(s->pending || s->type == SOURCE_EXIT);
|
||||
|
||||
/* Save the event source type, here, so that we still know it after the event callback which might invalidate
|
||||
* the event. */
|
||||
saved_type = s->type;
|
||||
|
||||
if (s->type != SOURCE_DEFER && s->type != SOURCE_EXIT) {
|
||||
r = source_set_pending(s, false);
|
||||
if (r < 0)
|
||||
@ -2318,7 +2323,7 @@ static int source_dispatch(sd_event_source *s) {
|
||||
|
||||
if (r < 0)
|
||||
log_debug_errno(r, "Event source %s (type %s) returned error, disabling: %m",
|
||||
strna(s->description), event_source_type_to_string(s->type));
|
||||
strna(s->description), event_source_type_to_string(saved_type));
|
||||
|
||||
if (s->n_ref == 0)
|
||||
source_free(s);
|
||||
|
@ -403,6 +403,11 @@ static int parse_argv(int argc, char *argv[]) {
|
||||
return -EINVAL;
|
||||
}
|
||||
|
||||
if (arg_pty && arg_no_block) {
|
||||
log_error("--pty is not compatible with --no-block.");
|
||||
return -EINVAL;
|
||||
}
|
||||
|
||||
if (arg_scope && with_timer()) {
|
||||
log_error("Timer options are not supported in --scope mode.");
|
||||
return -EINVAL;
|
||||
@ -784,21 +789,23 @@ static void run_context_free(RunContext *c) {
|
||||
}
|
||||
|
||||
static void run_context_check_done(RunContext *c) {
|
||||
bool done = true;
|
||||
bool done;
|
||||
|
||||
assert(c);
|
||||
|
||||
if (c->match)
|
||||
done = done && (c->active_state && STR_IN_SET(c->active_state, "inactive", "failed"));
|
||||
done = STRPTR_IN_SET(c->active_state, "inactive", "failed");
|
||||
else
|
||||
done = true;
|
||||
|
||||
if (c->forward)
|
||||
done = done && pty_forward_is_done(c->forward);
|
||||
if (c->forward && done) /* If the service is gone, it's time to drain the output */
|
||||
done = pty_forward_drain(c->forward);
|
||||
|
||||
if (done)
|
||||
sd_event_exit(c->event, EXIT_SUCCESS);
|
||||
}
|
||||
|
||||
static int on_properties_changed(sd_bus_message *m, void *userdata, sd_bus_error *error) {
|
||||
static int run_context_update(RunContext *c, const char *path) {
|
||||
|
||||
static const struct bus_properties_map map[] = {
|
||||
{ "ActiveState", "s", NULL, offsetof(RunContext, active_state) },
|
||||
@ -811,12 +818,11 @@ static int on_properties_changed(sd_bus_message *m, void *userdata, sd_bus_error
|
||||
{}
|
||||
};
|
||||
|
||||
RunContext *c = userdata;
|
||||
int r;
|
||||
|
||||
r = bus_map_all_properties(c->bus,
|
||||
"org.freedesktop.systemd1",
|
||||
sd_bus_message_get_path(m),
|
||||
path,
|
||||
map,
|
||||
c);
|
||||
if (r < 0) {
|
||||
@ -828,6 +834,15 @@ static int on_properties_changed(sd_bus_message *m, void *userdata, sd_bus_error
|
||||
return 0;
|
||||
}
|
||||
|
||||
static int on_properties_changed(sd_bus_message *m, void *userdata, sd_bus_error *error) {
|
||||
RunContext *c = userdata;
|
||||
|
||||
assert(m);
|
||||
assert(c);
|
||||
|
||||
return run_context_update(c, sd_bus_message_get_path(m));
|
||||
}
|
||||
|
||||
static int pty_forward_handler(PTYForward *f, int rcode, void *userdata) {
|
||||
RunContext *c = userdata;
|
||||
|
||||
@ -985,6 +1000,8 @@ static int start_transient_service(
|
||||
|
||||
if (arg_wait || master >= 0) {
|
||||
_cleanup_(run_context_free) RunContext c = {};
|
||||
_cleanup_free_ char *path = NULL;
|
||||
const char *mt;
|
||||
|
||||
c.bus = sd_bus_ref(bus);
|
||||
|
||||
@ -1007,27 +1024,27 @@ static int start_transient_service(
|
||||
pty_forward_set_handler(c.forward, pty_forward_handler, &c);
|
||||
}
|
||||
|
||||
if (arg_wait) {
|
||||
_cleanup_free_ char *path = NULL;
|
||||
const char *mt;
|
||||
|
||||
path = unit_dbus_path_from_name(service);
|
||||
if (!path)
|
||||
return log_oom();
|
||||
path = unit_dbus_path_from_name(service);
|
||||
if (!path)
|
||||
return log_oom();
|
||||
|
||||
mt = strjoina("type='signal',"
|
||||
"sender='org.freedesktop.systemd1',"
|
||||
"path='", path, "',"
|
||||
"interface='org.freedesktop.DBus.Properties',"
|
||||
"member='PropertiesChanged'");
|
||||
r = sd_bus_add_match(bus, &c.match, mt, on_properties_changed, &c);
|
||||
if (r < 0)
|
||||
return log_error_errno(r, "Failed to add properties changed signal.");
|
||||
mt = strjoina("type='signal',"
|
||||
"sender='org.freedesktop.systemd1',"
|
||||
"path='", path, "',"
|
||||
"interface='org.freedesktop.DBus.Properties',"
|
||||
"member='PropertiesChanged'");
|
||||
r = sd_bus_add_match(bus, &c.match, mt, on_properties_changed, &c);
|
||||
if (r < 0)
|
||||
return log_error_errno(r, "Failed to add properties changed signal.");
|
||||
|
||||
r = sd_bus_attach_event(bus, c.event, 0);
|
||||
if (r < 0)
|
||||
return log_error_errno(r, "Failed to attach bus to event loop.");
|
||||
}
|
||||
r = sd_bus_attach_event(bus, c.event, 0);
|
||||
if (r < 0)
|
||||
return log_error_errno(r, "Failed to attach bus to event loop.");
|
||||
|
||||
r = run_context_update(&c, path);
|
||||
if (r < 0)
|
||||
return r;
|
||||
|
||||
r = sd_event_loop(c.event);
|
||||
if (r < 0)
|
||||
@ -1041,7 +1058,13 @@ static int start_transient_service(
|
||||
fputc('\n', stdout);
|
||||
}
|
||||
|
||||
if (!arg_quiet) {
|
||||
if (arg_wait && !arg_quiet) {
|
||||
|
||||
/* Explicitly destroy the PTY forwarder, so that the PTY device is usable again, in 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));
|
||||
|
||||
@ -1416,7 +1439,7 @@ int main(int argc, char* argv[]) {
|
||||
|
||||
/* If --wait is used connect via the bus, unconditionally, as ref/unref is not supported via the limited direct
|
||||
* connection */
|
||||
if (arg_wait)
|
||||
if (arg_wait || arg_pty)
|
||||
r = bus_connect_transport(arg_transport, arg_host, arg_user, &bus);
|
||||
else
|
||||
r = bus_connect_transport_systemd(arg_transport, arg_host, arg_user, &bus);
|
||||
|
@ -69,6 +69,7 @@ struct PTYForward {
|
||||
bool read_from_master:1;
|
||||
|
||||
bool done:1;
|
||||
bool drain:1;
|
||||
|
||||
bool last_char_set:1;
|
||||
char last_char;
|
||||
@ -302,6 +303,11 @@ static int 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 && f->out_buffer_full == 0 && !f->master_readable)
|
||||
return pty_forward_done(f, 0);
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
@ -438,6 +444,9 @@ int pty_forward_new(
|
||||
r = sd_event_add_io(f->event, &f->stdin_event_source, STDIN_FILENO, EPOLLIN|EPOLLET, on_stdin_event, f);
|
||||
if (r < 0 && r != -EPERM)
|
||||
return r;
|
||||
|
||||
if (r >= 0)
|
||||
(void) sd_event_source_set_description(f->stdin_event_source, "ptyfwd-stdin");
|
||||
}
|
||||
|
||||
r = sd_event_add_io(f->event, &f->stdout_event_source, STDOUT_FILENO, EPOLLOUT|EPOLLET, on_stdout_event, f);
|
||||
@ -446,15 +455,21 @@ int pty_forward_new(
|
||||
f->stdout_writable = true;
|
||||
else if (r < 0)
|
||||
return r;
|
||||
else
|
||||
(void) sd_event_source_set_description(f->stdout_event_source, "ptyfwd-stdout");
|
||||
|
||||
r = sd_event_add_io(f->event, &f->master_event_source, master, EPOLLIN|EPOLLOUT|EPOLLET, on_master_event, f);
|
||||
if (r < 0)
|
||||
return r;
|
||||
|
||||
(void) sd_event_source_set_description(f->master_event_source, "ptyfwd-master");
|
||||
|
||||
r = sd_event_add_signal(f->event, &f->sigwinch_event_source, SIGWINCH, on_sigwinch_event, f);
|
||||
if (r < 0)
|
||||
return r;
|
||||
|
||||
(void) sd_event_source_set_description(f->sigwinch_event_source, "ptyfwd-sigwinch");
|
||||
|
||||
*ret = f;
|
||||
f = NULL;
|
||||
|
||||
@ -519,3 +534,18 @@ void pty_forward_set_handler(PTYForward *f, PTYForwardHandler cb, void *userdata
|
||||
f->handler = cb;
|
||||
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 f->out_buffer_full == 0 && !f->master_readable;
|
||||
}
|
||||
|
@ -51,4 +51,6 @@ bool pty_forward_is_done(PTYForward *f);
|
||||
|
||||
void pty_forward_set_handler(PTYForward *f, PTYForwardHandler handler, void *userdata);
|
||||
|
||||
bool pty_forward_drain(PTYForward *f);
|
||||
|
||||
DEFINE_TRIVIAL_CLEANUP_FUNC(PTYForward*, pty_forward_free);
|
||||
|
Loading…
Reference in New Issue
Block a user