Merge pull request #2789 from RyanDwyer/reload-idle

Reload config using idle event

include/sway/config.h

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

sway/commands/bind.c

@@ -30,33 +30,6 @@ void free_sway_binding(struct sway_binding *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.
  * 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);
-	if (strcasestr(binding->command, "reload")) {
-		binding->flags |= BINDING_RELOAD;
-	}
 
 	list_t *split = split_string(argv[0], "+");
 	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
  */
 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;
 	struct cmd_results *results = execute_command(binding->command, NULL, NULL);
 	if (results->status == CMD_SUCCESS) {
-		ipc_event_binding(binding_copy);
+		ipc_event_binding(binding);
 	} else {
 		wlr_log(WLR_DEBUG, "could not run command for binding: %s (%s)",
 			binding->command, results->error);
 	}
 
-	if (binding_copy->flags & BINDING_RELOAD) {
-		free_sway_binding(binding_copy);
-	}
 	free_cmd_results(results);
 }

sway/commands/reload.c

@@ -3,15 +3,12 @@
 #include "sway/commands.h"
 #include "sway/config.h"
 #include "sway/ipc-server.h"
+#include "sway/server.h"
 #include "sway/tree/arrange.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
 	list_t *bar_ids = create_list();
 	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)) {
-		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");
 
 	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);
 
 	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);
 }

sway/commands/seat/attach.c

@@ -23,6 +23,8 @@ struct cmd_results *seat_cmd_attach(int argc, char **argv) {
 	new_attachment->identifier = strdup(argv[0]);
 	list_add(new_config->attachments, new_attachment);
 
+	if (!config->validating) {
 		apply_seat_config(new_config);
+	}
 	return cmd_results_new(CMD_SUCCESS, NULL, NULL);
 }

sway/commands/seat/fallback.c

@@ -27,6 +27,8 @@ struct cmd_results *seat_cmd_fallback(int argc, char **argv) {
 			"Expected 'fallback <true|false>'");
 	}
 
+	if (!config->validating) {
 		apply_seat_config(new_config);
+	}
 	return cmd_results_new(CMD_SUCCESS, NULL, NULL);
 }

sway/config.c

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

sway/input/keyboard.c

@@ -264,31 +264,27 @@ static void handle_keyboard_key(struct wl_listener *listener, void *data) {
 	}
 
 	// Identify and execute active pressed binding
-	struct sway_binding *next_repeat_binding = NULL;
+	struct sway_binding *binding = NULL;
 	if (event->state == WLR_KEY_PRESSED) {
-		struct sway_binding *binding_pressed = NULL;
 		get_active_binding(&keyboard->state_keycodes,
-				config->current_mode->keycode_bindings, &binding_pressed,
+				config->current_mode->keycode_bindings, &binding,
 				code_modifiers, false, input_inhibited);
 		get_active_binding(&keyboard->state_keysyms_translated,
-				config->current_mode->keysym_bindings, &binding_pressed,
+				config->current_mode->keysym_bindings, &binding,
 				translated_modifiers, false, input_inhibited);
 		get_active_binding(&keyboard->state_keysyms_raw,
-				config->current_mode->keysym_bindings, &binding_pressed,
+				config->current_mode->keysym_bindings, &binding,
 				raw_modifiers, false, input_inhibited);
 
-		if (binding_pressed) {
+		if (binding) {
-			if ((binding_pressed->flags & BINDING_RELOAD) == 0) {
+			seat_execute_command(seat, binding);
-				next_repeat_binding = binding_pressed;
-			}
-			seat_execute_command(seat, binding_pressed);
 			handled = true;
 		}
 	}
 
 	// Set up (or clear) keyboard repeat for a pressed binding
-	if (next_repeat_binding && wlr_device->keyboard->repeat_info.delay > 0) {
+	if (binding && wlr_device->keyboard->repeat_info.delay > 0) {
-		keyboard->repeat_binding = next_repeat_binding;
+		keyboard->repeat_binding = binding;
 		if (wl_event_source_timer_update(keyboard->key_repeat_source,
 				wlr_device->keyboard->repeat_info.delay) < 0) {
 			wlr_log(WLR_DEBUG, "failed to set key repeat timer");