From 445bbe3469b057c8b518f7c787527128f724cc1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 30 Oct 2019 20:02:06 +0100 Subject: [PATCH 01/37] wayland: track multiple terminals The wayland 'term' member is gone and replaced by a list, 'terms'. This list contains all currently running terminal (windows). --- main.c | 12 +++++++----- terminal.c | 2 +- wayland.c | 24 +++++++++++++++++------- wayland.h | 3 +-- 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/main.c b/main.c index b09b5e3b..ebc35a20 100644 --- a/main.c +++ b/main.c @@ -146,20 +146,22 @@ main(int argc, char *const *argv) if ((term = term_init(&conf, fdm, wayl, argc, argv)) == NULL) goto out; - while (true) { - wl_display_flush(term->wl->display); /* TODO: figure out how to get rid of this */ + while (tll_length(wayl->terms) > 0) { + wl_display_flush(wayl->display); /* TODO: figure out how to get rid of this */ if (!fdm_poll(fdm)) break; } - if (term->quit) - ret = EXIT_SUCCESS; + ret = tll_length(wayl->terms) == 0 ? EXIT_SUCCESS : EXIT_FAILURE; out: shm_fini(); - int child_ret = term_destroy(term); + int child_ret = EXIT_SUCCESS; + tll_foreach(wayl->terms, it) + child_ret = term_destroy(it->item); + wayl_destroy(wayl); fdm_destroy(fdm); config_free(conf); diff --git a/terminal.c b/terminal.c index 301319ff..d889cfca 100644 --- a/terminal.c +++ b/terminal.c @@ -472,7 +472,7 @@ term_init(const struct config *conf, struct fdm *fdm, struct wayland *wayl, goto err; } - wayl->term = term; + tll_push_back(wayl->terms, term); return term; err: diff --git a/wayland.c b/wayland.c index 2fcf2340..ded823a5 100644 --- a/wayland.c +++ b/wayland.c @@ -123,8 +123,8 @@ output_scale(void *data, struct wl_output *wl_output, int32_t factor) mon->scale = factor; - struct terminal *term = mon->wayl->term; - if (term != NULL) { + tll_foreach(mon->wayl->terms, it) { + struct terminal *term = it->item; render_resize(term, term->width / term->scale, term->height / term->scale); wayl_reload_cursor_theme(mon->wayl, term); } @@ -394,7 +394,7 @@ fdm_wayl(struct fdm *fdm, int fd, int events, void *data) return false; } - return event_count != -1 && !wayl->term->quit; + return event_count != -1; } static bool @@ -763,14 +763,24 @@ wayl_update_cursor_surface(struct wayland *wayl, struct terminal *term) struct terminal * wayl_terminal_from_surface(struct wayland *wayl, struct wl_surface *surface) { - assert(surface == wayl->term->window->surface); - return wayl->term; + tll_foreach(wayl->terms, it) { + if (it->item->window->surface == surface) + return it->item; + } + + assert(false); + return NULL; } struct terminal * wayl_terminal_from_xdg_toplevel(struct wayland *wayl, struct xdg_toplevel *toplevel) { - assert(toplevel == wayl->term->window->xdg_toplevel); - return wayl->term; + tll_foreach(wayl->terms, it) { + if (it->item->window->xdg_toplevel == toplevel) + return it->item; + } + + assert(false); + return NULL; } diff --git a/wayland.h b/wayland.h index a8aba13c..9ac094f8 100644 --- a/wayland.h +++ b/wayland.h @@ -145,8 +145,7 @@ struct wayland { bool have_argb8888; tll(struct monitor) monitors; /* All available outputs */ - /* TODO: turn into a list to support multiple windows */ - struct terminal *term; + tll(struct terminal *) terms; struct terminal *focused; struct terminal *moused; }; From 9d5926ce12935e51de8c22a93f9f2b1cbfc6c6d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 30 Oct 2019 20:03:11 +0100 Subject: [PATCH 02/37] term: add term_shutdown() This function unmaps the terminal window, removes itself from the wayland list of terminals and then finally destroys itself. We ensure we don't get any callbacks/events referring to a free:d terminal struct, we close all terminal related FDs and unmap the wayland window. Then, to be really really sure there aren't any (by the FDM) queued up events, we delay the self-destruction to the next FDM poll iteration, by opening an event FD and adding it to the FDM. The callback for the event FD removes the FD from the FDM again, and closes it. And then proceeds to destroy the terminal. --- terminal.c | 134 ++++++++++++++++++++++++++++++++++++++++------------- terminal.h | 1 + 2 files changed, 103 insertions(+), 32 deletions(-) diff --git a/terminal.c b/terminal.c index d889cfca..87adae62 100644 --- a/terminal.c +++ b/terminal.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -30,12 +31,8 @@ fdm_ptmx(struct fdm *fdm, int fd, int events, void *data) { struct terminal *term = data; - if (events & EPOLLHUP) { - term->quit = true; - - if (!(events & EPOLLIN)) - return false; - } + if ((events & EPOLLHUP) && !(events & EPOLLIN)) + return term_shutdown(term); assert(events & EPOLLIN); @@ -101,7 +98,10 @@ fdm_ptmx(struct fdm *fdm, int fd, int events, void *data) } } - return !(events & EPOLLHUP); + if (events & EPOLLHUP) + return term_shutdown(term); + + return true; } static bool @@ -494,23 +494,108 @@ close_fds: return NULL; } +static void +close_fd(struct terminal *term, int *fd) +{ + if (*fd == -1) + return; + + fdm_del(term->fdm, *fd); + close(*fd); + *fd = -1; +} + +static void +close_fds(struct terminal *term) +{ + close_fd(term, &term->delayed_render_timer.lower_fd); + close_fd(term, &term->delayed_render_timer.upper_fd); + close_fd(term, &term->blink.fd); + close_fd(term, &term->flash.fd); + close_fd(term, &term->ptmx); +} + +static bool +fdm_shutdown(struct fdm *fdm, int fd, int events, void *data) +{ + LOG_DBG("FDM shutdown"); + struct terminal *term = data; + + fdm_del(term->fdm, fd); + close(fd); + + /* + * Now there shouldn't be any more callbacks or events that can + * trigger, meaning it should be safe to destroy the terminal. + */ + + assert(term->wl->focused != term); + assert(term->wl->moused != term); + + tll_foreach(term->wl->terms, it) { + if (it->item == term) { + tll_remove(term->wl->terms, it); + break; + } + } + + term_destroy(term); + return true; +} + +bool +term_shutdown(struct terminal *term) +{ + /* Ensure we don't get any more events */ + close_fds(term); + + /* Destroy the wayland window, and consume the trail of Wayland events */ + wayl_win_destroy(term->window); + wl_display_roundtrip(term->wl->display); + term->window = NULL; + + /* + * At this point, we no longer receive any more events referring to us, and it _should_ be safe to destroy ourselves now. + * + * However, in the (unlikely) event that there are events already + * queued up by the FDM, we delay the self-destruction to the next + * FDM poll iteration. + * + * This is done by opening an event FD and adding it to the FDM. + */ + + int event_fd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK); + if (event_fd == -1) { + LOG_ERRNO("failed to create terminal shutdown event FD"); + return false; + } + + if (!fdm_add(term->fdm, event_fd, EPOLLIN, &fdm_shutdown, term)) { + LOG_ERR("failed to add terminal shutdown event FD to the FDM"); + close(event_fd); + return false; + } + + if (write(event_fd, &(uint64_t){1}, sizeof(uint64_t)) != sizeof(uint64_t)) { + LOG_ERRNO("failed to send terminal shutdown event"); + fdm_del(term->fdm, event_fd); + close(event_fd); + return false; + } + + return true; +} + int term_destroy(struct terminal *term) { if (term == NULL) return 0; - wayl_win_destroy(term->window); + close_fds(term); - if (term->delayed_render_timer.lower_fd != -1) { - fdm_del(term->fdm, term->delayed_render_timer.lower_fd); - close(term->delayed_render_timer.lower_fd); - } - - if (term->delayed_render_timer.upper_fd != -1) { - fdm_del(term->fdm, term->delayed_render_timer.upper_fd); - close(term->delayed_render_timer.upper_fd); - } + if (term->window != NULL) + wayl_win_destroy(term->window); mtx_lock(&term->render.workers.lock); assert(tll_length(term->render.workers.queue) == 0); @@ -546,21 +631,6 @@ term_destroy(struct terminal *term) free(term->search.buf); - if (term->flash.fd != -1) { - fdm_del(term->fdm, term->flash.fd); - close(term->flash.fd); - } - - if (term->blink.fd != -1) { - fdm_del(term->fdm, term->blink.fd); - close(term->blink.fd); - } - - if (term->ptmx != -1) { - fdm_del(term->fdm, term->ptmx); - close(term->ptmx); - } - for (size_t i = 0; i < term->render.workers.count; i++) { if (term->render.workers.threads[i] != 0) thrd_join(term->render.workers.threads[i], NULL); diff --git a/terminal.h b/terminal.h index b16b9b51..5a0473b3 100644 --- a/terminal.h +++ b/terminal.h @@ -293,6 +293,7 @@ struct config; struct terminal *term_init( const struct config *conf, struct fdm *fdm, struct wayland *wayl, int argc, char *const *argv); +bool term_shutdown(struct terminal *term); int term_destroy(struct terminal *term); void term_reset(struct terminal *term, bool hard); From 2e0888bf3d62627365946688fefbdf90302e63d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 30 Oct 2019 20:05:34 +0100 Subject: [PATCH 03/37] wayland: xdg_toplevel_close(): call term_shutdown() --- wayland.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/wayland.c b/wayland.c index ded823a5..63afc5b0 100644 --- a/wayland.c +++ b/wayland.c @@ -325,9 +325,7 @@ xdg_toplevel_close(void *data, struct xdg_toplevel *xdg_toplevel) struct wayland *wayl = data; struct terminal *term = wayl_terminal_from_xdg_toplevel(wayl, xdg_toplevel); LOG_DBG("xdg-toplevel: close"); - - term->quit = true; - wl_display_roundtrip(wayl->display); + term_shutdown(term); } static const struct xdg_toplevel_listener xdg_toplevel_listener = { From 98ccd01c1b9f273c26466afcfba6f50c57e74cbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 30 Oct 2019 20:20:56 +0100 Subject: [PATCH 04/37] slave: fix debug logging (no 'term' variable) --- slave.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/slave.c b/slave.c index df784a7f..a4ff67a1 100644 --- a/slave.c +++ b/slave.c @@ -107,7 +107,7 @@ slave_spawn(int ptmx, int argc, char *const *argv, default: { close(fork_pipe[1]); /* Close write end */ - LOG_DBG("slave has PID %d", term->slave); + LOG_DBG("slave has PID %d", pid); int _errno; static_assert(sizeof(errno) == sizeof(_errno), "errno size mismatch"); @@ -123,7 +123,7 @@ slave_spawn(int ptmx, int argc, char *const *argv, "%s: failed to execute", argc == 0 ? conf_shell : argv[0]); return -1; } else - LOG_DBG("%s: successfully started", conf->shell); + LOG_DBG("%s: successfully started", conf_shell); break; } } From 54039c1fb44b691abe94fc412696bdc5e14d0a93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 30 Oct 2019 20:21:19 +0100 Subject: [PATCH 05/37] slave: log child's errno, not parents --- slave.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/slave.c b/slave.c index a4ff67a1..0e762bf0 100644 --- a/slave.c +++ b/slave.c @@ -119,8 +119,8 @@ slave_spawn(int ptmx, int argc, char *const *argv, LOG_ERRNO("failed to read from pipe"); return -1; } else if (ret == sizeof(_errno)) { - LOG_ERRNO( - "%s: failed to execute", argc == 0 ? conf_shell : argv[0]); + LOG_ERRNO_P( + "%s: failed to execute", _errno, argc == 0 ? conf_shell : argv[0]); return -1; } else LOG_DBG("%s: successfully started", conf_shell); From 58e31728a0c1c7e4c3b0ecdec12041774dc62b54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 30 Oct 2019 20:22:01 +0100 Subject: [PATCH 06/37] term: term_init(): create wayland window *last* * Configure all FDs *completely* before moving on * Start client before creating the wayland window --- terminal.c | 105 +++++++++++++++++++++++++++++------------------------ 1 file changed, 57 insertions(+), 48 deletions(-) diff --git a/terminal.c b/terminal.c index 87adae62..6699a25b 100644 --- a/terminal.c +++ b/terminal.c @@ -304,7 +304,7 @@ term_init(const struct config *conf, struct fdm *fdm, struct wayland *wayl, int delay_lower_fd = -1; int delay_upper_fd = -1; - struct terminal *term = NULL; + struct terminal *term = malloc(sizeof(*term)); if ((ptmx = posix_openpt(O_RDWR | O_NOCTTY)) == -1) { LOG_ERRNO("failed to open PTY"); @@ -325,8 +325,43 @@ term_init(const struct config *conf, struct fdm *fdm, struct wayland *wayl, goto close_fds; } + /* Read logic requires non-blocking mode */ + { + int fd_flags = fcntl(ptmx, F_GETFL); + if (fd_flags == -1) { + LOG_ERRNO("failed to set non blocking mode on PTY master"); + goto err; + } + + if (fcntl(ptmx, F_SETFL, fd_flags | O_NONBLOCK) == -1) { + LOG_ERRNO("failed to set non blocking mode on PTY master"); + goto err; + } + } + + if (!fdm_add(fdm, ptmx, EPOLLIN, &fdm_ptmx, term)) { + LOG_ERR("failed to add ptmx to FDM"); + goto err; + } + + if (!fdm_add(fdm, flash_fd, EPOLLIN, &fdm_flash, term)) { + LOG_ERR("failed to add flash timer FD to FDM"); + goto err; + } + + if (!fdm_add(fdm, blink_fd, EPOLLIN, &fdm_blink, term)) { + LOG_ERR("failed to add blink tiemr FD to FDM"); + goto err; + } + + if (!fdm_add(fdm, delay_lower_fd, EPOLLIN, &fdm_delayed_render, term) || + !fdm_add(fdm, delay_upper_fd, EPOLLIN, &fdm_delayed_render, term)) + { + LOG_ERR("failed to add delayed rendering timer FDs to FDM"); + goto err; + } + /* Initialize configure-based terminal attributes */ - term = malloc(sizeof(*term)); *term = (struct terminal) { .fdm = fdm, .quit = false, @@ -410,6 +445,10 @@ term_init(const struct config *conf, struct fdm *fdm, struct wayland *wayl, term->cell_height = (int)ceil(term->fextents.height); LOG_INFO("cell width=%d, height=%d", term->cell_width, term->cell_height); + /* Start the slave/client */ + if (slave_spawn(term->ptmx, argc, argv, conf->shell) == -1) + goto err; + /* Initiailze the Wayland window backend */ if ((term->window = wayl_win_init(wayl)) == NULL) goto err; @@ -432,46 +471,6 @@ term_init(const struct config *conf, struct fdm *fdm, struct wayland *wayl, height = max(height, term->cell_height); render_resize(term, width, height); - /* Start the slave/client */ - if (!slave_spawn(term->ptmx, argc, argv, conf->shell)) - goto err; - - /* Read logic requires non-blocking mode */ - { - int fd_flags = fcntl(ptmx, F_GETFL); - if (fd_flags == -1) { - LOG_ERRNO("failed to set non blocking mode on PTY master"); - goto err; - } - - if (fcntl(ptmx, F_SETFL, fd_flags | O_NONBLOCK) == -1) { - LOG_ERRNO("failed to set non blocking mode on PTY master"); - goto err; - } - } - - if (!fdm_add(fdm, ptmx, EPOLLIN, &fdm_ptmx, term)) { - LOG_ERR("failed to add ptmx to FDM"); - goto err; - } - - if (!fdm_add(fdm, flash_fd, EPOLLIN, &fdm_flash, term)) { - LOG_ERR("failed to add flash timer FD to FDM"); - goto err; - } - - if (!fdm_add(fdm, blink_fd, EPOLLIN, &fdm_blink, term)) { - LOG_ERR("failed to add blink tiemr FD to FDM"); - goto err; - } - - if (!fdm_add(fdm, delay_lower_fd, EPOLLIN, &fdm_delayed_render, term) || - !fdm_add(fdm, delay_upper_fd, EPOLLIN, &fdm_delayed_render, term)) - { - LOG_ERR("failed to add delayed rendering timer FDs to FDM"); - goto err; - } - tll_push_back(wayl->terms, term); return term; @@ -480,17 +479,27 @@ err: return NULL; close_fds: - if (ptmx != -1) + if (ptmx != -1) { + fdm_del(fdm, ptmx); close(ptmx); - if (flash_fd != -1) + } + if (flash_fd != -1) { + fdm_del(fdm, flash_fd); close(flash_fd); - if (blink_fd != -1) + } + if (blink_fd != -1) { + fdm_del(fdm, blink_fd); close(blink_fd); - if (delay_lower_fd != -1) + } + if (delay_lower_fd != -1) { + fdm_del(fdm, delay_lower_fd); close(delay_lower_fd); - if (delay_upper_fd != -1) + } + if (delay_upper_fd != -1) { + fdm_del(fdm, delay_upper_fd); close(delay_upper_fd); - assert(term == NULL); + } + free(term); return NULL; } From c7238ef7f3c6f182414c7456d3d4ed0faf2ee041 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 30 Oct 2019 20:24:54 +0100 Subject: [PATCH 07/37] term: assign client PID to term->slave --- terminal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terminal.c b/terminal.c index 6699a25b..7f3b3023 100644 --- a/terminal.c +++ b/terminal.c @@ -446,7 +446,7 @@ term_init(const struct config *conf, struct fdm *fdm, struct wayland *wayl, LOG_INFO("cell width=%d, height=%d", term->cell_width, term->cell_height); /* Start the slave/client */ - if (slave_spawn(term->ptmx, argc, argv, conf->shell) == -1) + if ((term->slave = slave_spawn(term->ptmx, argc, argv, conf->shell)) == -1) goto err; /* Initiailze the Wayland window backend */ From 883354ffb1856846ea002e570c1eccdfb54c09c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 30 Oct 2019 20:25:16 +0100 Subject: [PATCH 08/37] wayland: wayl_win_destroy(): return early if win == NULL --- wayland.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/wayland.c b/wayland.c index 63afc5b0..bc813f2e 100644 --- a/wayland.c +++ b/wayland.c @@ -684,6 +684,9 @@ out: void wayl_win_destroy(struct wl_window *win) { + if (win == NULL) + return; + tll_free(win->on_outputs); if (win->search_sub_surface != NULL) wl_subsurface_destroy(win->search_sub_surface); From 1ed78ab44303b5123a8c6acb6b624da84d0268f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 30 Oct 2019 20:25:31 +0100 Subject: [PATCH 09/37] wayland: temporary: track last terminal's exit value --- wayland.h | 1 + 1 file changed, 1 insertion(+) diff --git a/wayland.h b/wayland.h index 9ac094f8..d0a8332f 100644 --- a/wayland.h +++ b/wayland.h @@ -148,6 +148,7 @@ struct wayland { tll(struct terminal *) terms; struct terminal *focused; struct terminal *moused; + int last_exit_value; /* TODO: exit value from client(s)? */ }; struct wayland *wayl_init(struct fdm *fdm); From 644585a3e5e2ca678a7ad6ce74c8629196ae7d62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 30 Oct 2019 20:25:45 +0100 Subject: [PATCH 10/37] term: term_shutdown(): set exit value in wayland --- terminal.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/terminal.c b/terminal.c index 7f3b3023..04c39937 100644 --- a/terminal.c +++ b/terminal.c @@ -529,6 +529,7 @@ fdm_shutdown(struct fdm *fdm, int fd, int events, void *data) { LOG_DBG("FDM shutdown"); struct terminal *term = data; + struct wayland *wayl = term->wl; fdm_del(term->fdm, fd); close(fd); @@ -538,17 +539,18 @@ fdm_shutdown(struct fdm *fdm, int fd, int events, void *data) * trigger, meaning it should be safe to destroy the terminal. */ - assert(term->wl->focused != term); - assert(term->wl->moused != term); + assert(wayl->focused != term); + assert(wayl->moused != term); - tll_foreach(term->wl->terms, it) { + tll_foreach(wayl->terms, it) { if (it->item == term) { - tll_remove(term->wl->terms, it); + tll_remove(wayl->terms, it); break; } } - term_destroy(term); + wayl->last_exit_value = term_destroy(term); + LOG_WARN("last-exit-value: %d", wayl->last_exit_value); return true; } From b7546abca9828fb9017b801ed00a1d4f97044a2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 30 Oct 2019 20:26:08 +0100 Subject: [PATCH 11/37] main: get exit value from wayland struct --- main.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/main.c b/main.c index ebc35a20..9a843b12 100644 --- a/main.c +++ b/main.c @@ -158,9 +158,10 @@ main(int argc, char *const *argv) out: shm_fini(); - int child_ret = EXIT_SUCCESS; tll_foreach(wayl->terms, it) - child_ret = term_destroy(it->item); + term_destroy(it->item); + + int child_ret = wayl->last_exit_value; wayl_destroy(wayl); fdm_destroy(fdm); From 4ae22b72a1d565fea23f9f9022503b655bad2353 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 30 Oct 2019 20:28:21 +0100 Subject: [PATCH 12/37] term: fdm_shutdown(): remove debug output --- terminal.c | 1 - 1 file changed, 1 deletion(-) diff --git a/terminal.c b/terminal.c index 04c39937..f32586ff 100644 --- a/terminal.c +++ b/terminal.c @@ -550,7 +550,6 @@ fdm_shutdown(struct fdm *fdm, int fd, int events, void *data) } wayl->last_exit_value = term_destroy(term); - LOG_WARN("last-exit-value: %d", wayl->last_exit_value); return true; } From 80c4721e578ae20f6c5a6b7959fde909582eeb85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 19:51:33 +0100 Subject: [PATCH 13/37] fdm: don't free FD data that may be referenced by epoll return data It is perfectly possible, and legal, for a FDM handler to delete another handler. The problem however is when the epoll returned array of FD events contain the removed FD handler *after* the handler that removed it. That is, if the epoll returned array is: [FD=13, FD=37] and the handler for FD=13 removes FD=37, then given the current implementation, the FD user data (our handler callback etc) will point to a free:d address. Add support for this situation by deferring FD handler removal *when called from another handler*. This is done by "locking" the FDM struct before looping the handlers for FDs with events (and unlocking afterwards). In fdm_del(), we check if the FDM has been locked, in which case the FD is marked as deleted, and put on a deferred list. But not *actually* deleted. Meanwhile, in the FDM poll function, we skip calling handlers for marked-for-deletion FDs. Then, when all FDs have been processed, we loop the deferred list and finally deletes the FDs for real. --- fdm.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++-------- fdm.h | 1 + 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/fdm.c b/fdm.c index 9523ab5f..e26d394c 100644 --- a/fdm.c +++ b/fdm.c @@ -1,6 +1,7 @@ #include "fdm.h" #include +#include #include #include #include @@ -15,11 +16,14 @@ struct fd { int fd; void *data; fdm_handler_t handler; + bool deleted; }; struct fdm { int epoll_fd; - tll(struct fd) fds; + bool is_polling; + tll(struct fd *) fds; + tll(struct fd *) deferred_delete; }; struct fdm * @@ -34,7 +38,9 @@ fdm_init(void) struct fdm *fdm = malloc(sizeof(*fdm)); *fdm = (struct fdm){ .epoll_fd = epoll_fd, + .is_polling = false, .fds = tll_init(), + .deferred_delete = tll_init(), }; return fdm; } @@ -60,23 +66,31 @@ fdm_add(struct fdm *fdm, int fd, int events, fdm_handler_t handler, void *data) { #if defined(_DEBUG) tll_foreach(fdm->fds, it) { - if (it->item.fd == fd) { + if (it->item->fd == fd) { LOG_ERR("FD=%d already registered", fd); return false; } } #endif - tll_push_back( - fdm->fds, ((struct fd){.fd = fd, .data = data, .handler = handler})); + struct fd *fd_data = malloc(sizeof(*fd_data)); + *fd_data = (struct fd) { + .fd = fd, + .data = data, + .handler = handler, + .deleted = false, + }; + + tll_push_back(fdm->fds, fd_data); struct epoll_event ev = { .events = events, - .data = {.ptr = &tll_back(fdm->fds)}, + .data = {.ptr = fd_data}, }; if (epoll_ctl(fdm->epoll_fd, EPOLL_CTL_ADD, fd, &ev) < 0) { LOG_ERRNO("failed to register FD with epoll"); + free(fd_data); tll_pop_back(fdm->fds); return false; } @@ -84,14 +98,26 @@ fdm_add(struct fdm *fdm, int fd, int events, fdm_handler_t handler, void *data) return true; } -bool -fdm_del(struct fdm *fdm, int fd) +static bool +fdm_del_internal(struct fdm *fdm, int fd, bool close_fd) { + if (fd == -1) + return true; + tll_foreach(fdm->fds, it) { - if (it->item.fd == fd) { + if (it->item->fd == fd) { if (epoll_ctl(fdm->epoll_fd, EPOLL_CTL_DEL, fd, NULL) < 0) LOG_ERRNO("failed to unregister FD=%d from epoll", fd); + if (close_fd) + close(it->item->fd); + + it->item->deleted = true; + if (fdm->is_polling) + tll_push_back(fdm->deferred_delete, it->item); + else + free(it->item); + tll_remove(fdm->fds, it); return true; } @@ -101,6 +127,18 @@ fdm_del(struct fdm *fdm, int fd) return false; } +bool +fdm_del(struct fdm *fdm, int fd) +{ + return fdm_del_internal(fdm, fd, true); +} + +bool +fdm_del_no_close(struct fdm *fdm, int fd) +{ + return fdm_del_internal(fdm, fd, false); +} + bool fdm_poll(struct fdm *fdm) { @@ -114,10 +152,22 @@ fdm_poll(struct fdm *fdm) return false; } + fdm->is_polling = true; for (int i = 0; i < ret; i++) { struct fd *fd = events[i].data.ptr; - if (!fd->handler(fdm, fd->fd, events[i].events, fd->data)) + if (fd->deleted) + continue; + + if (!fd->handler(fdm, fd->fd, events[i].events, fd->data)) { + fdm->is_polling = false; return false; + } + } + fdm->is_polling = false; + + tll_foreach(fdm->deferred_delete, it) { + free(it->item); + tll_remove(fdm->deferred_delete, it); } return true; diff --git a/fdm.h b/fdm.h index cd0fee2a..fb12bffb 100644 --- a/fdm.h +++ b/fdm.h @@ -11,5 +11,6 @@ void fdm_destroy(struct fdm *fdm); bool fdm_add(struct fdm *fdm, int fd, int events, fdm_handler_t handler, void *data); bool fdm_del(struct fdm *fdm, int fd); +bool fdm_del_no_close(struct fdm *fdm, int fd); bool fdm_poll(struct fdm *fdm); From b793919aba8042fa8c3c43fa71d5f38015a5c121 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 19:59:39 +0100 Subject: [PATCH 14/37] wayland: fdm_del() now closes the FD --- wayland.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/wayland.c b/wayland.c index bc813f2e..5b2776fd 100644 --- a/wayland.c +++ b/wayland.c @@ -556,11 +556,10 @@ wayl_destroy(struct wayland *wayl) if (wayl == NULL) return; - if (wayl->kbd.repeat.fd != 0) { - fdm_del(wayl->fdm, wayl->kbd.repeat.fd); - close(wayl->kbd.repeat.fd); } + fdm_del(wayl->fdm, wayl->kbd.repeat.fd); + tll_foreach(wayl->monitors, it) { free(it->item.name); if (it->item.xdg != NULL) @@ -628,7 +627,7 @@ wayl_destroy(struct wayland *wayl) if (wayl->registry != NULL) wl_registry_destroy(wayl->registry); if (wayl->display != NULL) { - fdm_del(wayl->fdm, wl_display_get_fd(wayl->display)); + fdm_del_no_close(wayl->fdm, wl_display_get_fd(wayl->display)); wl_display_disconnect(wayl->display); } From 291a928a49fbf1da3553aec0730ddd987a3f969a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 20:01:36 +0100 Subject: [PATCH 15/37] render: call wl_display_flush() after rendering This allows us to remove that call from the main event loop. --- main.c | 1 - render.c | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/main.c b/main.c index 9a843b12..b9e0e5f7 100644 --- a/main.c +++ b/main.c @@ -147,7 +147,6 @@ main(int argc, char *const *argv) goto out; while (tll_length(wayl->terms) > 0) { - wl_display_flush(wayl->display); /* TODO: figure out how to get rid of this */ if (!fdm_poll(fdm)) break; diff --git a/render.c b/render.c index 29f2ebf0..b58eb7dc 100644 --- a/render.c +++ b/render.c @@ -693,6 +693,7 @@ grid_render(struct terminal *term) if (all_clean) { buf->busy = false; + wl_display_flush(term->wl->display); return; } @@ -727,6 +728,7 @@ grid_render(struct terminal *term) LOG_INFO("frame rendered in %lds %ldus", render_time.tv_sec, render_time.tv_usec); #endif + wl_display_flush(term->wl->display); } static void From 66b20972753ff705a0b081a02a969f7eb42b1c97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 20:02:02 +0100 Subject: [PATCH 16/37] wayland: wayl_init(): call wl_display_roundtrip() when done --- wayland.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/wayland.c b/wayland.c index 5b2776fd..91c3ac28 100644 --- a/wayland.c +++ b/wayland.c @@ -514,6 +514,9 @@ wayl_init(struct fdm *fdm) } wayl->kbd.repeat.fd = timerfd_create(CLOCK_BOOTTIME, TFD_CLOEXEC | TFD_NONBLOCK); + /* All wayland initialization done - make it so */ + wl_display_roundtrip(wayl->display); + if (wayl->kbd.repeat.fd == -1) { LOG_ERRNO("failed to create keyboard repeat timer FD"); goto out; @@ -540,8 +543,6 @@ wayl_init(struct fdm *fdm) goto out; } - //wl_display_dispatch_pending(wayl->display); - //wl_display_flush(wayl->display); return wayl; out: From 95b7c405d464b0eb04141cbbfe077371f5a9d47c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 20:19:53 +0100 Subject: [PATCH 17/37] wayland: wayl_win_destroy(): unmap windows before destroying This will trigger e.d. keyboard_leave() and wl_pointer_leave() events, which ensures there aren't any references to the destroyed window from the global wayland struct. Call wl_display_roundtrip() to trigger those events *before* we destroy the window. --- wayland.c | 21 +++++++++++++++++++++ wayland.h | 2 ++ 2 files changed, 23 insertions(+) diff --git a/wayland.c b/wayland.c index 91c3ac28..6a1d0fae 100644 --- a/wayland.c +++ b/wayland.c @@ -639,6 +639,7 @@ struct wl_window * wayl_win_init(struct wayland *wayl) { struct wl_window *win = calloc(1, sizeof(*win)); + win->wayl = wayl; win->surface = wl_compositor_create_surface(wayl->compositor); if (win->surface == NULL) { @@ -687,6 +688,25 @@ wayl_win_destroy(struct wl_window *win) if (win == NULL) return; + /* + * First, unmap all surfaces to trigger things like + * keyboard_leave() and wl_pointer_leave(). + * + * This ensures we remove all references to *this* window from the + * global wayland struct (since it no longer has neither keyboard + * nor mouse focus). + */ + + /* Scrollback search */ + wl_surface_attach(win->search_surface, NULL, 0, 0); + wl_surface_commit(win->search_surface); + wl_display_roundtrip(win->wayl->display); + + /* Main window */ + wl_surface_attach(win->surface, NULL, 0, 0); + wl_surface_commit(win->surface); + wl_display_roundtrip(win->wayl->display); + tll_free(win->on_outputs); if (win->search_sub_surface != NULL) wl_subsurface_destroy(win->search_sub_surface); @@ -702,6 +722,7 @@ wayl_win_destroy(struct wl_window *win) xdg_surface_destroy(win->xdg_surface); if (win->surface != NULL) wl_surface_destroy(win->surface); + free(win); } diff --git a/wayland.h b/wayland.h index d0a8332f..7e7bfd4c 100644 --- a/wayland.h +++ b/wayland.h @@ -72,7 +72,9 @@ struct wl_primary { uint32_t serial; }; +struct wayland; struct wl_window { + struct wayland *wayl; struct wl_surface *surface; struct xdg_surface *xdg_surface; struct xdg_toplevel *xdg_toplevel; From 2e78dcc5e5cd2d5a2ae6a1f136467fc99a6dfd57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 20:24:13 +0100 Subject: [PATCH 18/37] Don't use non-blocking FDs We use epoll() to determine when we can read/write FDs so there's absolutely no need for non-blocking. --- terminal.c | 24 +++++------------------- wayland.c | 15 +-------------- 2 files changed, 6 insertions(+), 33 deletions(-) diff --git a/terminal.c b/terminal.c index f32586ff..5dc75d7a 100644 --- a/terminal.c +++ b/terminal.c @@ -310,35 +310,21 @@ term_init(const struct config *conf, struct fdm *fdm, struct wayland *wayl, LOG_ERRNO("failed to open PTY"); goto close_fds; } - if ((flash_fd = timerfd_create(CLOCK_BOOTTIME, TFD_CLOEXEC | TFD_NONBLOCK)) == -1) { + if ((flash_fd = timerfd_create(CLOCK_BOOTTIME, TFD_CLOEXEC)) == -1) { LOG_ERRNO("failed to create flash timer FD"); goto close_fds; } - if ((blink_fd = timerfd_create(CLOCK_BOOTTIME, TFD_CLOEXEC | TFD_NONBLOCK)) == -1) { + if ((blink_fd = timerfd_create(CLOCK_BOOTTIME, TFD_CLOEXEC)) == -1) { LOG_ERRNO("failed to create blink timer FD"); goto close_fds; } - if ((delay_lower_fd = timerfd_create(CLOCK_BOOTTIME, TFD_CLOEXEC | TFD_NONBLOCK)) == -1 || - (delay_upper_fd = timerfd_create(CLOCK_BOOTTIME, TFD_CLOEXEC | TFD_NONBLOCK)) == -1) + if ((delay_lower_fd = timerfd_create(CLOCK_BOOTTIME, TFD_CLOEXEC)) == -1 || + (delay_upper_fd = timerfd_create(CLOCK_BOOTTIME, TFD_CLOEXEC)) == -1) { LOG_ERRNO("failed to create delayed rendering timer FDs"); goto close_fds; } - /* Read logic requires non-blocking mode */ - { - int fd_flags = fcntl(ptmx, F_GETFL); - if (fd_flags == -1) { - LOG_ERRNO("failed to set non blocking mode on PTY master"); - goto err; - } - - if (fcntl(ptmx, F_SETFL, fd_flags | O_NONBLOCK) == -1) { - LOG_ERRNO("failed to set non blocking mode on PTY master"); - goto err; - } - } - if (!fdm_add(fdm, ptmx, EPOLLIN, &fdm_ptmx, term)) { LOG_ERR("failed to add ptmx to FDM"); goto err; @@ -574,7 +560,7 @@ term_shutdown(struct terminal *term) * This is done by opening an event FD and adding it to the FDM. */ - int event_fd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK); + int event_fd = eventfd(0, EFD_CLOEXEC); if (event_fd == -1) { LOG_ERRNO("failed to create terminal shutdown event FD"); return false; diff --git a/wayland.c b/wayland.c index 6a1d0fae..21ab1359 100644 --- a/wayland.c +++ b/wayland.c @@ -407,9 +407,6 @@ fdm_repeat(struct fdm *fdm, int fd, int events, void *data) wayl->kbd.repeat.fd, &expiration_count, sizeof(expiration_count)); if (ret < 0) { - if (errno == EAGAIN) - return true; - LOG_ERRNO("failed to read repeat key from repeat timer fd"); return false; } @@ -513,26 +510,16 @@ wayl_init(struct fdm *fdm) goto out; } - wayl->kbd.repeat.fd = timerfd_create(CLOCK_BOOTTIME, TFD_CLOEXEC | TFD_NONBLOCK); /* All wayland initialization done - make it so */ wl_display_roundtrip(wayl->display); + wayl->kbd.repeat.fd = timerfd_create(CLOCK_BOOTTIME, TFD_CLOEXEC); if (wayl->kbd.repeat.fd == -1) { LOG_ERRNO("failed to create keyboard repeat timer FD"); goto out; } int wl_fd = wl_display_get_fd(wayl->display); - int fd_flags = fcntl(wl_fd, F_GETFL); - if (fd_flags == -1) { - LOG_ERRNO("failed to set non blocking mode on Wayland display connection"); - goto out; - } - if (fcntl(wl_fd, F_SETFL, fd_flags | O_NONBLOCK) == -1) { - LOG_ERRNO("failed to set non blocking mode on Wayland display connection"); - goto out; - } - if (!fdm_add(fdm, wl_fd, EPOLLIN, &fdm_wayl, wayl)) { LOG_ERR("failed to register Wayland connection with the FDM"); goto out; From bc815a33dbe998a108e395634c308f47a6780276 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 20:25:08 +0100 Subject: [PATCH 19/37] wayland: wayl_destroy(): destroy any remaining terminals --- wayland.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/wayland.c b/wayland.c index 21ab1359..97917e77 100644 --- a/wayland.c +++ b/wayland.c @@ -544,8 +544,17 @@ wayl_destroy(struct wayland *wayl) if (wayl == NULL) return; + tll_foreach(wayl->terms, it) { + static bool have_warned = false; + if (!have_warned) { + have_warned = true; + LOG_WARN("there are terminals still running"); + term_destroy(it->item); + } } + tll_free(wayl->terms); + fdm_del(wayl->fdm, wayl->kbd.repeat.fd); tll_foreach(wayl->monitors, it) { From dac1ba18c8e97e2a726a83e1cddc1c8f2a40468d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 20:25:44 +0100 Subject: [PATCH 20/37] render: limit length of title in call to xdg_toplevel_set_title() Trying to pass a too long title (not sure what "too long" is though...) will trigger a call to abort() inside the wayland-client library. --- render.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/render.c b/render.c index b58eb7dc..2386e073 100644 --- a/render.c +++ b/render.c @@ -950,9 +950,21 @@ render_resize(struct terminal *term, int width, int height) } void -render_set_title(struct terminal *term, const char *title) +render_set_title(struct terminal *term, const char *_title) { + /* TODO: figure out what the limit actually is */ + static const size_t max_len = 100; + + const char *title = _title; + char *copy = NULL; + + if (strlen(title) > max_len) { + copy = strndup(_title, max_len); + title = copy; + } + xdg_toplevel_set_title(term->window->xdg_toplevel, title); + free(copy); } void From 2286bcf23d3d36f1cfbaa5c66b02ab38542ee628 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 20:27:45 +0100 Subject: [PATCH 21/37] terminal: no need to check for EAGAIN - we don't use non-blocking --- terminal.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/terminal.c b/terminal.c index 5dc75d7a..29487bdf 100644 --- a/terminal.c +++ b/terminal.c @@ -40,9 +40,6 @@ fdm_ptmx(struct fdm *fdm, int fd, int events, void *data) ssize_t count = read(term->ptmx, buf, sizeof(buf)); if (count < 0) { - if (errno == EAGAIN) - return true; - LOG_ERRNO("failed to read from pseudo terminal"); return false; } @@ -116,9 +113,6 @@ fdm_flash(struct fdm *fdm, int fd, int events, void *data) term->flash.fd, &expiration_count, sizeof(expiration_count)); if (ret < 0) { - if (errno == EAGAIN) - return true; - LOG_ERRNO("failed to read flash timer"); return false; } @@ -144,9 +138,6 @@ fdm_blink(struct fdm *fdm, int fd, int events, void *data) term->blink.fd, &expiration_count, sizeof(expiration_count)); if (ret < 0) { - if (errno == EAGAIN) - return true; - LOG_ERRNO("failed to read blink timer"); return false; } @@ -192,17 +183,18 @@ fdm_delayed_render(struct fdm *fdm, int fd, int events, void *data) if (fd == term->delayed_render_timer.upper_fd) ret2 = read(term->delayed_render_timer.upper_fd, &unused, sizeof(unused)); - if ((ret1 < 0 || ret2 < 0) && errno != EAGAIN) + if ((ret1 < 0 || ret2 < 0)) { LOG_ERRNO("failed to read timeout timer"); - else if (ret1 > 0 || ret2 > 0) { - render_refresh(term); + return false; + } - /* Reset timers */ - term->delayed_render_timer.is_armed = false; - timerfd_settime(term->delayed_render_timer.lower_fd, 0, &(struct itimerspec){.it_value = {0}}, NULL); - timerfd_settime(term->delayed_render_timer.upper_fd, 0, &(struct itimerspec){.it_value = {0}}, NULL); - } else - assert(false); + render_refresh(term); + + /* Reset timers */ + struct itimerspec reset = {{0}}; + timerfd_settime(term->delayed_render_timer.lower_fd, 0, &reset, NULL); + timerfd_settime(term->delayed_render_timer.upper_fd, 0, &reset, NULL); + term->delayed_render_timer.is_armed = false; return true; } From fb0801fa5632f6a683320baa53d1f395efc0b231 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 20:28:11 +0100 Subject: [PATCH 22/37] terminal: delayed rendering: fdm_delayed_render() may be called with is_armged==false This would happen if *both* timers triggered in the same epoll() call. --- terminal.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/terminal.c b/terminal.c index 29487bdf..7e7d3b53 100644 --- a/terminal.c +++ b/terminal.c @@ -172,7 +172,8 @@ fdm_delayed_render(struct fdm *fdm, int fd, int events, void *data) return false; struct terminal *term = data; - assert(term->delayed_render_timer.is_armed); + if (!term->delayed_render_timer.is_armed) + return true; uint64_t unused; ssize_t ret1 = 0; From c824aa2ef57e5e16f9e56ac75d1161c03605b62d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 20:29:16 +0100 Subject: [PATCH 23/37] terminal: fdm_del() closes the FD --- terminal.c | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/terminal.c b/terminal.c index 7e7d3b53..3e4ecf12 100644 --- a/terminal.c +++ b/terminal.c @@ -458,26 +458,12 @@ err: return NULL; close_fds: - if (ptmx != -1) { - fdm_del(fdm, ptmx); - close(ptmx); - } - if (flash_fd != -1) { - fdm_del(fdm, flash_fd); - close(flash_fd); - } - if (blink_fd != -1) { - fdm_del(fdm, blink_fd); - close(blink_fd); - } - if (delay_lower_fd != -1) { - fdm_del(fdm, delay_lower_fd); - close(delay_lower_fd); - } - if (delay_upper_fd != -1) { - fdm_del(fdm, delay_upper_fd); - close(delay_upper_fd); - } + fdm_del(fdm, ptmx); + fdm_del(fdm, flash_fd); + fdm_del(fdm, blink_fd); + fdm_del(fdm, delay_lower_fd); + fdm_del(fdm, delay_upper_fd); + free(term); return NULL; } From 70b236d66d76abb82948f807cb9926278d380dac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 20:30:58 +0100 Subject: [PATCH 24/37] term_shutdown(): cleanup * Close FDs, then delay destruction to next epoll() call * Destroy window before destroying terminal --- terminal.c | 84 +++++++++++++++++++++++------------------------------- 1 file changed, 35 insertions(+), 49 deletions(-) diff --git a/terminal.c b/terminal.c index 3e4ecf12..e0c76708 100644 --- a/terminal.c +++ b/terminal.c @@ -468,77 +468,53 @@ close_fds: return NULL; } -static void -close_fd(struct terminal *term, int *fd) -{ - if (*fd == -1) - return; - - fdm_del(term->fdm, *fd); - close(*fd); - *fd = -1; -} - -static void -close_fds(struct terminal *term) -{ - close_fd(term, &term->delayed_render_timer.lower_fd); - close_fd(term, &term->delayed_render_timer.upper_fd); - close_fd(term, &term->blink.fd); - close_fd(term, &term->flash.fd); - close_fd(term, &term->ptmx); -} - static bool fdm_shutdown(struct fdm *fdm, int fd, int events, void *data) { LOG_DBG("FDM shutdown"); struct terminal *term = data; - struct wayland *wayl = term->wl; + /* Kill the event FD */ fdm_del(term->fdm, fd); - close(fd); - /* - * Now there shouldn't be any more callbacks or events that can - * trigger, meaning it should be safe to destroy the terminal. - */ + wayl_win_destroy(term->window); + term->window = NULL; + struct wayland *wayl __attribute__((unused)) = term->wl; assert(wayl->focused != term); assert(wayl->moused != term); - tll_foreach(wayl->terms, it) { - if (it->item == term) { - tll_remove(wayl->terms, it); - break; - } - } - wayl->last_exit_value = term_destroy(term); + int exit_code = term_destroy(term); + return true; } bool term_shutdown(struct terminal *term) { - /* Ensure we don't get any more events */ - close_fds(term); + if (term->is_shutting_down) + return true; - /* Destroy the wayland window, and consume the trail of Wayland events */ - wayl_win_destroy(term->window); - wl_display_roundtrip(term->wl->display); - term->window = NULL; + term->is_shutting_down = true; /* - * At this point, we no longer receive any more events referring to us, and it _should_ be safe to destroy ourselves now. - * - * However, in the (unlikely) event that there are events already - * queued up by the FDM, we delay the self-destruction to the next - * FDM poll iteration. - * - * This is done by opening an event FD and adding it to the FDM. + * Close FDs then postpone self-destruction to the next poll + * iteration, by creating an event FD that we trigger immediately. */ + fdm_del(term->fdm, term->delayed_render_timer.lower_fd); + fdm_del(term->fdm, term->delayed_render_timer.upper_fd); + fdm_del(term->fdm, term->blink.fd); + fdm_del(term->fdm, term->flash.fd); + fdm_del(term->fdm, term->ptmx); + + term->delayed_render_timer.lower_fd = -1; + term->delayed_render_timer.upper_fd = -1; + term->blink.fd = -1; + term->flash.fd = -1; + term->ptmx = -1; + int event_fd = eventfd(0, EFD_CLOEXEC); if (event_fd == -1) { LOG_ERRNO("failed to create terminal shutdown event FD"); @@ -554,7 +530,6 @@ term_shutdown(struct terminal *term) if (write(event_fd, &(uint64_t){1}, sizeof(uint64_t)) != sizeof(uint64_t)) { LOG_ERRNO("failed to send terminal shutdown event"); fdm_del(term->fdm, event_fd); - close(event_fd); return false; } @@ -567,7 +542,18 @@ term_destroy(struct terminal *term) if (term == NULL) return 0; - close_fds(term); + tll_foreach(term->wl->terms, it) { + if (it->item == term) { + tll_remove(term->wl->terms, it); + break; + } + } + + fdm_del(term->fdm, term->delayed_render_timer.lower_fd); + fdm_del(term->fdm, term->delayed_render_timer.upper_fd); + fdm_del(term->fdm, term->blink.fd); + fdm_del(term->fdm, term->flash.fd); + fdm_del(term->fdm, term->ptmx); if (term->window != NULL) wayl_win_destroy(term->window); From 1e41a25f00aab1038486cff37e286fd5c729a72b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 20:34:32 +0100 Subject: [PATCH 25/37] terminal: call user-defined callback when destroying terminal main uses this to get the exit code of the terminal. --- main.c | 33 +++++++++++++++++++++++---------- terminal.c | 9 ++++++++- terminal.h | 8 +++++++- wayland.h | 1 - 4 files changed, 38 insertions(+), 13 deletions(-) diff --git a/main.c b/main.c index b9e0e5f7..606b0199 100644 --- a/main.c +++ b/main.c @@ -26,14 +26,27 @@ static void print_usage(const char *prog_name) { - printf("Usage: %s [OPTION]...\n", prog_name); + printf("Usage: %s [OPTIONS]...\n", prog_name); printf("\n"); printf("Options:\n"); printf(" -f,--font=FONT comma separated list of fonts in fontconfig format (monospace)\n" " -t,--term=TERM value to set the environment variable TERM to (foot)\n" " -g,--geometry=WIDTHxHEIGHT set initial width and height\n" + " -s,--server run as a server\n" " -v,--version show the version number and quit\n"); - printf("\n"); +} + +struct shutdown_context { + struct terminal **term; + int exit_code; +}; + +static void +term_shutdown_cb(void *data, int exit_code) +{ + struct shutdown_context *ctx = data; + *ctx->term = NULL; + ctx->exit_code = exit_code; } int @@ -136,6 +149,7 @@ main(int argc, char *const *argv) struct fdm *fdm = NULL; struct wayland *wayl = NULL; struct terminal *term = NULL; + struct shutdown_context shutdown_ctx = {.term = &term, .exit_code = EXIT_FAILURE}; if ((fdm = fdm_init()) == NULL) goto out; @@ -143,7 +157,10 @@ main(int argc, char *const *argv) if ((wayl = wayl_init(fdm)) == NULL) goto out; - if ((term = term_init(&conf, fdm, wayl, argc, argv)) == NULL) + if (!as_server && (term = term_init(&conf, fdm, wayl, argc, argv, + &term_shutdown_cb, &shutdown_ctx)) == NULL) + goto out; + goto out; while (tll_length(wayl->terms) > 0) { @@ -157,15 +174,11 @@ main(int argc, char *const *argv) out: shm_fini(); - tll_foreach(wayl->terms, it) - term_destroy(it->item); - - int child_ret = wayl->last_exit_value; - + server_destroy(server); + term_destroy(term); wayl_destroy(wayl); fdm_destroy(fdm); config_free(conf); - return ret == EXIT_SUCCESS ? child_ret : ret; - + return ret == EXIT_SUCCESS && !as_server ? shutdown_ctx.exit_code : ret; } diff --git a/terminal.c b/terminal.c index e0c76708..cbe67b58 100644 --- a/terminal.c +++ b/terminal.c @@ -289,7 +289,8 @@ initialize_fonts(struct terminal *term, const struct config *conf) struct terminal * term_init(const struct config *conf, struct fdm *fdm, struct wayland *wayl, - int argc, char *const *argv) + int argc, char *const *argv, + void (*shutdown_cb)(void *data, int exit_code), void *shutdown_data) { int ptmx = -1; int flash_fd = -1; @@ -411,6 +412,8 @@ term_init(const struct config *conf, struct fdm *fdm, struct wayland *wayl, .lower_fd = delay_lower_fd, .upper_fd = delay_upper_fd, }, + .shutdown_cb = shutdown_cb, + .shutdown_data = shutdown_data, }; initialize_color_cube(term); @@ -484,8 +487,12 @@ fdm_shutdown(struct fdm *fdm, int fd, int events, void *data) assert(wayl->focused != term); assert(wayl->moused != term); + void (*cb)(void *, int) = term->shutdown_cb; + void *cb_data = term->shutdown_data; int exit_code = term_destroy(term); + if (cb != NULL) + cb(cb_data, exit_code); return true; } diff --git a/terminal.h b/terminal.h index 5a0473b3..3ec2802c 100644 --- a/terminal.h +++ b/terminal.h @@ -287,12 +287,18 @@ struct terminal { int lower_fd; int upper_fd; } delayed_render_timer; + + bool is_shutting_down; + void (*shutdown_cb)(void *data, int exit_code); + void *shutdown_data; }; struct config; struct terminal *term_init( const struct config *conf, struct fdm *fdm, struct wayland *wayl, - int argc, char *const *argv); + int argc, char *const *argv, + void (*shutdown_cb)(void *data, int exit_code), void *shutdown_data); + bool term_shutdown(struct terminal *term); int term_destroy(struct terminal *term); diff --git a/wayland.h b/wayland.h index 7e7bfd4c..951a0ece 100644 --- a/wayland.h +++ b/wayland.h @@ -150,7 +150,6 @@ struct wayland { tll(struct terminal *) terms; struct terminal *focused; struct terminal *moused; - int last_exit_value; /* TODO: exit value from client(s)? */ }; struct wayland *wayl_init(struct fdm *fdm); From 32c6ed70699e341256428ffafd214aab1a6f52a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 20:35:42 +0100 Subject: [PATCH 26/37] main: register signal handlers for SIGINT and SIGTERM --- main.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/main.c b/main.c index 606b0199..fa69459b 100644 --- a/main.c +++ b/main.c @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -23,6 +24,14 @@ #define min(x, y) ((x) < (y) ? (x) : (y)) #define max(x, y) ((x) > (y) ? (x) : (y)) +static volatile sig_atomic_t aborted = 0; + +static void +sig_handler(int signo) +{ + aborted = 1; +} + static void print_usage(const char *prog_name) { @@ -164,6 +173,12 @@ main(int argc, char *const *argv) goto out; while (tll_length(wayl->terms) > 0) { + const struct sigaction sa = {.sa_handler = &sig_handler}; + if (sigaction(SIGINT, &sa, NULL) < 0 || sigaction(SIGTERM, &sa, NULL) < 0) { + LOG_ERRNO("failed to register signal handlers"); + goto out; + } + if (!fdm_poll(fdm)) break; From a1efd65746cb0c464fac987e076ec3632e632445 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 20:37:22 +0100 Subject: [PATCH 27/37] server: implement a --server mode In this mode, foot listens on a UNIX socket and creates terminal windows when clients connect. A connecting client sends argc/argv to the server, and the server instantiates a new terminal window. When the terminal window is closed, the exit code is sent back to the client. --- main.c | 16 ++- meson.build | 1 + server.c | 310 ++++++++++++++++++++++++++++++++++++++++++++++++++++ server.h | 9 ++ 4 files changed, 334 insertions(+), 2 deletions(-) create mode 100644 server.c create mode 100644 server.h diff --git a/main.c b/main.c index fa69459b..f13a45d8 100644 --- a/main.c +++ b/main.c @@ -17,6 +17,7 @@ #include "config.h" #include "fdm.h" #include "font.h" +#include "server.h" #include "shm.h" #include "terminal.h" #include "version.h" @@ -77,11 +78,14 @@ main(int argc, char *const *argv) {"term", required_argument, 0, 't'}, {"font", required_argument, 0, 'f'}, {"geometry", required_argument, 0, 'g'}, + {"server", no_argument, 0, 's'}, {"version", no_argument, 0, 'v'}, {"help", no_argument, 0, 'h'}, {NULL, no_argument, 0, 0}, }; + bool as_server = false; + while (true) { int c = getopt_long(argc, argv, ":t:f:g:vh", longopts, NULL); if (c == -1) @@ -127,6 +131,10 @@ main(int argc, char *const *argv) break; } + case 's': + as_server = true; + break; + case 'v': printf("foot version %s\n", FOOT_VERSION); config_free(conf); @@ -158,6 +166,7 @@ main(int argc, char *const *argv) struct fdm *fdm = NULL; struct wayland *wayl = NULL; struct terminal *term = NULL; + struct server *server = NULL; struct shutdown_context shutdown_ctx = {.term = &term, .exit_code = EXIT_FAILURE}; if ((fdm = fdm_init()) == NULL) @@ -170,21 +179,24 @@ main(int argc, char *const *argv) &term_shutdown_cb, &shutdown_ctx)) == NULL) goto out; + if (as_server && (server = server_init(&conf, fdm, wayl)) == NULL) goto out; - while (tll_length(wayl->terms) > 0) { const struct sigaction sa = {.sa_handler = &sig_handler}; if (sigaction(SIGINT, &sa, NULL) < 0 || sigaction(SIGTERM, &sa, NULL) < 0) { LOG_ERRNO("failed to register signal handlers"); goto out; } + if (as_server) + LOG_INFO("running as server; launch terminals by running footclient"); + while (!aborted && (as_server || tll_length(wayl->terms) > 0)) { if (!fdm_poll(fdm)) break; } - ret = tll_length(wayl->terms) == 0 ? EXIT_SUCCESS : EXIT_FAILURE; + ret = aborted || tll_length(wayl->terms) == 0 ? EXIT_SUCCESS : EXIT_FAILURE; out: shm_fini(); diff --git a/meson.build b/meson.build index da91ff91..bb7ecef3 100644 --- a/meson.build +++ b/meson.build @@ -78,6 +78,7 @@ executable( 'render.c', 'render.h', 'search.c', 'search.h', 'selection.c', 'selection.h', + 'server.c', 'server.h', 'shm.c', 'shm.h', 'slave.c', 'slave.h', 'terminal.c', 'terminal.h', diff --git a/server.c b/server.c new file mode 100644 index 00000000..ae6d678e --- /dev/null +++ b/server.c @@ -0,0 +1,310 @@ +#include "server.h" + +#include + +#include +#include +#include +#include + +#include + +#define LOG_MODULE "server" +#define LOG_ENABLE_DBG 1 +#include "log.h" +#include "terminal.h" +#include "tllist.h" + +struct client; +struct server { + const struct config *conf; + struct fdm *fdm; + struct wayland *wayl; + + int fd; + char *sock_path; + + tll(struct client *) clients; +}; + +struct client { + struct server *server; + int fd; + + struct terminal *term; + int argc; + char **argv; +}; + +static void +client_destroy(struct client *client) +{ + if (client == NULL) + return; + + if (client->term != NULL) { + LOG_WARN("client FD=%d: terminal still alive", client->fd); + term_destroy(client->term); + } + + if (client->argv != NULL) { + for (int i = 0; i < client->argc; i++) + free(client->argv[i]); + free(client->argv); + } + + if (client->fd != -1) { + LOG_DBG("client FD=%d: disconnected", client->fd); + fdm_del(client->server->fdm, client->fd); + } + + tll_foreach(client->server->clients, it) { + if (it->item == client) { + tll_remove(client->server->clients, it); + break; + } + } + + free(client); +} + +static void +client_send_exit_code(struct client *client, int exit_code) +{ + if (client->fd == -1) + return; + + if (write(client->fd, &exit_code, sizeof(exit_code)) != sizeof(exit_code)) + LOG_ERRNO("failed to write slave exit code to client"); +} + +static void +term_shutdown_handler(void *data, int exit_code) +{ + struct client *client = data; + + client_send_exit_code(client, exit_code); + + client->term = NULL; + client_destroy(client); +} + +static bool +fdm_client(struct fdm *fdm, int fd, int events, void *data) +{ + struct client *client = data; + struct server *server = client->server; + + if (events & EPOLLHUP) + goto shutdown; + + assert(events & EPOLLIN); + + if (recv(fd, &client->argc, sizeof(client->argc), 0) != sizeof(client->argc)) + goto shutdown; + + client->argv = calloc(client->argc + 1, sizeof(client->argv[0])); + for (int i = 0; i < client->argc; i++) { + uint16_t len; + if (recv(fd, &len, sizeof(len), 0) != sizeof(len)) + goto shutdown; + + client->argv[i] = malloc(len + 1); + client->argv[i][len] = '\0'; + if (recv(fd, client->argv[i], len, 0) != len) + goto shutdown; + } + + assert(client->term == NULL); + client->term = term_init( + server->conf, server->fdm, server->wayl, client->argc, client->argv, + &term_shutdown_handler, client); + + if (client->term == NULL) { + LOG_ERR("failed to instantiate new terminal"); + goto shutdown; + } + + return true; + +shutdown: + LOG_DBG("client FD=%d: disconnected", client->fd); + + fdm_del(fdm, fd); + client->fd = -1; + + if (client->term != NULL && !client->term->is_shutting_down) + term_shutdown(client->term); + else + client_destroy(client); + + return true; +} + +static bool +fdm_server(struct fdm *fdm, int fd, int events, void *data) +{ + if (events & EPOLLHUP) + return false; + + struct server *server = data; + + struct sockaddr_un addr; + socklen_t addr_size = sizeof(addr); + int client_fd = accept4( + server->fd, (struct sockaddr *)&addr, &addr_size, SOCK_CLOEXEC); + + if (client_fd == -1) { + LOG_ERRNO("failed to accept client connection"); + return false; + } + + struct client *client = malloc(sizeof(*client)); + *client = (struct client) { + .server = server, + .fd = client_fd, + }; + + if (!fdm_add(server->fdm, client_fd, EPOLLIN, &fdm_client, client)) { + LOG_ERR("client FD=%d: failed to add client to FDM", client_fd); + close(client_fd); + free(client); + return false; + } + + LOG_DBG("client FD=%d: connected", client_fd); + tll_push_back(server->clients, client); + return true; +} + +static char * +get_socket_path(void) +{ + const char *xdg_runtime = getenv("XDG_RUNTIME_DIR"); + if (xdg_runtime == NULL) + return strdup("/tmp/foot.sock"); + + char *path = malloc(strlen(xdg_runtime) + 1 + strlen("foot.sock") + 1); + sprintf(path, "%s/foot.sock", xdg_runtime); + return path; +} + +enum connect_status {CONNECT_ERR, CONNECT_FAIL, CONNECT_SUCCESS}; + +static enum connect_status +try_connect(const char *sock_path) +{ + enum connect_status ret = CONNECT_ERR; + + int fd = socket(AF_UNIX, SOCK_STREAM, 0); + if (fd == -1) { + LOG_ERRNO("failed to create UNIX socket"); + goto err; + } + + struct sockaddr_un addr = {.sun_family = AF_UNIX}; + strncpy(addr.sun_path, sock_path, sizeof(addr.sun_path) - 1); + + ret = connect(fd, (const struct sockaddr *)&addr, sizeof(addr)) < 0 + ? CONNECT_FAIL : CONNECT_SUCCESS; + +err: + if (fd != -1) + close(fd); + return ret; +} + +struct server * +server_init(const struct config *conf, struct fdm *fdm, struct wayland *wayl) +{ + int fd = socket(AF_UNIX, SOCK_STREAM, 0); + if (fd == -1) { + LOG_ERRNO("failed to create UNIX socket"); + return NULL; + } + + struct server *server = NULL; + char *sock_path = NULL; + + if ((sock_path = get_socket_path()) == NULL) + goto err; + + switch (try_connect(sock_path)) { + case CONNECT_FAIL: + break; + + case CONNECT_SUCCESS: + LOG_ERR("foot --server already running"); + /* FALLTHROUGH */ + + case CONNECT_ERR: + goto err; + } + + unlink(sock_path); + + if (fcntl(fd, F_SETFL, fcntl(fd, F_GETFL) | FD_CLOEXEC) < 0) { + LOG_ERRNO("failed to set FD_CLOEXEC on socket"); + goto err; + } + + struct sockaddr_un addr = {.sun_family = AF_UNIX}; + strncpy(addr.sun_path, sock_path, sizeof(addr.sun_path) - 1); + + if (bind(fd, (const struct sockaddr *)&addr, sizeof(addr)) < 0) { + LOG_ERRNO("%s: failed to bind", addr.sun_path); + goto err; + } + + if (listen(fd, 0) < 0) { + LOG_ERRNO("%s: failed to listen", addr.sun_path); + goto err; + } + + server = malloc(sizeof(*server)); + *server = (struct server) { + .conf = conf, + .fdm = fdm, + .wayl = wayl, + + .fd = fd, + .sock_path = sock_path, + + .clients = tll_init(), + }; + + if (!fdm_add(fdm, fd, EPOLLIN, &fdm_server, server)) { + LOG_ERR("failed to add server FD to the FDM"); + goto err; + } + + return server; + +err: + free(server); + free(sock_path); + if (fd != -1) + close(fd); + return NULL; +} + +void +server_destroy(struct server *server) +{ + if (server == NULL) + return; + + LOG_DBG("server destroy, %zu clients still alive", + tll_length(server->clients)); + + tll_foreach(server->clients, it) { + client_send_exit_code(it->item, 1); + client_destroy(it->item); + } + + tll_free(server->clients); + + fdm_del(server->fdm, server->fd); + free(server->sock_path); + free(server); +} diff --git a/server.h b/server.h new file mode 100644 index 00000000..a5f34071 --- /dev/null +++ b/server.h @@ -0,0 +1,9 @@ +#pragma once + +#include "fdm.h" +#include "config.h" +#include "wayland.h" + +struct server; +struct server *server_init(const struct config *conf, struct fdm *fdm, struct wayland *wayl); +void server_destroy(struct server *server); From 6637c8aeda76a46a953267b082aada7557eab49c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 20:39:34 +0100 Subject: [PATCH 28/37] client: a standalone binary that connects to a foot --server --- client.c | 130 ++++++++++++++++++++++++++++++++++++ completions/meson.build | 1 + completions/zsh/_footclient | 6 ++ meson.build | 6 ++ 4 files changed, 143 insertions(+) create mode 100644 client.c create mode 100644 completions/zsh/_footclient diff --git a/client.c b/client.c new file mode 100644 index 00000000..bcd3fa6b --- /dev/null +++ b/client.c @@ -0,0 +1,130 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +#define LOG_MODULE "foot-client" +#include "log.h" +#include "version.h" + +static volatile sig_atomic_t aborted = 0; + +static void +sig_handler(int signo) +{ + aborted = 1; +} + +static void +print_usage(const char *prog_name) +{ + printf("Usage: %s [OPTIONS]...\n", prog_name); + printf("\n"); + printf("Options:\n"); + printf(" -v,--version show the version number and quit\n"); +} + +int +main(int argc, char *const *argv) +{ + int ret = EXIT_FAILURE; + + const char *const prog_name = argv[0]; + + while (true) { + int c = getopt_long(argc, argv, ":hv", NULL, NULL); + if (c == -1) + break; + + switch (c) { + case 'v': + printf("footclient version %s\n", FOOT_VERSION); + return EXIT_SUCCESS; + + case 'h': + print_usage(prog_name); + return EXIT_SUCCESS; + + case ':': + fprintf(stderr, "error: -%c: missing required argument\n", optopt); + return EXIT_FAILURE; + + case '?': + fprintf(stderr, "error: -%c: invalid option\n", optopt); + return EXIT_FAILURE; + } + } + + argc -= optind; + argv += optind; + + int fd = socket(AF_UNIX, SOCK_STREAM, 0); + if (fd == -1) { + LOG_ERRNO("failed to create socket"); + goto err; + } + + bool connected = false; + struct sockaddr_un addr = {.sun_family = AF_UNIX}; + + const char *xdg_runtime = getenv("XDG_RUNTIME_DIR"); + if (xdg_runtime != NULL) { + snprintf(addr.sun_path, sizeof(addr.sun_path), "%s/foot.sock", xdg_runtime); + + if (connect(fd, (const struct sockaddr *)&addr, sizeof(addr)) == 0) + connected = true; + } + + if (!connected) { + strncpy(addr.sun_path, "/tmp/foot.sock", sizeof(addr.sun_path) - 1); + if (connect(fd, (const struct sockaddr *)&addr, sizeof(addr)) < 0) { + LOG_ERRNO("failed to connect (is 'foot --server' running?)"); + goto err; + } + } + + if (send(fd, &argc, sizeof(argc), 0) != sizeof(argc)) { + LOG_ERRNO("failed to send argc/argv to server"); + goto err; + } + + for (int i = 0; i < argc; i++) { + uint16_t len = strlen(argv[i]); + + if (send(fd, &len, sizeof(len), 0) != sizeof(len) || + send(fd, argv[i], len, 0) != sizeof(len)) + { + LOG_ERRNO("failed to send argc/argv to server"); + goto err; + } + } + + if (sigaction(SIGINT, &(struct sigaction){.sa_handler = &sig_handler}, NULL) < 0) { + LOG_ERRNO("failed to register signal handlers"); + goto err; + } + + int exit_code; + ssize_t rcvd = recv(fd, &exit_code, sizeof(exit_code), 0); + + if (rcvd == -1 && errno == EINTR) + assert(aborted); + else if (rcvd != sizeof(exit_code)) + LOG_ERRNO("failed to read server response"); + else + ret = exit_code; + +err: + if (fd != -1) + close(fd); + return ret; +} diff --git a/completions/meson.build b/completions/meson.build index 5ab3dded..bf844d89 100644 --- a/completions/meson.build +++ b/completions/meson.build @@ -1,2 +1,3 @@ zsh_install_dir = join_paths(get_option('datadir'), 'zsh/site-functions') install_data('zsh/_foot', install_dir: zsh_install_dir) +install_data('zsh/_footclient', install_dir: zsh_install_dir) diff --git a/completions/zsh/_footclient b/completions/zsh/_footclient new file mode 100644 index 00000000..6ab98fd0 --- /dev/null +++ b/completions/zsh/_footclient @@ -0,0 +1,6 @@ +#compdef footclient + +_arguments \ + -s \ + '(-v --version)'{-v,--version}'[show the version number and quit]' \ + '(-h --help)'{-h,--help}'[show help message and quit]' diff --git a/meson.build b/meson.build index bb7ecef3..8fed58bb 100644 --- a/meson.build +++ b/meson.build @@ -90,6 +90,12 @@ executable( dependencies: [threads, math, freetype, fontconfig, pixman, wayland_client, wayland_cursor, xkb], install: true) +executable( + 'footclient', + 'client.c', + 'log.c', 'log.h', + install: true) + custom_target( 'terminfo', output: 'f', From 3032ac33da61a2e51b4da743eca62a98cf79886f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 20:40:10 +0100 Subject: [PATCH 29/37] completions: zsh: add --server and --term to completions --- completions/zsh/_foot | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/completions/zsh/_foot b/completions/zsh/_foot index cbdc7032..cbb1237c 100644 --- a/completions/zsh/_foot +++ b/completions/zsh/_foot @@ -2,13 +2,19 @@ _arguments \ -s \ - '(-v --version)'{-v,--version}'[show the version number and quit]' \ - '(-h --help)'{-h,--help}'[show help message and quit]' \ '(-f --font)'{-f,--font}'[font name and style in fontconfig format (monospace)]:font:->fonts' \ - '(-g --geometry)'{-g,--geometry}'[window WIDTHxHEIGHT, in pixels (84x24 cells)]' + '(-t,--term)'{-t,--term}'[value to set the environment variable TERM to (foot)]:term:->terms' \ + '(-g --geometry)'{-g,--geometry}'[window WIDTHxHEIGHT, in pixels (84x24 cells)]' \ + '(-s --server)'{-s,--server}'[run as server; open terminals by running footclient]' \ + '(-v --version)'{-v,--version}'[show the version number and quit]' \ + '(-h --help)'{-h,--help}'[show help message and quit]' -case "${state}" in +case ${state} in fonts) _values 'font families' $(fc-list : family | tr -d ' ') ;; + + terms) + _values 'terminal definitions' $(find /usr/share/terminfo -type f -printf "%f\n") + ;; esac From 43462b24f369a1b5d71be455adbcc1f474b33a1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 20:40:42 +0100 Subject: [PATCH 30/37] font: cache loaded fonts globally Each font instance has a ref-counter. Whenever we want to instantiate a font that has already been loaded, we instead return the already-loaded instance, and bump the ref counter. When the last font instance is destroyed, it is also removed from the cache. --- font.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/font.c b/font.c index 4b1be833..237f8fee 100644 --- a/font.c +++ b/font.c @@ -22,6 +22,13 @@ static mtx_t ft_lock; static const size_t cache_size = 512; +struct font_cache_entry { + uint64_t hash; + struct font *font; +}; + +static tll(struct font_cache_entry) font_cache = tll_init(); + static void __attribute__((constructor)) init(void) { @@ -33,6 +40,9 @@ init(void) static void __attribute__((destructor)) fini(void) { + while (tll_length(font_cache) > 0) + font_destroy(tll_front(font_cache).font); + mtx_destroy(&ft_lock); FT_Done_FreeType(ft_lib); FcFini(); @@ -275,12 +285,46 @@ from_name(const char *name, bool is_fallback) return font; } +static uint64_t +sdbm_hash(const char *s) +{ + uint64_t hash = 0; + + for (; *s != '\0'; s++) { + int c = *s; + hash = c + (hash << 6) + (hash << 16) - hash; + } + + return hash; +} + +static uint64_t +font_hash(font_list_t names, const char *attributes) +{ + uint64_t hash = 0; + tll_foreach(names, it) + hash ^= sdbm_hash(it->item); + + if (attributes != NULL) + hash ^= sdbm_hash(attributes); + + return hash; +} + struct font * font_from_name(font_list_t names, const char *attributes) { if (tll_length(names) == 0) return false; + uint64_t hash = font_hash(names, attributes); + tll_foreach(font_cache, it) { + if (it->item.hash == hash) { + it->item.font->ref_counter++; + return it->item.font; + } + } + struct font *font = NULL; bool have_attrs = attributes != NULL && strlen(attributes) > 0; @@ -312,6 +356,7 @@ font_from_name(font_list_t names, const char *attributes) font->fallbacks, ((struct font_fallback){.pattern = strdup(name)})); } + tll_push_back(font_cache, ((struct font_cache_entry){.hash = hash, .font = font})); return font; } @@ -605,6 +650,13 @@ font_destroy(struct font *font) if (--font->ref_counter > 0) return; + tll_foreach(font_cache, it) { + if (it->item.font == font) { + tll_remove(font_cache, it); + break; + } + } + free(font->name); tll_foreach(font->fallbacks, it) { From 8b4ef78f7a9692c14e20a32dcf0c250f7812828a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 20:45:57 +0100 Subject: [PATCH 31/37] wayland: wayl_win_destroy(): looks like we need another roundtrip --- wayland.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/wayland.c b/wayland.c index 97917e77..3d19c4de 100644 --- a/wayland.c +++ b/wayland.c @@ -719,6 +719,8 @@ wayl_win_destroy(struct wl_window *win) if (win->surface != NULL) wl_surface_destroy(win->surface); + wl_display_roundtrip(win->wayl->display); + free(win); } From 77ea7fc61c307c81698904c03796c1920e7a0856 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 20:50:00 +0100 Subject: [PATCH 32/37] client: add long options for --verbose and --help --- client.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/client.c b/client.c index bcd3fa6b..25dcbd27 100644 --- a/client.c +++ b/client.c @@ -40,8 +40,14 @@ main(int argc, char *const *argv) const char *const prog_name = argv[0]; + static const struct option longopts[] = { + {"version", no_argument, 0, 'v'}, + {"help", no_argument, 0, 'h'}, + {NULL, no_argument, 0, 0}, + }; + while (true) { - int c = getopt_long(argc, argv, ":hv", NULL, NULL); + int c = getopt_long(argc, argv, ":hv", longopts, NULL); if (c == -1) break; From d2b395bd43a312fd0350f38a07540a65961be43c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 20:51:09 +0100 Subject: [PATCH 33/37] PKGBUILD: need to execute footclient, to generate profiling data --- PKGBUILD | 2 ++ 1 file changed, 2 insertions(+) diff --git a/PKGBUILD b/PKGBUILD index 20207c43..eab42e1e 100644 --- a/PKGBUILD +++ b/PKGBUILD @@ -22,6 +22,8 @@ build() { ./foot --term=xterm -- sh -c "../scripts/generate-alt-random-writes.py --scroll --scroll-region --colors-regular --colors-bright --colors-rgb ${tmp_file} && cat ${tmp_file}" rm "${tmp_file}" + ./footclient --version + meson configure -Db_pgo=use ninja } From 69d62d3cd2918bee089fc879e381820349497483 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 21:01:15 +0100 Subject: [PATCH 34/37] slave: set TERM environment variable in slave process --- main.c | 1 - slave.c | 5 ++++- slave.h | 2 +- terminal.c | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/main.c b/main.c index f13a45d8..c1abe5bc 100644 --- a/main.c +++ b/main.c @@ -161,7 +161,6 @@ main(int argc, char *const *argv) argv += optind; setlocale(LC_ALL, ""); - setenv("TERM", conf.term, 1); struct fdm *fdm = NULL; struct wayland *wayl = NULL; diff --git a/slave.c b/slave.c index 0e762bf0..1f70a0d8 100644 --- a/slave.c +++ b/slave.c @@ -1,5 +1,6 @@ #define _XOPEN_SOURCE 500 #include "slave.h" + #include #include #include @@ -68,7 +69,7 @@ err: pid_t slave_spawn(int ptmx, int argc, char *const *argv, - const char *conf_shell) + const char *term_env, const char *conf_shell) { int fork_pipe[2]; if (pipe2(fork_pipe, O_CLOEXEC) < 0) { @@ -88,6 +89,8 @@ slave_spawn(int ptmx, int argc, char *const *argv, /* Child */ close(fork_pipe[0]); /* Close read end */ + setenv("TERM", term_env, 1); + char **_shell_argv = NULL; char *const *shell_argv = argv; diff --git a/slave.h b/slave.h index cfb18af2..f1a9848a 100644 --- a/slave.h +++ b/slave.h @@ -4,4 +4,4 @@ #include pid_t slave_spawn( - int ptmx, int argc, char *const *argv, const char *conf_shell); + int ptmx, int argc, char *const *argv, const char *term_env, const char *conf_shell); diff --git a/terminal.c b/terminal.c index cbe67b58..d6aaf4ff 100644 --- a/terminal.c +++ b/terminal.c @@ -428,7 +428,7 @@ term_init(const struct config *conf, struct fdm *fdm, struct wayland *wayl, LOG_INFO("cell width=%d, height=%d", term->cell_width, term->cell_height); /* Start the slave/client */ - if ((term->slave = slave_spawn(term->ptmx, argc, argv, conf->shell)) == -1) + if ((term->slave = slave_spawn(term->ptmx, argc, argv, conf->term, conf->shell)) == -1) goto err; /* Initiailze the Wayland window backend */ From 0bd2ddd8ad60a77e27cdb459da8e319647fd6f59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 21:03:08 +0100 Subject: [PATCH 35/37] term_init(): initialize slave TERM from term_init() argument --- main.c | 2 +- server.c | 4 ++-- terminal.c | 4 ++-- terminal.h | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/main.c b/main.c index c1abe5bc..26a8f300 100644 --- a/main.c +++ b/main.c @@ -174,7 +174,7 @@ main(int argc, char *const *argv) if ((wayl = wayl_init(fdm)) == NULL) goto out; - if (!as_server && (term = term_init(&conf, fdm, wayl, argc, argv, + if (!as_server && (term = term_init(&conf, fdm, wayl, conf.term, argc, argv, &term_shutdown_cb, &shutdown_ctx)) == NULL) goto out; diff --git a/server.c b/server.c index ae6d678e..f48ed262 100644 --- a/server.c +++ b/server.c @@ -117,8 +117,8 @@ fdm_client(struct fdm *fdm, int fd, int events, void *data) assert(client->term == NULL); client->term = term_init( - server->conf, server->fdm, server->wayl, client->argc, client->argv, - &term_shutdown_handler, client); + server->conf, server->fdm, server->wayl, server->conf->term, + client->argc, client->argv, &term_shutdown_handler, client); if (client->term == NULL) { LOG_ERR("failed to instantiate new terminal"); diff --git a/terminal.c b/terminal.c index d6aaf4ff..f71d90a6 100644 --- a/terminal.c +++ b/terminal.c @@ -289,7 +289,7 @@ initialize_fonts(struct terminal *term, const struct config *conf) struct terminal * term_init(const struct config *conf, struct fdm *fdm, struct wayland *wayl, - int argc, char *const *argv, + const char *term_env, int argc, char *const *argv, void (*shutdown_cb)(void *data, int exit_code), void *shutdown_data) { int ptmx = -1; @@ -428,7 +428,7 @@ term_init(const struct config *conf, struct fdm *fdm, struct wayland *wayl, LOG_INFO("cell width=%d, height=%d", term->cell_width, term->cell_height); /* Start the slave/client */ - if ((term->slave = slave_spawn(term->ptmx, argc, argv, conf->term, conf->shell)) == -1) + if ((term->slave = slave_spawn(term->ptmx, argc, argv, term_env, conf->shell)) == -1) goto err; /* Initiailze the Wayland window backend */ diff --git a/terminal.h b/terminal.h index 3ec2802c..11571519 100644 --- a/terminal.h +++ b/terminal.h @@ -296,7 +296,7 @@ struct terminal { struct config; struct terminal *term_init( const struct config *conf, struct fdm *fdm, struct wayland *wayl, - int argc, char *const *argv, + const char *term_env, int argc, char *const *argv, void (*shutdown_cb)(void *data, int exit_code), void *shutdown_data); bool term_shutdown(struct terminal *term); From 438d6eaff0b4280492c8a0a161a33daec5de3707 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 21:10:47 +0100 Subject: [PATCH 36/37] client/server: add -t,--term to footclient --- client.c | 20 ++++++++++++++++++-- completions/zsh/_footclient | 7 +++++++ server.c | 16 +++++++++++++++- 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/client.c b/client.c index 25dcbd27..bac72838 100644 --- a/client.c +++ b/client.c @@ -30,7 +30,8 @@ print_usage(const char *prog_name) printf("Usage: %s [OPTIONS]...\n", prog_name); printf("\n"); printf("Options:\n"); - printf(" -v,--version show the version number and quit\n"); + printf(" -t,--term=TERM value to set the environment variable TERM to (foot)\n" + " -v,--version show the version number and quit\n"); } int @@ -41,17 +42,24 @@ main(int argc, char *const *argv) const char *const prog_name = argv[0]; static const struct option longopts[] = { + {"term", required_argument, 0, 't'}, {"version", no_argument, 0, 'v'}, {"help", no_argument, 0, 'h'}, {NULL, no_argument, 0, 0}, }; + const char *term = ""; + while (true) { - int c = getopt_long(argc, argv, ":hv", longopts, NULL); + int c = getopt_long(argc, argv, ":t:hv", longopts, NULL); if (c == -1) break; switch (c) { + case 't': + term = optarg; + break; + case 'v': printf("footclient version %s\n", FOOT_VERSION); return EXIT_SUCCESS; @@ -98,6 +106,14 @@ main(int argc, char *const *argv) } } + uint16_t term_len = strlen(term); + if (send(fd, &term_len, sizeof(term_len), 0) != sizeof(term_len) || + send(fd, term, term_len, 0) != term_len) + { + LOG_ERRNO("failed to send TERM to server"); + goto err; + } + if (send(fd, &argc, sizeof(argc), 0) != sizeof(argc)) { LOG_ERRNO("failed to send argc/argv to server"); goto err; diff --git a/completions/zsh/_footclient b/completions/zsh/_footclient index 6ab98fd0..47a050cb 100644 --- a/completions/zsh/_footclient +++ b/completions/zsh/_footclient @@ -2,5 +2,12 @@ _arguments \ -s \ + '(-t,--term)'{-t,--term}'[value to set the environment variable TERM to (foot)]:term:->terms' \ '(-v --version)'{-v,--version}'[show the version number and quit]' \ '(-h --help)'{-h,--help}'[show help message and quit]' + +case ${state} in + terms) + _values 'terminal definitions' $(find /usr/share/terminfo -type f -printf "%f\n") + ;; +esac diff --git a/server.c b/server.c index f48ed262..6307b1bf 100644 --- a/server.c +++ b/server.c @@ -94,12 +94,22 @@ fdm_client(struct fdm *fdm, int fd, int events, void *data) { struct client *client = data; struct server *server = client->server; + char *term_env = NULL; if (events & EPOLLHUP) goto shutdown; assert(events & EPOLLIN); + uint16_t term_env_len; + if (recv(fd, &term_env_len, sizeof(term_env_len), 0) != sizeof(term_env_len)) + goto shutdown; + + term_env = malloc(term_env_len + 1); + term_env[term_env_len] = '\0'; + if (recv(fd, term_env, term_env_len, 0) != term_env_len) + goto shutdown; + if (recv(fd, &client->argc, sizeof(client->argc), 0) != sizeof(client->argc)) goto shutdown; @@ -117,7 +127,8 @@ fdm_client(struct fdm *fdm, int fd, int events, void *data) assert(client->term == NULL); client->term = term_init( - server->conf, server->fdm, server->wayl, server->conf->term, + server->conf, server->fdm, server->wayl, + term_env_len > 0 ? term_env : server->conf->term, client->argc, client->argv, &term_shutdown_handler, client); if (client->term == NULL) { @@ -125,11 +136,14 @@ fdm_client(struct fdm *fdm, int fd, int events, void *data) goto shutdown; } + free(term_env); return true; shutdown: LOG_DBG("client FD=%d: disconnected", client->fd); + free(term_env); + fdm_del(fdm, fd); client->fd = -1; From 2544e93c0e47f52362f6ce6945d7bf17d0b37e24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 1 Nov 2019 21:13:37 +0100 Subject: [PATCH 37/37] client: register signal handler for SIGTERM too --- client.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client.c b/client.c index bac72838..6b36a4bb 100644 --- a/client.c +++ b/client.c @@ -130,7 +130,8 @@ main(int argc, char *const *argv) } } - if (sigaction(SIGINT, &(struct sigaction){.sa_handler = &sig_handler}, NULL) < 0) { + const struct sigaction sa = {.sa_handler = &sig_handler}; + if (sigaction(SIGINT, &sa, NULL) < 0 || sigaction(SIGTERM, &sa, NULL) < 0) { LOG_ERRNO("failed to register signal handlers"); goto err; }