diff --git a/RELEASE-NOTES-next b/RELEASE-NOTES-next index d5db5322..78984c8a 100644 --- a/RELEASE-NOTES-next +++ b/RELEASE-NOTES-next @@ -42,3 +42,4 @@ working. Please reach out to us in that case! • build: correctly provide auxiliary functions when needed • build: fix issues with parallel build • set _NET_DESKTOP_VIEWPORT after randr changes + • fix a bug with i3-nagbar not starting after it has already started once diff --git a/include/util.h b/include/util.h index f0db03a5..09ad941f 100644 --- a/include/util.h +++ b/include/util.h @@ -143,13 +143,13 @@ char *pango_escape_markup(char *input); void start_nagbar(pid_t *nagbar_pid, char *argv[]); /** - * Kills the i3-nagbar process, if *nagbar_pid != -1. + * Kills the i3-nagbar process, if nagbar_pid != -1. * * If wait_for_it is set (restarting i3), this function will waitpid(), * otherwise, ev is assumed to handle it (reloading). * */ -void kill_nagbar(pid_t *nagbar_pid, bool wait_for_it); +void kill_nagbar(pid_t nagbar_pid, bool wait_for_it); /** * Converts a string into a long using strtol(). diff --git a/src/commands.c b/src/commands.c index e6f6ef6a..3f8c2695 100644 --- a/src/commands.c +++ b/src/commands.c @@ -1615,8 +1615,16 @@ void cmd_exit(I3_CMD) { */ void cmd_reload(I3_CMD) { LOG("reloading\n"); - kill_nagbar(&config_error_nagbar_pid, false); - kill_nagbar(&command_error_nagbar_pid, false); + + kill_nagbar(config_error_nagbar_pid, false); + kill_nagbar(command_error_nagbar_pid, false); + /* start_nagbar() will refuse to start a new process if the passed pid is + * set. This will happen when our child watcher is triggered by libev when + * the loop re-starts. However, config errors might be detected before + * that since we will read the config right now with load_configuration. + * See #4104. */ + config_error_nagbar_pid = command_error_nagbar_pid = -1; + load_configuration(NULL, C_RELOAD); x_set_i3_atoms(); /* Send an IPC event just in case the ws names have changed */ diff --git a/src/main.c b/src/main.c index d9b10ecb..83b3e181 100644 --- a/src/main.c +++ b/src/main.c @@ -182,6 +182,10 @@ static void i3_exit(void) { unlink(config.ipc_socket_path); xcb_disconnect(conn); + /* If a nagbar is active, kill it */ + kill_nagbar(config_error_nagbar_pid, false); + kill_nagbar(command_error_nagbar_pid, false); + /* We need ev >= 4 for the following code. Since it is not *that* important (it * only makes sure that there are no i3-nagbar instances left behind) we still * support old systems with libev 3. */ diff --git a/src/util.c b/src/util.c index 6d8d2ee7..cf7f41c8 100644 --- a/src/util.c +++ b/src/util.c @@ -287,8 +287,8 @@ static char *store_restart_layout(void) { void i3_restart(bool forget_layout) { char *restart_filename = forget_layout ? NULL : store_restart_layout(); - kill_nagbar(&config_error_nagbar_pid, true); - kill_nagbar(&command_error_nagbar_pid, true); + kill_nagbar(config_error_nagbar_pid, true); + kill_nagbar(command_error_nagbar_pid, true); restore_geometry(); @@ -336,30 +336,19 @@ char *pango_escape_markup(char *input) { static void nagbar_exited(EV_P_ ev_child *watcher, int revents) { ev_child_stop(EV_A_ watcher); - if (!WIFEXITED(watcher->rstatus)) { - ELOG("ERROR: i3-nagbar did not exit normally.\n"); - return; - } - int exitcode = WEXITSTATUS(watcher->rstatus); - DLOG("i3-nagbar process exited with status %d\n", exitcode); - if (exitcode == 2) { - ELOG("ERROR: i3-nagbar could not be found. Is it correctly installed on your system?\n"); + if (!WIFEXITED(watcher->rstatus)) { + ELOG("i3-nagbar (%d) did not exit normally. This is not an error if the config was reloaded while a nagbar was active.\n", watcher->pid); + } else if (exitcode != 0) { + ELOG("i3-nagbar (%d) process exited with status %d\n", watcher->pid, exitcode); + } else { + DLOG("i3-nagbar (%d) process exited with status %d\n", watcher->pid, exitcode); } - *((pid_t *)watcher->data) = -1; -} - -/* - * Cleanup handler. Will be called when i3 exits. Kills i3-nagbar with signal - * SIGKILL (9) to make sure there are no left-over i3-nagbar processes. - * - */ -static void nagbar_cleanup(EV_P_ ev_cleanup *watcher, int revent) { - pid_t *nagbar_pid = (pid_t *)watcher->data; - if (*nagbar_pid != -1) { - LOG("Sending SIGKILL (%d) to i3-nagbar with PID %d\n", SIGKILL, *nagbar_pid); - kill(*nagbar_pid, SIGKILL); + pid_t *nagbar_pid = watcher->data; + if (*nagbar_pid == watcher->pid) { + /* Only reset if the watched nagbar is the active nagbar */ + *nagbar_pid = -1; } } @@ -395,27 +384,20 @@ void start_nagbar(pid_t *nagbar_pid, char *argv[]) { ev_child_init(child, &nagbar_exited, *nagbar_pid, 0); child->data = nagbar_pid; ev_child_start(main_loop, child); - - /* install a cleanup watcher (will be called when i3 exits and i3-nagbar is - * still running) */ - ev_cleanup *cleanup = smalloc(sizeof(ev_cleanup)); - ev_cleanup_init(cleanup, nagbar_cleanup); - cleanup->data = nagbar_pid; - ev_cleanup_start(main_loop, cleanup); } /* - * Kills the i3-nagbar process, if *nagbar_pid != -1. + * Kills the i3-nagbar process, if nagbar_pid != -1. * * If wait_for_it is set (restarting i3), this function will waitpid(), * otherwise, ev is assumed to handle it (reloading). * */ -void kill_nagbar(pid_t *nagbar_pid, bool wait_for_it) { - if (*nagbar_pid == -1) +void kill_nagbar(pid_t nagbar_pid, bool wait_for_it) { + if (nagbar_pid == -1) return; - if (kill(*nagbar_pid, SIGTERM) == -1) + if (kill(nagbar_pid, SIGTERM) == -1) warn("kill(configerror_nagbar) failed"); if (!wait_for_it) @@ -425,7 +407,7 @@ void kill_nagbar(pid_t *nagbar_pid, bool wait_for_it) { * exec(), our old pid is no longer watched. So, ev won’t handle SIGCHLD * for us and we would end up with a process. Therefore we * waitpid() here. */ - waitpid(*nagbar_pid, NULL, 0); + waitpid(nagbar_pid, NULL, 0); } /*