From fbd84aca4a2cbaa4e7daaa9d1a546d2248be8fec Mon Sep 17 00:00:00 2001 From: "Devin J. Pohly" Date: Mon, 3 Jul 2023 14:51:09 -0500 Subject: [PATCH 1/6] Unify signal handling under wl_event_loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merge our signal handlers into a single function and let Wayland deal with all the struct sigaction stuff. ΔSLOC: -3 --- dwl.c | 83 +++++++++++++++++++++++++---------------------------------- 1 file changed, 35 insertions(+), 48 deletions(-) diff --git a/dwl.c b/dwl.c index 5a1b1b5..b5e146d 100644 --- a/dwl.c +++ b/dwl.c @@ -260,6 +260,7 @@ static void focusmon(const Arg *arg); static void focusstack(const Arg *arg); static Client *focustop(Monitor *m); static void fullscreennotify(struct wl_listener *listener, void *data); +static int handlesig(int signo, void *data); static void incnmaster(const Arg *arg); static void inputdevice(struct wl_listener *listener, void *data); static int keybinding(uint32_t mods, xkb_keysym_t sym); @@ -283,7 +284,6 @@ static void pointerfocus(Client *c, struct wlr_surface *surface, double sx, double sy, uint32_t time); static void printstatus(void); static void quit(const Arg *arg); -static void quitsignal(int signo); static void rendermon(struct wl_listener *listener, void *data); static void requeststartdrag(struct wl_listener *listener, void *data); static void resize(Client *c, struct wlr_box geo, int interact); @@ -297,7 +297,6 @@ static void setmon(Client *c, Monitor *m, uint32_t newtags); static void setpsel(struct wl_listener *listener, void *data); static void setsel(struct wl_listener *listener, void *data); static void setup(void); -static void sigchld(int unused); static void spawn(const Arg *arg); static void startdrag(struct wl_listener *listener, void *data); static void tag(const Arg *arg); @@ -327,6 +326,8 @@ static pid_t child_pid = -1; static int locked; static void *exclusive_focus; static struct wl_display *dpy; +static struct wl_event_loop *eventloop; +static struct wl_event_source *sighandler[4]; static struct wlr_backend *backend; static struct wlr_scene *scene; static struct wlr_scene_tree *layers[NUM_LAYERS]; @@ -652,6 +653,7 @@ checkidleinhibitor(struct wlr_surface *exclude) void cleanup(void) { + int i; #ifdef XWAYLAND wlr_xwayland_destroy(xwayland); #endif @@ -667,6 +669,8 @@ cleanup(void) wlr_cursor_destroy(cursor); wlr_output_layout_destroy(output_layout); wlr_seat_destroy(seat); + for (i = 0; i < LENGTH(sighandler); i++) + wl_event_source_remove(sighandler[i]); wl_display_destroy(dpy); } @@ -820,8 +824,7 @@ createkeyboard(struct wlr_keyboard *keyboard) wlr_seat_set_keyboard(seat, keyboard); - kb->key_repeat_source = wl_event_loop_add_timer( - wl_display_get_event_loop(dpy), keyrepeat, kb); + kb->key_repeat_source = wl_event_loop_add_timer(eventloop, keyrepeat, kb); /* And add the keyboard to our list of keyboards */ wl_list_insert(&keyboards, &kb->link); @@ -1333,6 +1336,28 @@ fullscreennotify(struct wl_listener *listener, void *data) setfullscreen(c, client_wants_fullscreen(c)); } +int +handlesig(int signo, void *data) +{ + if (signo == SIGCHLD) { +#ifdef XWAYLAND + siginfo_t in; + /* wlroots expects to reap the XWayland process itself, so we + * use WNOWAIT to keep the child waitable until we know it's not + * XWayland. + */ + while (!waitid(P_ALL, 0, &in, WEXITED|WNOHANG|WNOWAIT) && in.si_pid + && (!xwayland || in.si_pid != xwayland->server->pid)) + waitpid(in.si_pid, NULL, 0); +#else + while (waitpid(-1, NULL, WNOHANG) > 0); +#endif + } else if (signo == SIGINT || signo == SIGTERM) { + quit(NULL); + } + return 0; +} + void incnmaster(const Arg *arg) { @@ -1875,12 +1900,6 @@ quit(const Arg *arg) wl_display_terminate(dpy); } -void -quitsignal(int signo) -{ - quit(NULL); -} - void rendermon(struct wl_listener *listener, void *data) { @@ -1944,8 +1963,6 @@ run(char *startup_cmd) { /* Add a Unix socket to the Wayland display. */ const char *socket = wl_display_add_socket_auto(dpy); - struct sigaction sa = {.sa_flags = SA_RESTART, .sa_handler = SIG_IGN}; - sigemptyset(&sa.sa_mask); if (!socket) die("startup: display_add_socket_auto"); setenv("WAYLAND_DISPLAY", socket, 1); @@ -1973,8 +1990,6 @@ run(char *startup_cmd) close(piperw[1]); close(piperw[0]); } - /* If nobody is reading the status output, don't terminate */ - sigaction(SIGPIPE, &sa, NULL); printstatus(); /* At this point the outputs are initialized, choose initial selmon based on @@ -2127,20 +2142,14 @@ setsel(struct wl_listener *listener, void *data) void setup(void) { - int layer; - - /* Set up signal handlers */ - struct sigaction sa = {.sa_flags = SA_RESTART, .sa_handler = sigchld}; - sigemptyset(&sa.sa_mask); - sigaction(SIGCHLD, &sa, NULL); - - sa.sa_handler = quitsignal; - sigaction(SIGINT, &sa, NULL); - sigaction(SIGTERM, &sa, NULL); + int i, sig[] = {SIGCHLD, SIGINT, SIGTERM, SIGPIPE}; /* The Wayland display is managed by libwayland. It handles accepting * clients from the Unix socket, manging Wayland globals, and so on. */ dpy = wl_display_create(); + eventloop = wl_display_get_event_loop(dpy); + for (i = 0; i < LENGTH(sighandler); i++) + sighandler[i] = wl_event_loop_add_signal(eventloop, sig[i], handlesig, NULL); /* The backend is a wlroots feature which abstracts the underlying input and * output hardware. The autocreate option will choose the most suitable @@ -2155,8 +2164,8 @@ setup(void) /* Initialize the scene graph used to lay out windows */ scene = wlr_scene_create(); - for (layer = 0; layer < NUM_LAYERS; layer++) - layers[layer] = wlr_scene_tree_create(&scene->tree); + for (i = 0; i < NUM_LAYERS; i++) + layers[i] = wlr_scene_tree_create(&scene->tree); /* Create a renderer with the default implementation */ if (!(drw = wlr_renderer_autocreate(backend))) @@ -2308,28 +2317,6 @@ setup(void) #endif } -void -sigchld(int unused) -{ -#ifdef XWAYLAND - siginfo_t in; - /* We should be able to remove this function in favor of a simple - * struct sigaction sa = {.sa_handler = SIG_IGN}; - * sigaction(SIGCHLD, &sa, NULL); - * but the Xwayland implementation in wlroots currently prevents us from - * setting our own disposition for SIGCHLD. - */ - /* WNOWAIT leaves the child in a waitable state, in case this is the - * XWayland process - */ - while (!waitid(P_ALL, 0, &in, WEXITED|WNOHANG|WNOWAIT) && in.si_pid - && (!xwayland || in.si_pid != xwayland->server->pid)) - waitpid(in.si_pid, NULL, 0); -#else - while (waitpid(-1, NULL, WNOHANG) > 0); -#endif -} - void spawn(const Arg *arg) { From 18415278718bf8e162b0fbf5d3551134d1eb705a Mon Sep 17 00:00:00 2001 From: "Devin J. Pohly" Date: Thu, 13 Jul 2023 16:20:51 -0500 Subject: [PATCH 2/6] properly destroy scene MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ΔSLOC: +1 --- dwl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dwl.c b/dwl.c index b5e146d..76ba33d 100644 --- a/dwl.c +++ b/dwl.c @@ -663,6 +663,7 @@ cleanup(void) waitpid(child_pid, NULL, 0); } wlr_backend_destroy(backend); + wlr_scene_node_destroy(&scene->tree.node); wlr_renderer_destroy(drw); wlr_allocator_destroy(alloc); wlr_xcursor_manager_destroy(cursor_mgr); From 831fc36bc91ac595340300ec5cef5e81f263c3b3 Mon Sep 17 00:00:00 2001 From: "Devin J. Pohly" Date: Thu, 13 Jul 2023 16:22:50 -0500 Subject: [PATCH 3/6] Make drag_icon a persistent scene node MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If there is no current drag icon, this node will be empty, but we now have `drag_icon != NULL` as an invariant. This allows us to eliminate a conditional, since there's no harm in moving an empty node's coordinates around with the pointer. ΔSLOC: -1 --- dwl.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/dwl.c b/dwl.c index 76ba33d..190601e 100644 --- a/dwl.c +++ b/dwl.c @@ -331,6 +331,7 @@ static struct wl_event_source *sighandler[4]; static struct wlr_backend *backend; static struct wlr_scene *scene; static struct wlr_scene_tree *layers[NUM_LAYERS]; +static struct wlr_scene_tree *drag_icon; /* Map from ZWLR_LAYER_SHELL_* constants to Lyr* enum */ static const int layermap[] = { LyrBg, LyrBottom, LyrTop, LyrOverlay }; static struct wlr_renderer *drw; @@ -1663,7 +1664,6 @@ motionnotify(uint32_t time) LayerSurface *l = NULL; int type; struct wlr_surface *surface = NULL; - struct wlr_drag_icon *icon; /* time is 0 in internal calls meant to restore pointer focus. */ if (time) { @@ -1674,10 +1674,9 @@ motionnotify(uint32_t time) selmon = xytomon(cursor->x, cursor->y); } - /* Update drag icon's position if any */ - if (seat->drag && (icon = seat->drag->icon)) - wlr_scene_node_set_position(icon->data, cursor->x + icon->surface->sx, - cursor->y + icon->surface->sy); + /* Update drag icon's position */ + wlr_scene_node_set_position(&drag_icon->node, cursor->x, cursor->y); + /* If we are currently grabbing the mouse, handle and return */ if (cursor_mode == CurMove) { /* Move the grabbed client to the new position. */ @@ -2167,6 +2166,8 @@ setup(void) scene = wlr_scene_create(); for (i = 0; i < NUM_LAYERS; i++) layers[i] = wlr_scene_tree_create(&scene->tree); + drag_icon = wlr_scene_tree_create(&scene->tree); + wlr_scene_node_place_below(&drag_icon->node, &layers[LyrBlock]->node); /* Create a renderer with the default implementation */ if (!(drw = wlr_renderer_autocreate(backend))) @@ -2338,8 +2339,7 @@ startdrag(struct wl_listener *listener, void *data) if (!drag->icon) return; - drag->icon->data = icon = wlr_scene_subsurface_tree_create(&scene->tree, drag->icon->surface); - wlr_scene_node_place_below(&icon->node, &layers[LyrBlock]->node); + drag->icon->data = icon = wlr_scene_subsurface_tree_create(drag_icon, drag->icon->surface); motionnotify(0); wl_signal_add(&drag->icon->events.destroy, &drag_icon_destroy); } From 4b15bbeb33ba5409bc436ca44200b5605a73b344 Mon Sep 17 00:00:00 2001 From: "Devin J. Pohly" Date: Fri, 14 Jul 2023 00:02:39 -0400 Subject: [PATCH 4/6] Remove unused icon variable --- dwl.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/dwl.c b/dwl.c index 190601e..96ffb64 100644 --- a/dwl.c +++ b/dwl.c @@ -2334,12 +2334,10 @@ void startdrag(struct wl_listener *listener, void *data) { struct wlr_drag *drag = data; - struct wlr_scene_tree *icon; - if (!drag->icon) return; - drag->icon->data = icon = wlr_scene_subsurface_tree_create(drag_icon, drag->icon->surface); + drag->icon->data = &wlr_scene_subsurface_tree_create(drag_icon, drag->icon->surface)->node; motionnotify(0); wl_signal_add(&drag->icon->events.destroy, &drag_icon_destroy); } From 76ba2cdab04a952d8cd0503a5f2afc7a18f53538 Mon Sep 17 00:00:00 2001 From: "Devin J. Pohly" Date: Fri, 14 Jul 2023 00:02:54 -0400 Subject: [PATCH 5/6] Remove now-unneeded call to motionnotify This appears to have been here for the side effect of updating the drag icon's position. --- dwl.c | 1 - 1 file changed, 1 deletion(-) diff --git a/dwl.c b/dwl.c index 96ffb64..f323d1b 100644 --- a/dwl.c +++ b/dwl.c @@ -2338,7 +2338,6 @@ startdrag(struct wl_listener *listener, void *data) return; drag->icon->data = &wlr_scene_subsurface_tree_create(drag_icon, drag->icon->surface)->node; - motionnotify(0); wl_signal_add(&drag->icon->events.destroy, &drag_icon_destroy); } From ca4a97b9335296c40f558baa1ead14578b166d70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leonardo=20Hern=C3=A1ndez=20Hern=C3=A1ndez?= Date: Thu, 13 Jul 2023 19:36:26 -0600 Subject: [PATCH 6/6] do not use wl_event_loop for signal handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ΔSLOC: -4 Fixes: https://github.com/djpohly/dwl/issues/456 Fixes: https://github.com/djpohly/dwl/issues/459 --- dwl.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/dwl.c b/dwl.c index f323d1b..93f66ef 100644 --- a/dwl.c +++ b/dwl.c @@ -260,7 +260,7 @@ static void focusmon(const Arg *arg); static void focusstack(const Arg *arg); static Client *focustop(Monitor *m); static void fullscreennotify(struct wl_listener *listener, void *data); -static int handlesig(int signo, void *data); +static void handlesig(int signo); static void incnmaster(const Arg *arg); static void inputdevice(struct wl_listener *listener, void *data); static int keybinding(uint32_t mods, xkb_keysym_t sym); @@ -326,8 +326,6 @@ static pid_t child_pid = -1; static int locked; static void *exclusive_focus; static struct wl_display *dpy; -static struct wl_event_loop *eventloop; -static struct wl_event_source *sighandler[4]; static struct wlr_backend *backend; static struct wlr_scene *scene; static struct wlr_scene_tree *layers[NUM_LAYERS]; @@ -654,7 +652,6 @@ checkidleinhibitor(struct wlr_surface *exclude) void cleanup(void) { - int i; #ifdef XWAYLAND wlr_xwayland_destroy(xwayland); #endif @@ -671,8 +668,6 @@ cleanup(void) wlr_cursor_destroy(cursor); wlr_output_layout_destroy(output_layout); wlr_seat_destroy(seat); - for (i = 0; i < LENGTH(sighandler); i++) - wl_event_source_remove(sighandler[i]); wl_display_destroy(dpy); } @@ -826,7 +821,8 @@ createkeyboard(struct wlr_keyboard *keyboard) wlr_seat_set_keyboard(seat, keyboard); - kb->key_repeat_source = wl_event_loop_add_timer(eventloop, keyrepeat, kb); + kb->key_repeat_source = wl_event_loop_add_timer( + wl_display_get_event_loop(dpy), keyrepeat, kb); /* And add the keyboard to our list of keyboards */ wl_list_insert(&keyboards, &kb->link); @@ -1338,8 +1334,8 @@ fullscreennotify(struct wl_listener *listener, void *data) setfullscreen(c, client_wants_fullscreen(c)); } -int -handlesig(int signo, void *data) +void +handlesig(int signo) { if (signo == SIGCHLD) { #ifdef XWAYLAND @@ -1357,7 +1353,6 @@ handlesig(int signo, void *data) } else if (signo == SIGINT || signo == SIGTERM) { quit(NULL); } - return 0; } void @@ -2143,13 +2138,15 @@ void setup(void) { int i, sig[] = {SIGCHLD, SIGINT, SIGTERM, SIGPIPE}; + struct sigaction sa = {.sa_flags = SA_RESTART, .sa_handler = handlesig}; + sigemptyset(&sa.sa_mask); + + for (i = 0; i < LENGTH(sig); i++) + sigaction(sig[i], &sa, NULL); /* The Wayland display is managed by libwayland. It handles accepting * clients from the Unix socket, manging Wayland globals, and so on. */ dpy = wl_display_create(); - eventloop = wl_display_get_event_loop(dpy); - for (i = 0; i < LENGTH(sighandler); i++) - sighandler[i] = wl_event_loop_add_signal(eventloop, sig[i], handlesig, NULL); /* The backend is a wlroots feature which abstracts the underlying input and * output hardware. The autocreate option will choose the most suitable