Reload config using idle event

This patch makes it so when you run reload, the actual reloading is
deferred to the next time the event loop becomes idle. This avoids
several use-after-frees and removes the workarounds we have to avoid
them.

When you run reload, we validate the config before creating the idle
event. This is so the reload command will still return an error if there
are validation errors. To allow this, load_main_config has been adjusted
so it doesn't apply the config if validating is true rather than
applying it unconditionally.

This also fixes a memory leak in the reload command where if the config
failed to load, the bar_ids list would not be freed.
This commit is contained in:
Ryan Dwyer 2018-10-08 19:28:53 +10:00
parent 89f4ebef06
commit 82423991a8
5 changed files with 34 additions and 62 deletions

View file

@ -35,7 +35,6 @@ enum binding_flags {
BINDING_BORDER=4, // mouse only; trigger on container border BINDING_BORDER=4, // mouse only; trigger on container border
BINDING_CONTENTS=8, // mouse only; trigger on container contents BINDING_CONTENTS=8, // mouse only; trigger on container contents
BINDING_TITLEBAR=16, // mouse only; trigger on container titlebar BINDING_TITLEBAR=16, // mouse only; trigger on container titlebar
BINDING_RELOAD=32, // the binding runs the reload command
}; };
/** /**

View file

@ -30,33 +30,6 @@ void free_sway_binding(struct sway_binding *binding) {
free(binding); free(binding);
} }
static struct sway_binding *sway_binding_dup(struct sway_binding *sb) {
struct sway_binding *new_sb = calloc(1, sizeof(struct sway_binding));
if (!new_sb) {
return NULL;
}
new_sb->type = sb->type;
new_sb->order = sb->order;
new_sb->flags = sb->flags;
new_sb->modifiers = sb->modifiers;
new_sb->command = strdup(sb->command);
new_sb->keys = create_list();
int i;
for (i = 0; i < sb->keys->length; ++i) {
xkb_keysym_t *key = malloc(sizeof(xkb_keysym_t));
if (!key) {
free_sway_binding(new_sb);
return NULL;
}
*key = *(xkb_keysym_t *)sb->keys->items[i];
list_add(new_sb->keys, key);
}
return new_sb;
}
/** /**
* Returns true if the bindings have the same key and modifier combinations. * Returns true if the bindings have the same key and modifier combinations.
* Note that keyboard layout is not considered, so the bindings might actually * Note that keyboard layout is not considered, so the bindings might actually
@ -214,9 +187,6 @@ static struct cmd_results *cmd_bindsym_or_bindcode(int argc, char **argv,
} }
binding->command = join_args(argv + 1, argc - 1); binding->command = join_args(argv + 1, argc - 1);
if (strcasestr(binding->command, "reload")) {
binding->flags |= BINDING_RELOAD;
}
list_t *split = split_string(argv[0], "+"); list_t *split = split_string(argv[0], "+");
for (int i = 0; i < split->length; ++i) { for (int i = 0; i < split->length; ++i) {
@ -306,31 +276,16 @@ struct cmd_results *cmd_bindcode(int argc, char **argv) {
* Execute the command associated to a binding * Execute the command associated to a binding
*/ */
void seat_execute_command(struct sway_seat *seat, struct sway_binding *binding) { void seat_execute_command(struct sway_seat *seat, struct sway_binding *binding) {
wlr_log(WLR_DEBUG, "running command for binding: %s", wlr_log(WLR_DEBUG, "running command for binding: %s", binding->command);
binding->command);
struct sway_binding *binding_copy = binding;
// if this is a reload command we need to make a duplicate of the
// binding since it will be gone after the reload has completed.
if (binding->flags & BINDING_RELOAD) {
binding_copy = sway_binding_dup(binding);
if (!binding_copy) {
wlr_log(WLR_ERROR, "Failed to duplicate binding during reload");
return;
}
}
config->handler_context.seat = seat; config->handler_context.seat = seat;
struct cmd_results *results = execute_command(binding->command, NULL, NULL); struct cmd_results *results = execute_command(binding->command, NULL, NULL);
if (results->status == CMD_SUCCESS) { if (results->status == CMD_SUCCESS) {
ipc_event_binding(binding_copy); ipc_event_binding(binding);
} else { } else {
wlr_log(WLR_DEBUG, "could not run command for binding: %s (%s)", wlr_log(WLR_DEBUG, "could not run command for binding: %s (%s)",
binding->command, results->error); binding->command, results->error);
} }
if (binding_copy->flags & BINDING_RELOAD) {
free_sway_binding(binding_copy);
}
free_cmd_results(results); free_cmd_results(results);
} }

View file

@ -3,15 +3,12 @@
#include "sway/commands.h" #include "sway/commands.h"
#include "sway/config.h" #include "sway/config.h"
#include "sway/ipc-server.h" #include "sway/ipc-server.h"
#include "sway/server.h"
#include "sway/tree/arrange.h" #include "sway/tree/arrange.h"
#include "list.h" #include "list.h"
#include "log.h"
struct cmd_results *cmd_reload(int argc, char **argv) { static void do_reload(void *data) {
struct cmd_results *error = NULL;
if ((error = checkarg(argc, "reload", EXPECTED_EQUAL_TO, 0))) {
return error;
}
// store bar ids to check against new bars for barconfig_update events // store bar ids to check against new bars for barconfig_update events
list_t *bar_ids = create_list(); list_t *bar_ids = create_list();
for (int i = 0; i < config->bars->length; ++i) { for (int i = 0; i < config->bars->length; ++i) {
@ -20,9 +17,12 @@ struct cmd_results *cmd_reload(int argc, char **argv) {
} }
if (!load_main_config(config->current_config_path, true, false)) { if (!load_main_config(config->current_config_path, true, false)) {
return cmd_results_new(CMD_FAILURE, "reload", wlr_log(WLR_ERROR, "Error(s) reloading config");
"Error(s) reloading config."); list_foreach(bar_ids, free);
list_free(bar_ids);
return;
} }
ipc_event_workspace(NULL, NULL, "reload"); ipc_event_workspace(NULL, NULL, "reload");
load_swaybars(); load_swaybars();
@ -37,12 +37,26 @@ struct cmd_results *cmd_reload(int argc, char **argv) {
} }
} }
for (int i = 0; i < bar_ids->length; ++i) { list_foreach(bar_ids, free);
free(bar_ids->items[i]);
}
list_free(bar_ids); list_free(bar_ids);
arrange_root(); arrange_root();
}
struct cmd_results *cmd_reload(int argc, char **argv) {
struct cmd_results *error = NULL;
if ((error = checkarg(argc, "reload", EXPECTED_EQUAL_TO, 0))) {
return error;
}
if (!load_main_config(config->current_config_path, true, true)) {
return cmd_results_new(CMD_FAILURE, "reload",
"Error(s) reloading config.");
}
// The reload command frees a lot of stuff, so to avoid use-after-frees
// we schedule the reload to happen using an idle event.
wl_event_loop_add_idle(server.wl_event_loop, do_reload, NULL);
return cmd_results_new(CMD_SUCCESS, NULL, NULL); return cmd_results_new(CMD_SUCCESS, NULL, NULL);
} }

View file

@ -457,6 +457,12 @@ bool load_main_config(const char *file, bool is_active, bool validating) {
success = success && load_config(path, config, success = success && load_config(path, config,
&config->swaynag_config_errors); &config->swaynag_config_errors);
if (validating) {
free_config(config);
config = old_config;
return success;
}
if (is_active) { if (is_active) {
for (int i = 0; i < config->output_configs->length; i++) { for (int i = 0; i < config->output_configs->length; i++) {
apply_output_config_to_outputs(config->output_configs->items[i]); apply_output_config_to_outputs(config->output_configs->items[i]);

View file

@ -278,9 +278,7 @@ static void handle_keyboard_key(struct wl_listener *listener, void *data) {
raw_modifiers, false, input_inhibited); raw_modifiers, false, input_inhibited);
if (binding_pressed) { if (binding_pressed) {
if ((binding_pressed->flags & BINDING_RELOAD) == 0) { next_repeat_binding = binding_pressed;
next_repeat_binding = binding_pressed;
}
seat_execute_command(seat, binding_pressed); seat_execute_command(seat, binding_pressed);
handled = true; handled = true;
} }