diff --git a/src/c2.c b/src/c2.c index b8891b7006..08733c1454 100644 --- a/src/c2.c +++ b/src/c2.c @@ -442,11 +442,11 @@ TEST_CASE(c2_parse) { TEST_STREQUAL3(str, "name = \"xterm\"", len); struct wm *wm = wm_new(); - wm_import_incomplete(wm, 1, XCB_NONE); + struct wm_ref *node = wm_new_mock_window(wm, 1); struct win test_win = { .name = "xterm", - .tree_ref = wm_find(wm, 1), + .tree_ref = node, }; TEST_TRUE(c2_match(state, &test_win, cond, NULL)); c2_list_postprocess(state, NULL, cond); @@ -568,6 +568,7 @@ TEST_CASE(c2_parse) { TEST_STREQUAL3(str, rule, len); c2_list_free(&cond, NULL); + wm_free_mock_window(wm, test_win.tree_ref); wm_free(wm); } diff --git a/src/event.c b/src/event.c index 953c882feb..77b20cadaf 100644 --- a/src/event.c +++ b/src/event.c @@ -203,10 +203,14 @@ static inline void ev_focus_out(session_t *ps, xcb_focus_out_event_t *ev) { } static inline void ev_create_notify(session_t *ps, xcb_create_notify_event_t *ev) { - if (wm_is_wid_masked(ps->wm, ev->parent)) { - return; + auto parent = wm_find(ps->wm, ev->parent); + if (parent == NULL) { + log_error("Create notify received for window %#010x, but its parent " + "window %#010x is not in our tree. Expect malfunction.", + ev->window, ev->parent); + assert(false); } - wm_import_incomplete(ps->wm, ev->window, ev->parent); + wm_import_start(ps->wm, &ps->c, ps->atoms, ev->window, parent); } /// Handle configure event of a regular window @@ -215,7 +219,11 @@ static void configure_win(session_t *ps, xcb_configure_notify_event_t *ce) { auto below = wm_find(ps->wm, ce->above_sibling); if (!cursor) { - log_error("Configure event received for unknown window %#010x", ce->window); + if (wm_is_consistent(ps->wm)) { + log_error("Configure event received for unknown window %#010x", + ce->window); + assert(false); + } return; } @@ -223,6 +231,7 @@ static void configure_win(session_t *ps, xcb_configure_notify_event_t *ce) { log_error("Configure event received for window %#010x, but its sibling " "window %#010x is not in our tree. Expect malfunction.", ce->window, ce->above_sibling); + assert(false); } else if (below != NULL) { wm_stack_move_to_above(ps->wm, cursor, below); } else { @@ -288,7 +297,7 @@ static inline void ev_configure_notify(session_t *ps, xcb_configure_notify_event if (ev->window == ps->c.screen_info->root) { set_root_flags(ps, ROOT_FLAGS_CONFIGURED); - } else if (!wm_is_wid_masked(ps->wm, ev->event)) { + } else { configure_win(ps, ev); } } @@ -314,15 +323,14 @@ static inline void ev_map_notify(session_t *ps, xcb_map_notify_event_t *ev) { return; } - if (wm_is_wid_masked(ps->wm, ev->event)) { - return; - } - auto cursor = wm_find(ps->wm, ev->window); if (cursor == NULL) { - log_error("Map event received for unknown window %#010x, overlay is " - "%#010x", - ev->window, ps->overlay); + if (wm_is_consistent(ps->wm)) { + log_debug("Map event received for unknown window %#010x, overlay " + "is %#010x", + ev->window, ps->overlay); + assert(false); + } return; } @@ -349,13 +357,12 @@ static inline void ev_unmap_notify(session_t *ps, xcb_unmap_notify_event_t *ev) return; } - if (wm_is_wid_masked(ps->wm, ev->event)) { - return; - } - auto cursor = wm_find(ps->wm, ev->window); if (cursor == NULL) { - log_error("Unmap event received for unknown window %#010x", ev->window); + if (wm_is_consistent(ps->wm)) { + log_error("Unmap event received for unknown window %#010x", ev->window); + assert(false); + } return; } auto w = wm_ref_deref(cursor); @@ -372,14 +379,14 @@ static inline void ev_reparent_notify(session_t *ps, xcb_reparent_notify_event_t } static inline void ev_circulate_notify(session_t *ps, xcb_circulate_notify_event_t *ev) { - if (wm_is_wid_masked(ps->wm, ev->event)) { - return; - } - auto cursor = wm_find(ps->wm, ev->window); if (cursor == NULL) { - log_error("Circulate event received for unknown window %#010x", ev->window); + if (wm_is_consistent(ps->wm)) { + log_debug("Circulate event received for unknown window %#010x", + ev->window); + assert(false); + } return; } @@ -451,18 +458,18 @@ static inline void ev_property_notify(session_t *ps, xcb_property_notify_event_t return; } - if (wm_is_wid_masked(ps->wm, ev->window)) { - return; - } - ps->pending_updates = true; auto cursor = wm_find(ps->wm, ev->window); if (cursor == NULL) { - log_error("Property notify received for unknown window %#010x", ev->window); + if (wm_is_consistent(ps->wm)) { + log_error("Property notify received for unknown window %#010x", + ev->window); + assert(false); + } return; } - auto toplevel_cursor = wm_ref_toplevel_of(cursor); + auto toplevel_cursor = wm_ref_toplevel_of(ps->wm, cursor); if (ev->atom == ps->atoms->aWM_STATE) { log_debug("WM_STATE changed for window %#010x (%s): %s", ev->window, ev_window_name(ps, ev->window), @@ -470,6 +477,11 @@ static inline void ev_property_notify(session_t *ps, xcb_property_notify_event_t wm_set_has_wm_state(ps->wm, cursor, ev->state != XCB_PROPERTY_DELETE); } + if (toplevel_cursor == NULL) { + assert(!wm_is_consistent(ps->wm)); + return; + } + // We only care if the property is set on the toplevel itself, or on its // client window if it has one. WM_STATE is an exception, it is handled // always because it is what determines if a window is a client window. @@ -629,10 +641,6 @@ ev_selection_clear(session_t *ps, xcb_selection_clear_event_t attr_unused *ev) { } void ev_handle(session_t *ps, xcb_generic_event_t *ev) { - if (XCB_EVENT_RESPONSE_TYPE(ev) != KeymapNotify) { - x_discard_pending_errors(&ps->c, ev->full_sequence); - } - xcb_window_t wid = ev_window(ps, ev); if (ev->response_type != ps->damage_event + XCB_DAMAGE_NOTIFY) { log_debug("event %10.10s serial %#010x window %#010x \"%s\"", @@ -702,7 +710,6 @@ void ev_handle(session_t *ps, xcb_generic_event_t *ev) { case XCB_SELECTION_CLEAR: ev_selection_clear(ps, (xcb_selection_clear_event_t *)ev); break; - case 0: x_handle_error(&ps->c, (xcb_generic_error_t *)ev); break; default: if (ps->shape_exists && ev->response_type == ps->shape_event) { ev_shape_notify(ps, (xcb_shape_notify_event_t *)ev); diff --git a/src/event.h b/src/event.h index 629dec0f3f..675b4d8c10 100644 --- a/src/event.h +++ b/src/event.h @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MPL-2.0 // Copyright (c) 2019, Yuxuan Shui +#pragma once #include #include "common.h" diff --git a/src/inspect.c b/src/inspect.c index 33a646af70..d6e0160225 100644 --- a/src/inspect.c +++ b/src/inspect.c @@ -34,10 +34,13 @@ setup_window(struct x_connection *c, struct atom *atoms, struct options *options return NULL; } - auto toplevel = wm_ref_toplevel_of(cursor); + auto toplevel = wm_ref_toplevel_of(wm, cursor); + BUG_ON_NULL(toplevel); struct win *w = ccalloc(1, struct win); w->state = WSTATE_MAPPED; w->tree_ref = toplevel; + log_debug("Toplevel is %#010x", wm_ref_win_id(toplevel)); + log_debug("Client is %#010x", win_client_id(w, true)); win_update_wintype(c, atoms, w); win_update_frame_extents(c, atoms, w, win_client_id(w, /*fallback_to_self=*/true), options->frame_opacity); @@ -195,14 +198,40 @@ int inspect_main(int argc, char **argv, const char *config_file) { return 1; } - auto atoms attr_unused = init_atoms(c.c); + auto atoms = init_atoms(c.c); auto state = c2_state_new(atoms); options_postprocess_c2_lists(state, &c, &options); struct wm *wm = wm_new(); - wm_import_incomplete(wm, c.screen_info->root, XCB_NONE); - wm_complete_import(wm, &c, atoms); + wm_import_start(wm, &c, atoms, c.screen_info->root, NULL); + // Process events until the window tree is consistent + while (x_has_pending_requests(&c)) { + auto ev = x_poll_for_event(&c); + if (ev == NULL) { + continue; + } + switch (ev->response_type) { + case XCB_CREATE_NOTIFY:; + auto create = (xcb_create_notify_event_t *)ev; + auto parent = wm_find(wm, create->parent); + wm_import_start(wm, &c, atoms, + ((xcb_create_notify_event_t *)ev)->window, parent); + break; + case XCB_DESTROY_NOTIFY: + wm_destroy(wm, ((xcb_destroy_notify_event_t *)ev)->window); + break; + case XCB_REPARENT_NOTIFY:; + auto reparent = (xcb_reparent_notify_event_t *)ev; + wm_reparent(wm, reparent->window, reparent->parent); + break; + default: + // Ignore ConfigureNotify and CirculateNotify, because we don't + // use stacking order for window rules. + break; + } + free(ev); + } auto target = select_window(&c); log_info("Target window: %#x", target); diff --git a/src/picom.c b/src/picom.c index ca8bdde6a0..c05d2d7dca 100644 --- a/src/picom.c +++ b/src/picom.c @@ -509,7 +509,11 @@ static void recheck_focus(session_t *ps) { return; } - cursor = wm_ref_toplevel_of(cursor); + cursor = wm_ref_toplevel_of(ps->wm, cursor); + if (cursor == NULL) { + assert(!wm_is_consistent(ps->wm)); + return; + } // And we set the focus state here auto w = wm_ref_deref(cursor); @@ -1558,8 +1562,8 @@ static void handle_x_events(struct session *ps) { } xcb_generic_event_t *ev; - while ((ev = xcb_poll_for_event(ps->c.c))) { - ev_handle(ps, ev); + while ((ev = x_poll_for_event(&ps->c))) { + ev_handle(ps, (xcb_generic_event_t *)ev); free(ev); }; int err = xcb_connection_has_error(ps->c.c); @@ -1575,10 +1579,6 @@ static void handle_x_events_ev(EV_P attr_unused, ev_prepare *w, int revents attr } static void handle_new_windows(session_t *ps) { - while (!wm_complete_import(ps->wm, &ps->c, ps->atoms)) { - handle_x_events(ps); - } - // Check tree changes first, because later property updates need accurate // client window information struct win *w = NULL; @@ -1666,18 +1666,17 @@ static void handle_pending_updates(struct session *ps, double delta_t) { ps->server_grabbed = true; // Catching up with X server - /// Handle all events X server has sent us so far, so that our internal state will - /// catch up with the X server's state. It only makes sense to call this function - /// in the X critical section, otherwise we will be chasing a moving goal post. - /// - /// (Disappointingly, grabbing the X server doesn't actually prevent the X server - /// state from changing. It should, but in practice we have observed window still - /// being destroyed while we have the server grabbed. This is very disappointing - /// and forces us to use some hacky code to recover when we discover we are - /// out-of-sync.) + // Handle all events X server has sent us so far, so that our internal state will + // catch up with the X server's state. It only makes sense to call this function + // in the X critical section, otherwise we will be chasing a moving goal post. + // + // (Disappointingly, grabbing the X server doesn't actually prevent the X server + // state from changing. It should, but in practice we have observed window still + // being destroyed while we have the server grabbed. This is very disappointing + // and forces us to use some hacky code to recover when we discover we are + // out-of-sync.) handle_x_events(ps); - if (ps->pending_updates || wm_has_incomplete_imports(ps->wm) || - wm_has_tree_changes(ps->wm)) { + if (ps->pending_updates || wm_has_tree_changes(ps->wm)) { log_debug("Delayed handling of events, entering critical section"); // Process new windows, and maybe allocate struct managed_win for them handle_new_windows(ps); @@ -1804,9 +1803,9 @@ static void draw_callback_impl(EV_P_ session_t *ps, int revents attr_unused) { log_fatal("Couldn't find specified benchmark window."); exit(1); } - w = wm_ref_toplevel_of(w); + w = wm_ref_toplevel_of(ps->wm, w); - auto win = wm_ref_deref(w); + auto win = w == NULL ? NULL : wm_ref_deref(w); if (win != NULL) { add_damage_from_win(ps, win); } else { @@ -1978,7 +1977,7 @@ static void draw_callback(EV_P_ ev_timer *w, int revents) { static void x_event_callback(EV_P attr_unused, ev_io *w, int revents attr_unused) { session_t *ps = (session_t *)w; - xcb_generic_event_t *ev = xcb_poll_for_event(ps->c.c); + xcb_generic_event_t *ev = x_poll_for_event(&ps->c); if (ev) { ev_handle(ps, ev); free(ev); @@ -2511,38 +2510,7 @@ static session_t *session_init(int argc, char **argv, Display *dpy, } ps->wm = wm_new(); - wm_import_incomplete(ps->wm, ps->c.screen_info->root, XCB_NONE); - - e = xcb_request_check(ps->c.c, xcb_grab_server_checked(ps->c.c)); - if (e) { - log_fatal_x_error(e, "Failed to grab X server"); - free(e); - goto err; - } - - ps->server_grabbed = true; - - // We are going to pull latest information from X server now, events sent by X - // earlier is irrelevant at this point. - // A better solution is probably grabbing the server from the very start. But I - // think there still could be race condition that mandates discarding the events. - x_discard_events(&ps->c); - - wm_complete_import(ps->wm, &ps->c, ps->atoms); - - e = xcb_request_check(ps->c.c, xcb_ungrab_server_checked(ps->c.c)); - if (e) { - log_fatal_x_error(e, "Failed to ungrab server"); - free(e); - goto err; - } - - ps->server_grabbed = false; - - log_debug("Initial stack:"); - wm_stack_foreach(ps->wm, i) { - log_debug(" %#010x", wm_ref_win_id(i)); - } + wm_import_start(ps->wm, &ps->c, ps->atoms, ps->c.screen_info->root, NULL); ps->command_builder = command_builder_new(); ps->expose_rects = dynarr_new(rect_t, 0); @@ -2737,8 +2705,10 @@ static void session_destroy(session_t *ps) { ev_signal_stop(ps->loop, &ps->usr1_signal); ev_signal_stop(ps->loop, &ps->int_signal); - wm_free(ps->wm); + // The X connection could hold references to wm if there are pending async + // requests. Therefore the wm must be freed after the X connection. free_x_connection(&ps->c); + wm_free(ps->wm); } /** diff --git a/src/wm/tree.c b/src/wm/tree.c index 3c4c493234..99e195d30f 100644 --- a/src/wm/tree.c +++ b/src/wm/tree.c @@ -40,16 +40,25 @@ static void wm_tree_enqueue_change(struct wm_tree *tree, struct wm_tree_change c // `WM_TREE_CHANGE_TOPLEVEL_NEW` change in the queue. bool found = false; list_foreach_safe(struct wm_tree_change_list, i, &tree->changes, siblings) { - if (i->item.type == WM_TREE_CHANGE_TOPLEVEL_NEW && - wm_treeid_eq(i->item.toplevel, change.toplevel)) { + if (!wm_treeid_eq(i->item.toplevel, change.toplevel)) { + continue; + } + if (i->item.type == WM_TREE_CHANGE_TOPLEVEL_NEW) { list_remove(&i->siblings); list_insert_after(&tree->free_changes, &i->siblings); found = true; - } else if (wm_treeid_eq(i->item.toplevel, change.toplevel) && found) { - // We also need to delete all other changes related to - // this toplevel in between the new and gone changes. + } else if (found) { + // We also need to delete all other changes + // related to this toplevel in between the new and + // gone changes. list_remove(&i->siblings); list_insert_after(&tree->free_changes, &i->siblings); + } else if (i->item.type == WM_TREE_CHANGE_CLIENT) { + // Need to update client changes, so they points to the + // zombie instead of the old toplevel node, since the old + // toplevel node could be freed before tree changes are + // processed. + i->item.client.toplevel = change.killed; } } if (found) { @@ -126,6 +135,10 @@ struct wm_tree_change wm_tree_dequeue_change(struct wm_tree *tree) { /// Return the next node in the subtree after `node` in a pre-order traversal. Returns /// NULL if `node` is the last node in the traversal. struct wm_tree_node *wm_tree_next(struct wm_tree_node *node, struct wm_tree_node *subroot) { + if (node == NULL) { + return NULL; + } + if (!list_is_empty(&node->children)) { // Descend if there are children return list_entry(node->children.next, struct wm_tree_node, siblings); @@ -171,7 +184,8 @@ struct wm_tree_node *wm_tree_find(const struct wm_tree *tree, xcb_window_t id) { return node; } -struct wm_tree_node *wm_tree_find_toplevel_for(struct wm_tree_node *node) { +struct wm_tree_node * +wm_tree_find_toplevel_for(const struct wm_tree *tree, struct wm_tree_node *node) { BUG_ON_NULL(node); BUG_ON_NULL(node->parent); // Trying to find toplevel for the root // window @@ -180,7 +194,7 @@ struct wm_tree_node *wm_tree_find_toplevel_for(struct wm_tree_node *node) { for (auto curr = node; curr->parent != NULL; curr = curr->parent) { toplevel = curr; } - return toplevel; + return toplevel->parent == tree->root ? toplevel : NULL; } /// Change whether a tree node has the `WM_STATE` property set. @@ -234,31 +248,17 @@ void wm_tree_set_wm_state(struct wm_tree *tree, struct wm_tree_node *node, bool } } -struct wm_tree_node * -wm_tree_new_window(struct wm_tree *tree, xcb_window_t id, struct wm_tree_node *parent) { +struct wm_tree_node *wm_tree_new_window(struct wm_tree *tree, xcb_window_t id) { auto node = ccalloc(1, struct wm_tree_node); node->id.x = id; node->id.gen = tree->gen++; node->has_wm_state = false; list_init_head(&node->children); + return node; +} - BUG_ON(parent == NULL && tree->nodes != NULL); // Trying to create a second - // root window +void wm_tree_add_window(struct wm_tree *tree, struct wm_tree_node *node) { HASH_ADD_INT(tree->nodes, id.x, node); - - node->parent = parent; - if (parent != NULL) { - list_insert_after(&parent->children, &node->siblings); - if (parent->parent == NULL) { - // Parent is root, this is a new toplevel window - wm_tree_enqueue_change(tree, (struct wm_tree_change){ - .toplevel = node->id, - .type = WM_TREE_CHANGE_TOPLEVEL_NEW, - .new_ = node, - }); - } - } - return node; } static void @@ -288,10 +288,12 @@ void wm_tree_detach(struct wm_tree *tree, struct wm_tree_node *subroot) { BUG_ON(subroot == NULL); BUG_ON(subroot->parent == NULL); // Trying to detach the root window?! - auto toplevel = wm_tree_find_toplevel_for(subroot); + auto toplevel = wm_tree_find_toplevel_for(tree, subroot); if (toplevel != subroot) { list_remove(&subroot->siblings); - wm_tree_refresh_client_and_queue_change(tree, toplevel); + if (toplevel != NULL) { + wm_tree_refresh_client_and_queue_change(tree, toplevel); + } } else { // Detached a toplevel, create a zombie for it auto zombie = ccalloc(1, struct wm_tree_node); @@ -299,6 +301,7 @@ void wm_tree_detach(struct wm_tree *tree, struct wm_tree_node *subroot) { zombie->id = subroot->id; zombie->is_zombie = true; zombie->win = subroot->win; + subroot->win = NULL; list_init_head(&zombie->children); list_replace(&subroot->siblings, &zombie->siblings); wm_tree_enqueue_change(tree, (struct wm_tree_change){ @@ -307,38 +310,32 @@ void wm_tree_detach(struct wm_tree *tree, struct wm_tree_node *subroot) { .killed = zombie, }); } + subroot->parent = NULL; } -void wm_tree_destroy_window(struct wm_tree *tree, struct wm_tree_node *node) { - BUG_ON(node == NULL); - BUG_ON(node->parent == NULL); // Trying to destroy the root window?! - - if (node->has_wm_state) { - wm_tree_set_wm_state(tree, node, false); +void wm_tree_attach(struct wm_tree *tree, struct wm_tree_node *child, + struct wm_tree_node *parent) { + BUG_ON(child->parent != NULL); // Trying to attach a window that's already + // attached + child->parent = parent; + if (parent == NULL) { + BUG_ON(tree->root != NULL); // Trying to create a second root + // window + tree->root = child; + return; } - HASH_DEL(tree->nodes, node); - - if (!list_is_empty(&node->children)) { - log_error("Window %#010x is destroyed, but it still has children. Expect " - "malfunction.", - node->id.x); - list_foreach_safe(struct wm_tree_node, i, &node->children, siblings) { - log_error(" Child window %#010x", i->id.x); - wm_tree_destroy_window(tree, i); - } - } + list_insert_after(&parent->children, &child->siblings); - if (node->parent->parent == NULL) { - node->is_zombie = true; + auto toplevel = wm_tree_find_toplevel_for(tree, child); + if (child == toplevel) { wm_tree_enqueue_change(tree, (struct wm_tree_change){ - .toplevel = node->id, - .type = WM_TREE_CHANGE_TOPLEVEL_KILLED, - .killed = node, + .toplevel = child->id, + .type = WM_TREE_CHANGE_TOPLEVEL_NEW, + .new_ = child, }); - } else { - list_remove(&node->siblings); - free(node); + } else if (toplevel != NULL) { + wm_tree_refresh_client_and_queue_change(tree, toplevel); } } @@ -357,7 +354,7 @@ void wm_tree_move_to_end(struct wm_tree *tree, struct wm_tree_node *node, bool t } else { list_insert_after(&node->parent->children, &node->siblings); } - if (node->parent->parent == NULL) { + if (node->parent == tree->root) { wm_tree_enqueue_change(tree, (struct wm_tree_change){ .type = WM_TREE_CHANGE_TOPLEVEL_RESTACKED, }); @@ -379,45 +376,13 @@ void wm_tree_move_to_above(struct wm_tree *tree, struct wm_tree_node *node, list_remove(&node->siblings); list_insert_before(&other->siblings, &node->siblings); - if (node->parent->parent == NULL) { + if (node->parent == tree->root) { wm_tree_enqueue_change(tree, (struct wm_tree_change){ .type = WM_TREE_CHANGE_TOPLEVEL_RESTACKED, }); } } -void wm_tree_reparent(struct wm_tree *tree, struct wm_tree_node *node, - struct wm_tree_node *new_parent) { - BUG_ON(node == NULL); - BUG_ON(new_parent == NULL); // Trying make `node` a root window - - if (node->parent == new_parent) { - // Reparent to the same parent moves the window to the top of the stack - wm_tree_move_to_end(tree, node, false); - return; - } - - wm_tree_detach(tree, node); - - // Reparented window always becomes the topmost child of the new parent - list_insert_after(&new_parent->children, &node->siblings); - node->parent = new_parent; - - auto toplevel = wm_tree_find_toplevel_for(node); - if (node == toplevel) { - // This node could have a stale `->win` if it was a toplevel at - // some point in the past. - node->win = NULL; - wm_tree_enqueue_change(tree, (struct wm_tree_change){ - .toplevel = node->id, - .type = WM_TREE_CHANGE_TOPLEVEL_NEW, - .new_ = node, - }); - } else { - wm_tree_refresh_client_and_queue_change(tree, toplevel); - } -} - void wm_tree_clear(struct wm_tree *tree) { struct wm_tree_node *cur, *tmp; HASH_ITER(hh, tree->nodes, cur, tmp) { @@ -438,17 +403,21 @@ TEST_CASE(tree_manipulation) { struct wm_tree tree; wm_tree_init(&tree); - wm_tree_new_window(&tree, 1, NULL); + wm_tree_add_window(&tree, wm_tree_new_window(&tree, 1)); auto root = wm_tree_find(&tree, 1); assert(root != NULL); assert(root->parent == NULL); + tree.root = root; + auto change = wm_tree_dequeue_change(&tree); assert(change.type == WM_TREE_CHANGE_NONE); - wm_tree_new_window(&tree, 2, root); - auto node2 = wm_tree_find(&tree, 2); + auto node2 = wm_tree_new_window(&tree, 2); + wm_tree_add_window(&tree, node2); + wm_tree_attach(&tree, node2, root); assert(node2 != NULL); + assert(node2 == wm_tree_find(&tree, 2)); assert(node2->parent == root); change = wm_tree_dequeue_change(&tree); @@ -456,15 +425,16 @@ TEST_CASE(tree_manipulation) { assert(change.type == WM_TREE_CHANGE_TOPLEVEL_NEW); assert(wm_treeid_eq(node2->id, change.toplevel)); - wm_tree_new_window(&tree, 3, root); - auto node3 = wm_tree_find(&tree, 3); - assert(node3 != NULL); + auto node3 = wm_tree_new_window(&tree, 3); + wm_tree_add_window(&tree, node3); + wm_tree_attach(&tree, node3, root); change = wm_tree_dequeue_change(&tree); assert(change.toplevel.x == 3); assert(change.type == WM_TREE_CHANGE_TOPLEVEL_NEW); - wm_tree_reparent(&tree, node2, node3); + wm_tree_detach(&tree, node2); + wm_tree_attach(&tree, node2, node3); assert(node2->parent == node3); assert(node3->children.next == &node2->siblings); @@ -481,8 +451,9 @@ TEST_CASE(tree_manipulation) { assert(wm_treeid_eq(change.client.old, WM_TREEID_NONE)); assert(change.client.new_.x == 2); - wm_tree_new_window(&tree, 4, node3); - auto node4 = wm_tree_find(&tree, 4); + auto node4 = wm_tree_new_window(&tree, 4); + wm_tree_add_window(&tree, node4); + wm_tree_attach(&tree, node4, node3); change = wm_tree_dequeue_change(&tree); assert(change.type == WM_TREE_CHANGE_NONE); @@ -491,7 +462,9 @@ TEST_CASE(tree_manipulation) { // node3 already has node2 as its client window, so the new one should be ignored. assert(change.type == WM_TREE_CHANGE_NONE); - wm_tree_destroy_window(&tree, node2); + wm_tree_detach(&tree, node2); + HASH_DEL(tree.nodes, node2); + free(node2); change = wm_tree_dequeue_change(&tree); assert(change.toplevel.x == 3); assert(change.type == WM_TREE_CHANGE_CLIENT); @@ -499,8 +472,12 @@ TEST_CASE(tree_manipulation) { assert(change.client.new_.x == 4); // Test window ID reuse - wm_tree_destroy_window(&tree, node4); - node4 = wm_tree_new_window(&tree, 4, node3); + wm_tree_detach(&tree, node4); + HASH_DEL(tree.nodes, node4); + free(node4); + node4 = wm_tree_new_window(&tree, 4); + wm_tree_add_window(&tree, node4); + wm_tree_attach(&tree, node4, node3); wm_tree_set_wm_state(&tree, node4, true); change = wm_tree_dequeue_change(&tree); @@ -509,8 +486,12 @@ TEST_CASE(tree_manipulation) { assert(change.client.old.x == 4); assert(change.client.new_.x == 4); - auto node5 = wm_tree_new_window(&tree, 5, root); - wm_tree_destroy_window(&tree, node5); + auto node5 = wm_tree_new_window(&tree, 5); + wm_tree_add_window(&tree, node5); + wm_tree_attach(&tree, node5, root); + wm_tree_detach(&tree, node5); + HASH_DEL(tree.nodes, node5); + free(node5); change = wm_tree_dequeue_change(&tree); assert(change.type == WM_TREE_CHANGE_NONE); // Changes cancelled out diff --git a/src/wm/win.c b/src/wm/win.c index 43cb5629d8..af3bde3055 100644 --- a/src/wm/win.c +++ b/src/wm/win.c @@ -1591,7 +1591,7 @@ static struct wm_ref *win_get_leader_raw(session_t *ps, struct win *w, int recur // ancestors if (w->cache_leader && w->cache_leader != client_win && w->cache_leader != w->tree_ref) { - auto parent = wm_ref_toplevel_of(w->cache_leader); + auto parent = wm_ref_toplevel_of(ps->wm, w->cache_leader); auto wp = parent ? wm_ref_deref(parent) : NULL; if (wp) { // Dead loop? diff --git a/src/wm/wm.c b/src/wm/wm.c index c8d37156b0..58e75c5eac 100644 --- a/src/wm/wm.c +++ b/src/wm/wm.c @@ -15,6 +15,20 @@ #include "wm.h" #include "wm_internal.h" +struct wm_query_tree_request { + struct x_async_request_base base; + struct wm_tree_node *node; + struct wm *wm; + struct atom *atoms; + size_t pending_index; +}; + +struct wm_get_property_request { + struct x_async_request_base base; + struct wm *wm; + xcb_window_t wid; +}; + struct wm { /// Pointer to win of current active window. Used by /// EWMH _NET_ACTIVE_WINDOW focus detection. In theory, @@ -27,14 +41,20 @@ struct wm { struct wm_tree_node *active_leader; struct wm_tree tree; - struct wm_tree_node *root; - - /// Incomplete imports. See `wm_import_incomplete` for an explanation. - /// This is a dynarr. - struct wm_tree_node **incompletes; - /// Tree nodes that we have chosen to forget, but we might still receive some - /// events from, we keep them here to ignore those events. - struct wm_tree_node **masked; + /// This is a virtual root for all "orphaned" windows. A window is orphaned + /// if it is not reachable from the root node. This can only be non-empty if + /// the tree is not consistent, i.e. there are pending async query tree requests. + /// + /// Note an orphaned window cannot be a toplevel. This is trivially true because + /// a toplevel has the root window as its parent, and once the root window is + /// created its list of children is always up to date. + struct wm_tree_node orphan_root; + + /// Tree nodes that have pending async query tree requests. We also have async get + /// property requests, but they are not tracked because they don't affect the tree + /// structure. We guarantee that when there are pending query tree requests, no + /// tree nodes will be freed. This is a dynarr. + struct wm_query_tree_request **pending_query_trees; }; // TODO(yshui): this is a bit weird and I am not decided on it yet myself. Maybe we can @@ -86,19 +106,6 @@ void wm_ref_set(struct wm_ref *cursor, struct win *w) { to_tree_node_mut(cursor)->win = w; } -static ptrdiff_t wm_find_masked(struct wm *wm, xcb_window_t wid) { - dynarr_foreach(wm->masked, m) { - if ((*m)->id.x == wid) { - return m - wm->masked; - } - } - return -1; -} - -bool wm_is_wid_masked(struct wm *wm, xcb_window_t wid) { - return wm_find_masked(wm, wid) != -1; -} - struct win *wm_active_win(struct wm *wm) { return wm->active_win; } @@ -132,7 +139,7 @@ struct wm_ref *wm_ref_above(const struct wm_ref *cursor) { } struct wm_ref *wm_root_ref(const struct wm *wm) { - return (struct wm_ref *)&wm->root->siblings; + return (struct wm_ref *)&wm->tree.root->siblings; } struct wm_ref *wm_ref_topmost_child(const struct wm_ref *cursor) { @@ -152,12 +159,16 @@ struct wm_ref *wm_find(const struct wm *wm, xcb_window_t id) { struct wm_ref *wm_find_by_client(const struct wm *wm, xcb_window_t client) { auto node = wm_tree_find(&wm->tree, client); - return node != NULL ? (struct wm_ref *)&wm_tree_find_toplevel_for(node)->siblings - : NULL; + if (node == NULL) { + return NULL; + } + auto toplevel = wm_tree_find_toplevel_for(&wm->tree, node); + return toplevel != NULL ? (struct wm_ref *)&toplevel->siblings : NULL; } -struct wm_ref *wm_ref_toplevel_of(struct wm_ref *cursor) { - return (struct wm_ref *)&wm_tree_find_toplevel_for(to_tree_node_mut(cursor))->siblings; +struct wm_ref *wm_ref_toplevel_of(const struct wm *wm, struct wm_ref *cursor) { + auto toplevel = wm_tree_find_toplevel_for(&wm->tree, to_tree_node_mut(cursor)); + return toplevel != NULL ? (struct wm_ref *)&toplevel->siblings : NULL; } struct wm_ref *wm_ref_client_of(struct wm_ref *cursor) { @@ -165,29 +176,44 @@ struct wm_ref *wm_ref_client_of(struct wm_ref *cursor) { return client != NULL ? (struct wm_ref *)&client->siblings : NULL; } -void wm_remove(struct wm *wm, struct wm_ref *w) { - wm_tree_destroy_window(&wm->tree, (struct wm_tree_node *)w); +struct wm_ref *wm_stack_end(struct wm *wm) { + return (struct wm_ref *)&wm->tree.root->children; } -struct wm_ref *wm_stack_end(struct wm *wm) { - return (struct wm_ref *)&wm->root->children; +static long wm_find_pending_query_tree(struct wm *wm, struct wm_tree_node *node) { + dynarr_foreach(wm->pending_query_trees, i) { + if ((*i)->node == node) { + return i - wm->pending_query_trees; + } + } + return -1; } /// Move window `w` so it's right above `below`, if `below` is 0, `w` is moved /// to the bottom of the stack void wm_stack_move_to_above(struct wm *wm, struct wm_ref *cursor, struct wm_ref *below) { - wm_tree_move_to_above(&wm->tree, to_tree_node_mut(cursor), to_tree_node_mut(below)); + auto node = to_tree_node_mut(cursor); + if (node->parent == &wm->orphan_root) { + // If this window is orphaned, moving it around its siblings is + // meaningless. Same below. + return; + } + wm_tree_move_to_above(&wm->tree, node, to_tree_node_mut(below)); } void wm_stack_move_to_end(struct wm *wm, struct wm_ref *cursor, bool to_bottom) { - wm_tree_move_to_end(&wm->tree, to_tree_node_mut(cursor), to_bottom); + auto node = to_tree_node_mut(cursor); + if (node->parent == &wm->orphan_root) { + return; + } + wm_tree_move_to_end(&wm->tree, node, to_bottom); } struct wm *wm_new(void) { auto wm = ccalloc(1, struct wm); wm_tree_init(&wm->tree); - wm->incompletes = dynarr_new(struct wm_tree_node *, 4); - wm->masked = dynarr_new(struct wm_tree_node *, 8); + list_init_head(&wm->orphan_root.children); + wm->pending_query_trees = dynarr_new(struct wm_query_tree_request *, 0); return wm; } @@ -195,47 +221,72 @@ void wm_free(struct wm *wm) { // Free all `struct win`s associated with tree nodes, this leaves dangling // pointers, but we are freeing the tree nodes immediately after, so everything // is fine (TM). - wm_stack_foreach_safe(wm, i, next) { - auto w = wm_ref_deref(i); - auto tree_node = to_tree_node_mut(i); - free(w); - - if (tree_node->is_zombie) { - // This mainly happens on `session_destroy`, e.g. when there's - // ongoing animations. - log_debug("Leftover zombie node for window %#010x", tree_node->id.x); - wm_tree_reap_zombie(tree_node); + if (wm->tree.root != NULL) { + wm_stack_foreach_safe(wm, i, next) { + auto w = wm_ref_deref(i); + auto tree_node = to_tree_node_mut(i); + free(w); + + if (tree_node->is_zombie) { + // This mainly happens on `session_destroy`, e.g. when + // there's ongoing animations. + log_debug("Leftover zombie node for window %#010x", + tree_node->id.x); + wm_tree_reap_zombie(tree_node); + } } } wm_tree_clear(&wm->tree); - dynarr_free_pod(wm->incompletes); - dynarr_free_pod(wm->masked); + assert(wm_is_consistent(wm)); + assert(list_is_empty(&wm->orphan_root.children)); + dynarr_free_pod(wm->pending_query_trees); free(wm); } -void wm_destroy(struct wm *wm, xcb_window_t wid) { - auto masked = wm_find_masked(wm, wid); - if (masked != -1) { - free(wm->masked[masked]); - dynarr_remove_swap(wm->masked, (size_t)masked); - return; +/// Once the window tree reaches a consistent state, we know any tree nodes that are not +/// reachable from the root must have been destroyed, so we can safely free them. +/// +/// There are cases where we won't receive DestroyNotify events for these windows. For +/// example, if a window is reparented to a window that is not yet in our tree, then +/// destroyed, we won't receive a DestroyNotify event for it. +static void wm_reap_orphans(struct wm *wm) { + // Reap orphaned windows + while (!list_is_empty(&wm->orphan_root.children)) { + auto node = + list_entry(wm->orphan_root.children.next, struct wm_tree_node, siblings); + list_remove(&node->siblings); + if (!list_is_empty(&node->children)) { + log_error("Orphaned window %#010x still has children", node->id.x); + list_splice(&node->children, &wm->orphan_root.children); + } + HASH_DEL(wm->tree.nodes, node); + free(node); } +} +void wm_destroy(struct wm *wm, xcb_window_t wid) { struct wm_tree_node *node = wm_tree_find(&wm->tree, wid); if (!node) { - log_error("Trying to destroy window %#010x, but it's not in the tree. " - "Expect malfunction.", - wid); + if (wm_is_consistent(wm)) { + log_error("Window %#010x destroyed, but it's not in our tree.", wid); + } return; } - auto index = dynarr_find_pod(wm->incompletes, node); - if (index != -1) { - dynarr_remove_swap(wm->incompletes, (size_t)index); - } + log_debug("Destroying window %#010x", wid); - wm_tree_destroy_window(&wm->tree, node); + if (!list_is_empty(&node->children)) { + log_error("Window %#010x is destroyed but it still has children", wid); + } + wm_tree_detach(&wm->tree, node); + // There could be an in-flight query tree request for this window, we orphan it. + // It will be reaped when all query tree requests are completed. (Or right now if + // the tree is already consistent.) + wm_tree_attach(&wm->tree, node, &wm->orphan_root); + if (wm_is_consistent(wm)) { + wm_reap_orphans(wm); + } } void wm_reap_zombie(struct wm_ref *zombie) { @@ -245,172 +296,229 @@ void wm_reap_zombie(struct wm_ref *zombie) { void wm_reparent(struct wm *wm, xcb_window_t wid, xcb_window_t parent) { auto window = wm_tree_find(&wm->tree, wid); auto new_parent = wm_tree_find(&wm->tree, parent); - // We delete the window here if parent is not found, or if the parent is - // an incomplete import. We will rediscover this window later in - // `wm_complete_import`. Keeping it around will only confuse us. - bool should_forget = - new_parent == NULL || dynarr_find_pod(wm->incompletes, new_parent) != -1; + + // We orphan the window here if parent is not found. We will reconnect + // this window later as query tree requests being completed. if (window == NULL) { - log_debug("Reparenting window %#010x which is not in our tree. Assuming " - "it came from fog of war.", - wid); - if (!should_forget) { - wm_import_incomplete(wm, wid, parent); + if (wm_is_consistent(wm)) { + log_error("Window %#010x reparented, but it's not in " + "our tree.", + wid); } return; } - if (should_forget) { - log_debug("Window %#010x reparented to window %#010x which is " - "%s, forgetting and masking instead.", - window->id.x, parent, - new_parent == NULL ? "not in our tree" : "an incomplete import"); - - wm_tree_detach(&wm->tree, window); - for (auto curr = window; curr != NULL;) { - auto next = wm_tree_next(curr, window); - HASH_DEL(wm->tree.nodes, curr); - auto incomplete_index = dynarr_find_pod(wm->incompletes, curr); - if (incomplete_index != -1) { - dynarr_remove_swap(wm->incompletes, (size_t)incomplete_index); - // Incomplete imports cannot have children. - assert(list_is_empty(&curr->children)); - - // Incomplete imports won't have event masks set, so we - // don't need to mask them. - free(curr); - } else { - dynarr_push(wm->masked, curr); - } - curr = next; - } + + if (window->parent == new_parent) { + // Reparent to the same parent moves the window to the top of the + // stack + wm_tree_move_to_end(&wm->tree, window, false); return; } - wm_tree_reparent(&wm->tree, window, new_parent); + wm_tree_detach(&wm->tree, window); + + // Attaching `window` to `new_parent` will change the children list of + // `new_parent`, if there is a pending query tree request for `new_parent`, doing + // so will create an overlap. In other words, `window` will appear in the query + // tree reply too. Generally speaking, we want to keep a node's children list + // empty while there is a pending query tree request for it. (Imagine sending the + // query tree "locks" the children list until the reply is processed). Same logic + // applies to `wm_import_start`. + // + // Alternatively if the new parent isn't in our tree yet, we orphan the window + // too. + if (new_parent == NULL || wm_find_pending_query_tree(wm, new_parent) != -1) { + log_debug("Window %#010x is attached to window %#010x which is " + "currently been queried, orphaning.", + window->id.x, new_parent->id.x); + wm_tree_attach(&wm->tree, window, &wm->orphan_root); + } else { + wm_tree_attach(&wm->tree, window, new_parent); + } } void wm_set_has_wm_state(struct wm *wm, struct wm_ref *cursor, bool has_wm_state) { wm_tree_set_wm_state(&wm->tree, to_tree_node_mut(cursor), has_wm_state); } -void wm_import_incomplete(struct wm *wm, xcb_window_t wid, xcb_window_t parent) { - auto masked = wm_find_masked(wm, wid); - if (masked != -1) { - // A new window created with the same wid means the window we chose to - // forget has been deleted (without us knowing), and its ID was then - // reused. - free(wm->masked[masked]); - dynarr_remove_swap(wm->masked, (size_t)masked); - } - auto parent_cursor = NULL; - if (parent != XCB_NONE) { - parent_cursor = wm_tree_find(&wm->tree, parent); - if (parent_cursor == NULL) { - log_error("Importing window %#010x, but its parent %#010x is not " - "in our tree, ignoring. Expect malfunction.", - wid, parent); - return; - } - } - log_debug("Importing window %#010x with parent %#010x", wid, parent); - auto new = wm_tree_new_window(&wm->tree, wid, parent_cursor); - dynarr_push(wm->incompletes, new); - if (parent == XCB_NONE) { - BUG_ON(wm->root != NULL); // Can't have more than one root - wm->root = new; - } -} static const xcb_event_mask_t WM_IMPORT_EV_MASK = XCB_EVENT_MASK_SUBSTRUCTURE_NOTIFY | XCB_EVENT_MASK_PROPERTY_CHANGE; -static void wm_complete_import_single(struct wm *wm, struct x_connection *c, - struct atom *atoms, struct wm_tree_node *node) { - log_debug("Finishing importing window %#010x with parent %#010lx.", node->id.x, - node->parent != NULL ? node->parent->id.x : XCB_NONE); - x_set_error_action_ignore( - c, xcb_change_window_attributes(c->c, node->id.x, XCB_CW_EVENT_MASK, - (const uint32_t[]){WM_IMPORT_EV_MASK})); - if (wid_has_prop(c->c, node->id.x, atoms->aWM_STATE)) { - wm_tree_set_wm_state(&wm->tree, node, true); + +static void wm_import_start_no_flush(struct wm *wm, struct x_connection *c, struct atom *atoms, + xcb_window_t wid, struct wm_tree_node *parent); + +static void +wm_handle_query_tree_reply(struct x_connection *c, struct x_async_request_base *base, + xcb_raw_generic_event_t *reply_or_error) { + auto req = (struct wm_query_tree_request *)base; + auto wm = req->wm; + { + auto last_req = dynarr_last(wm->pending_query_trees); + dynarr_remove_swap(req->wm->pending_query_trees, req->pending_index); + last_req->pending_index = req->pending_index; } - auto masked = wm_find_masked(wm, node->id.x); - if (masked != -1) { - free(wm->masked[masked]); - dynarr_remove_swap(wm->masked, (size_t)masked); + if (reply_or_error == NULL) { + goto out; } -} -static bool wm_complete_import_subtree(struct wm *wm, struct x_connection *c, - struct atom *atoms, struct wm_tree_node *subroot) { - bool out_of_sync = false; - wm_complete_import_single(wm, c, atoms, subroot); + auto node = req->node; - for (auto curr = subroot; curr != NULL; curr = wm_tree_next(curr, subroot)) { - // Surprise! This newly imported window might already have children. - // Although we haven't setup SubstructureNotify for it yet, it's still - // possible for another window to be reparented to it. + if (reply_or_error->response_type == 0) { + // This is an error, most likely the window is gone when we tried + // to query it. + xcb_generic_error_t *err = (xcb_generic_error_t *)reply_or_error; + log_debug("Query tree request for window %#010x failed with " + "error %s", + node->id.x, x_strerror(err)); + goto out; + } - xcb_query_tree_reply_t *tree = XCB_AWAIT(xcb_query_tree, c->c, curr->id.x); - if (!tree) { - log_error("Disappearing window subtree rooted at %#010x. We are " - "out-of-sync.", - curr->id.x); - out_of_sync = true; + xcb_query_tree_reply_t *reply = (xcb_query_tree_reply_t *)reply_or_error; + log_debug("Finished querying tree for window %#010x", node->id.x); + + auto children = xcb_query_tree_children(reply); + log_debug("Window %#010x has %d children", node->id.x, + xcb_query_tree_children_length(reply)); + for (int i = 0; i < xcb_query_tree_children_length(reply); i++) { + auto child = children[i]; + auto child_node = wm_tree_find(&wm->tree, child); + if (child_node == NULL) { + wm_import_start_no_flush(wm, c, req->atoms, child, node); continue; } - auto children = xcb_query_tree_children(tree); - auto children_len = xcb_query_tree_children_length(tree); - for (int i = 0; i < children_len; i++) { - // `children` goes from bottom to top, and `wm_tree_new_window` - // puts the new window at the top of the stacking order, which - // means the windows will naturally be in the correct stacking - // order. - auto existing = wm_tree_find(&wm->tree, children[i]); - if (existing != NULL) { - // This should never happen: we haven't subscribed to - // child creation events yet, and any window reparented to - // an incomplete is deleted. report an error and try to - // recover. - auto index = dynarr_find_pod(wm->incompletes, existing); - if (index != -1) { - dynarr_remove_swap(wm->incompletes, (size_t)index); - } - log_error("Window %#010x already exists in the tree, but " - "it appeared again as a child of window " - "%#010x. Deleting the old one, but expect " - "malfunction.", - children[i], curr->id.x); - wm_tree_destroy_window(&wm->tree, existing); - } - existing = wm_tree_new_window(&wm->tree, children[i], curr); - wm_complete_import_single(wm, c, atoms, existing); - } - free(tree); + // If child node exists, it must be a previously orphaned node. + assert(child_node->parent == &wm->orphan_root); + wm_tree_detach(&wm->tree, child_node); + wm_tree_attach(&wm->tree, child_node, node); + } + +out: + free(req); + xcb_flush(c->c); // Actually send the requests + if (wm_is_consistent(wm)) { + wm_reap_orphans(wm); } - return !out_of_sync; } -bool wm_complete_import(struct wm *wm, struct x_connection *c, struct atom *atoms) { - while (!dynarr_is_empty(wm->incompletes)) { - auto i = dynarr_pop(wm->incompletes); - // This function modifies `wm->incompletes`, so we can't use - // `dynarr_foreach`. - if (!wm_complete_import_subtree(wm, c, atoms, i)) { - log_debug("Out-of-sync with the X server, try to resync."); - return false; +static void wm_handle_get_wm_state_reply(struct x_connection * /*c*/, + struct x_async_request_base *base, + xcb_raw_generic_event_t *reply_or_error) { + auto req = (struct wm_get_property_request *)base; + if (reply_or_error == NULL) { + free(req); + return; + } + + // We guarantee that if a query tree request is pending, its corresponding + // window tree node won't be reaped. But we don't guarantee the same for + // get property requests. So we need to search the node by window ID again. + + if (reply_or_error->response_type == 0) { + // This is an error, most likely the window is gone when we tried + // to query it. (Note the tree node might have been freed at this + // point if the query tree request also failed earlier.) + xcb_generic_error_t *err = (xcb_generic_error_t *)reply_or_error; + log_debug("Get WM_STATE request for window %#010x failed with " + "error %s", + req->wid, x_strerror(err)); + free(req); + return; + } + + auto node = wm_tree_find(&req->wm->tree, req->wid); + BUG_ON_NULL(node); // window must exist at this point, but it might be + // freed then recreated while we were waiting for the + // reply. + auto reply = (xcb_get_property_reply_t *)reply_or_error; + wm_tree_set_wm_state(&req->wm->tree, node, reply->type != XCB_NONE); + free(req); +} + +static void wm_import_start_no_flush(struct wm *wm, struct x_connection *c, struct atom *atoms, + xcb_window_t wid, struct wm_tree_node *parent) { + log_debug("Starting import process for window %#010x", wid); + x_set_error_action_ignore( + c, xcb_change_window_attributes(c->c, wid, XCB_CW_EVENT_MASK, + (const uint32_t[]){WM_IMPORT_EV_MASK})); + + // Try to see if any orphaned window has the same window ID, if so, it must + // have been destroyed without us knowing, so we should reuse the node. + auto new = wm_tree_find(&wm->tree, wid); + if (new == NULL) { + new = wm_tree_new_window(&wm->tree, wid); + wm_tree_add_window(&wm->tree, new); + } else { + if (new->parent == parent) { + // What's going on??? + log_error("Importing window %#010x a second time", wid); + assert(false); + return; + } + if (new->parent != &wm->orphan_root) { + log_error("Window %#010x appeared in the children list of both " + "%#010x (previous) and %#010x (current).", + wid, new->parent->id.x, parent->id.x); + assert(false); } + + wm_tree_detach(&wm->tree, new); + // Need to bump the generation number, as `new` is actually an entirely + // new window, just reusing the same window ID. + new->id.gen = wm->tree.gen++; + } + wm_tree_attach(&wm->tree, new, parent); + // In theory, though very very unlikely, a window could be reparented (so we won't + // receive its DestroyNotify), destroyed, and then a new window was created with + // the same window ID, all before the previous query tree request is completed. In + // this case, we shouldn't resend another query tree request. (And we also know in + // this case the previous get property request isn't completed either.) + if (wm_find_pending_query_tree(wm, new) != -1) { + return; } - dynarr_foreach(wm->masked, m) { - free(*m); + { + auto cookie = xcb_query_tree(c->c, wid); + auto req = ccalloc(1, struct wm_query_tree_request); + req->base.callback = wm_handle_query_tree_reply; + req->base.sequence = cookie.sequence; + req->node = new; + req->wm = wm; + req->atoms = atoms; + req->pending_index = dynarr_len(wm->pending_query_trees); + dynarr_push(wm->pending_query_trees, req); + x_await_request(c, &req->base); + } + + // (It's OK to resend the get property request even if one is already in-flight, + // unlike query tree.) + { + auto cookie = + xcb_get_property(c->c, 0, wid, atoms->aWM_STATE, XCB_ATOM_ANY, 0, 2); + auto req = ccalloc(1, struct wm_get_property_request); + req->base.callback = wm_handle_get_wm_state_reply; + req->base.sequence = cookie.sequence; + req->wm = wm; + req->wid = wid; + x_await_request(c, &req->base); } - dynarr_clear_pod(wm->masked); - return true; } -bool wm_has_incomplete_imports(const struct wm *wm) { - return !dynarr_is_empty(wm->incompletes); +void wm_import_start(struct wm *wm, struct x_connection *c, struct atom *atoms, + xcb_window_t wid, struct wm_ref *parent) { + struct wm_tree_node *parent_node = parent != NULL ? to_tree_node_mut(parent) : NULL; + if (parent_node != NULL && wm_find_pending_query_tree(wm, parent_node) != -1) { + // Parent node is currently being queried, we can't attach the new window + // to it as that will change its children list. + return; + } + wm_import_start_no_flush(wm, c, atoms, wid, parent_node); + xcb_flush(c->c); // Actually send the requests +} + +bool wm_is_consistent(const struct wm *wm) { + return dynarr_is_empty(wm->pending_query_trees); } bool wm_has_tree_changes(const struct wm *wm) { @@ -439,3 +547,11 @@ struct wm_change wm_dequeue_change(struct wm *wm) { } return ret; } + +struct wm_ref *wm_new_mock_window(struct wm *wm, xcb_window_t wid) { + auto node = wm_tree_new_window(&wm->tree, wid); + return (struct wm_ref *)&node->siblings; +} +void wm_free_mock_window(struct wm * /*wm*/, struct wm_ref *cursor) { + free(to_tree_node_mut(cursor)); +} diff --git a/src/wm/wm.h b/src/wm/wm.h index 7c8f58f651..7fa9391bb8 100644 --- a/src/wm/wm.h +++ b/src/wm/wm.h @@ -98,6 +98,9 @@ static inline bool wm_treeid_eq(wm_treeid a, wm_treeid b) { return a.gen == b.gen && a.x == b.x; } +/// Create a new window management object. +/// Caller is expected to first call `wm_import_start` with the root window after +/// creating the object. Otherwise many operations will fail or crash. struct wm *wm_new(void); void wm_free(struct wm *wm); @@ -115,12 +118,10 @@ void wm_set_active_leader(struct wm *wm, struct wm_ref *leader); /// Find a window in the hash table from window id. struct wm_ref *attr_pure wm_find(const struct wm *wm, xcb_window_t id); -/// Remove a window from the hash table. -void wm_remove(struct wm *wm, struct wm_ref *w); // Find the WM frame of a client window. `id` is the client window id. struct wm_ref *attr_pure wm_find_by_client(const struct wm *wm, xcb_window_t client); /// Find the toplevel of a window by going up the window tree. -struct wm_ref *attr_pure wm_ref_toplevel_of(struct wm_ref *cursor); +struct wm_ref *attr_pure wm_ref_toplevel_of(const struct wm *wm, struct wm_ref *cursor); /// Return the client window of a window. Must be called with a cursor to a toplevel. /// Returns NULL if there is no client window. struct wm_ref *attr_pure wm_ref_client_of(struct wm_ref *cursor); @@ -154,59 +155,57 @@ void wm_reap_zombie(struct wm_ref *zombie); void wm_reparent(struct wm *wm, xcb_window_t wid, xcb_window_t parent); void wm_set_has_wm_state(struct wm *wm, struct wm_ref *cursor, bool has_wm_state); -/// Create a tree node for `wid`, with `parent` as its parent. The parent must already -/// be in the window tree. This function creates a placeholder tree node, without -/// contacting the X server, thus can be called outside of the X critical section. The -/// expectation is that the caller will later call `wm_complete_import` inside the -/// X critical section to complete the import. +/// Start the import process for `wid`. /// -/// ## NOTE +/// This function sets up event masks for `wid`, and start an async query tree request on +/// it. When the query tree request is completed, `wm_handle_query_tree_reply` will be +/// called to actually insert the window into the window tree. +/// `wm_handle_query_tree_reply` will also in turn start the import process for all +/// children of `wid`. /// -/// The reason for this complicated dance is because we want to catch all windows ever -/// created on X server's side. For a newly created windows, we will setup a -/// SubstructureNotify event mask to catch any new windows created under it. But between -/// the time we received the creation event and the time we setup the event mask, if any -/// windows were created under the new window, we will miss them. Therefore we have to -/// scan the new windows in X critical section so they won't change as we scan. +/// The reason for this two step process is because we want to catch all windows ever +/// created on X server's side. It's not enough to just set up event masks and wait for +/// events. Because at the point in time we set up the event mask, some child windows +/// could have already been created. It would have been nice if there is a way to listen +/// for events on the whole window tree, for all present and future windows. But alas, the +/// X11 protocol is lacking in this area. /// -/// On the other hand, we can't push everything to the X critical section, because -/// updating the window stack requires knowledge of all windows in the stack. Think -/// ConfigureNotify, if we don't know about the `above_sibling` window, we don't know -/// where to put the window. So we need to create an incomplete import first. +/// The best thing we can do is set up the event mask to catch all future events, and then +/// query the current state. But there are complications with this approach, too. Because +/// there is no way to atomically do these two things in one go, things could happen +/// between these two steps, for which we will receive events. Some of these are easy to +/// deal with, e.g. if a window is created, we will get an event for that, and later we +/// will see that window again in the query tree reply. These are easy to ignore. Some are +/// more complex. Because there could be some child windows we are not aware of. We could +/// get events for windows that we don't know about. We try our best to ignore those +/// events. /// -/// But wait, this is actually way more involved. Because we choose to not set up event -/// masks for incomplete imports (we can also choose otherwise, but that also has its own -/// set of problems), there is a whole subtree of windows we don't know about. And those -/// windows might involve in reparent events. To handle that, we essentially put "fog of -/// war" under any incomplete imports, anything reparented into the fog is lost, and will -/// be rediscovered later during subtree scans. If a window is reparented out of the fog, -/// then it's treated as if a brand new window was created. -/// -/// But wait again, there's more. We can delete "lost" windows on our side and unset event -/// masks, but again because this is racy, we might still receive some events for those -/// windows. So we have to keep a list of "lost" windows, and correctly ignore events sent -/// for them. (And since we are actively ignoring events from these windows, we might as -/// well not unset their event masks, saves us a trip to the X server). +/// Another problem with this is, the usual way we send a request then process the reply +/// is synchronous. i.e. with `xcb__reply(xcb_(...))`. As previously +/// mentioned, we might receive events before the reply. And in this case we would process +/// the reply _before_ any of those events! This might be benign, but it is difficult to +/// audit all the possible cases and events to make sure this always work. One example is, +/// imagine a window A is being imported, and another window B, which is already in the +/// tree got reparented to A. We would think B appeared in two places if we processed +/// query tree reply before the reparent event. For that, we introduce the concept of +/// "async requests". Replies to these requests are received and processed like other X +/// events. With that, we don't need to worry about the order of events and replies. /// /// (Now you have a glimpse of how much X11 sucks.) -void wm_import_incomplete(struct wm *wm, xcb_window_t wid, xcb_window_t parent); - -/// Check if there are any incomplete imports in the window tree. -bool wm_has_incomplete_imports(const struct wm *wm); +void wm_import_start(struct wm *wm, struct x_connection *c, struct atom *atoms, + xcb_window_t wid, struct wm_ref *parent); /// Check if there are tree change events bool wm_has_tree_changes(const struct wm *wm); -/// Complete the previous incomplete imports by querying the X server. This function will -/// recursively import all children of previously created placeholders, and add them to -/// the window tree. This function must be called from the X critical section. This -/// function also subscribes to SubstructureNotify events for all newly imported windows, -/// as well as for the (now completed) placeholder windows. -/// -/// Returns false if our internal state is behind the X server's. In other words, if a -/// window we think should be there, does not in fact exist. Returns true otherwise. -bool wm_complete_import(struct wm *wm, struct x_connection *c, struct atom *atoms); +struct wm_change wm_dequeue_change(struct wm *wm); -bool wm_is_wid_masked(struct wm *wm, xcb_window_t wid); +/// Whether the window tree should be in a consistent state. When the window tree is +/// consistent, we should not be receiving X events that refer to windows that we don't +/// know about. And we also should not have any orphaned windows in the tree. +bool wm_is_consistent(const struct wm *wm); -struct wm_change wm_dequeue_change(struct wm *wm); +// Unit testing helpers + +struct wm_ref *wm_new_mock_window(struct wm *wm, xcb_window_t wid); +void wm_free_mock_window(struct wm *wm, struct wm_ref *cursor); diff --git a/src/wm/wm_internal.h b/src/wm/wm_internal.h index 9e33808a9b..f9a8c9ffb7 100644 --- a/src/wm/wm_internal.h +++ b/src/wm/wm_internal.h @@ -1,6 +1,5 @@ #pragma once -#include #include #include @@ -24,6 +23,7 @@ struct wm_tree { uint64_t gen; /// wm tree nodes indexed by their X window ID. struct wm_tree_node *nodes; + struct wm_tree_node *root; struct list_node changes; struct list_node free_changes; @@ -77,7 +77,10 @@ struct wm_tree_change { /// shutting down. void wm_tree_clear(struct wm_tree *tree); struct wm_tree_node *attr_pure wm_tree_find(const struct wm_tree *tree, xcb_window_t id); -struct wm_tree_node *attr_pure wm_tree_find_toplevel_for(struct wm_tree_node *node); +/// Find the toplevel that is an ancestor of `node` or `node` itself. Returns NULL if +/// `node` is part of an orphaned subtree. +struct wm_tree_node *attr_pure wm_tree_find_toplevel_for(const struct wm_tree *tree, + struct wm_tree_node *node); struct wm_tree_node *attr_pure wm_tree_next(struct wm_tree_node *node, struct wm_tree_node *subroot); /// Create a new window node in the tree, with X window ID `id`, and parent `parent`. If @@ -85,15 +88,17 @@ struct wm_tree_node *attr_pure wm_tree_next(struct wm_tree_node *node, /// permitted, and the root window cannot be destroyed once created, until /// `wm_tree_clear` is called. If `parent` is not NULL, the new node will be put at the /// top of the stacking order among its siblings. -struct wm_tree_node * -wm_tree_new_window(struct wm_tree *tree, xcb_window_t id, struct wm_tree_node *parent); +struct wm_tree_node *wm_tree_new_window(struct wm_tree *tree, xcb_window_t id); +void wm_tree_add_window(struct wm_tree *tree, struct wm_tree_node *node); void wm_tree_destroy_window(struct wm_tree *tree, struct wm_tree_node *node); /// Detach the subtree rooted at `subroot` from `tree`. The subtree root is removed from /// its parent, and the disconnected tree nodes won't be able to be found via /// `wm_tree_find`. Relevant events will be generated. void wm_tree_detach(struct wm_tree *tree, struct wm_tree_node *subroot); -void wm_tree_reparent(struct wm_tree *tree, struct wm_tree_node *node, - struct wm_tree_node *new_parent); +/// Attach `node` to `parent`. `node` becomes the topmost child of `parent`. If `parent` +/// is NULL, `node` becomes the root window. +void wm_tree_attach(struct wm_tree *tree, struct wm_tree_node *child, + struct wm_tree_node *parent); void wm_tree_move_to_above(struct wm_tree *tree, struct wm_tree_node *node, struct wm_tree_node *other); /// Move `node` to the top or the bottom of its parent's child window stack. @@ -104,6 +109,7 @@ void wm_tree_set_wm_state(struct wm_tree *tree, struct wm_tree_node *node, bool static inline void wm_tree_init(struct wm_tree *tree) { tree->nodes = NULL; + tree->gen = 1; list_init_head(&tree->changes); list_init_head(&tree->free_changes); } diff --git a/src/x.c b/src/x.c index 256ba42ba7..6291306980 100644 --- a/src/x.c +++ b/src/x.c @@ -18,6 +18,8 @@ #include #include #include +#include +#include #include #include "atom.h" @@ -31,26 +33,24 @@ // === Error handling === -/** - * Xlib error handler function. - */ -static int xerror(Display attr_unused *dpy, XErrorEvent *ev) { - if (!ps_g) { - // Do not ignore errors until the session has been initialized - return 0; +/// Discard pending error handlers. +/// +/// We have received reply with sequence number `sequence`, which means all pending +/// replies with sequence number strictly less than `sequence` will never be received. So +/// discard them. +static void x_discard_pending_errors(struct x_connection *c, uint32_t sequence) { + if (sequence < c->last_sequence) { + // Overflown, drop from `pending_x_errors` until its sequence number + // decreases. + log_debug("X sequence number overflown, %u -> %u", c->last_sequence, sequence); + list_foreach_safe(struct pending_x_error, i, &c->pending_x_errors, siblings) { + if (sequence >= i->sequence) { + break; + } + list_remove(&i->siblings); + free(i); + } } - - // Fake a xcb error, fill in just enough information - xcb_generic_error_t xcb_err; - xcb_err.full_sequence = (uint32_t)ev->serial; - xcb_err.major_code = ev->request_code; - xcb_err.minor_code = ev->minor_code; - xcb_err.error_code = ev->error_code; - x_handle_error(&ps_g->c, &xcb_err); - return 0; -} - -void x_discard_pending_errors(struct x_connection *c, uint32_t sequence) { list_foreach_safe(struct pending_x_error, i, &c->pending_x_errors, siblings) { if (sequence <= i->sequence) { break; @@ -168,7 +168,7 @@ void x_print_error_impl(unsigned long serial, uint8_t major, uint16_t minor, /// Handle X errors. /// /// This function logs X errors, or aborts the program based on severity of the error. -void x_handle_error(struct x_connection *c, xcb_generic_error_t *ev) { +static void x_handle_error(struct x_connection *c, xcb_generic_error_t *ev) { x_discard_pending_errors(c, ev->full_sequence); struct pending_x_error *first_error_action = NULL; if (!list_is_empty(&c->pending_x_errors)) { @@ -204,6 +204,25 @@ void x_handle_error(struct x_connection *c, xcb_generic_error_t *ev) { ev->error_code)); } +/** + * Xlib error handler function. + */ +static int xerror(Display attr_unused *dpy, XErrorEvent *ev) { + if (!ps_g) { + // Do not ignore errors until the session has been initialized + return 0; + } + + // Fake a xcb error, fill in just enough information + xcb_generic_error_t xcb_err; + xcb_err.full_sequence = (uint32_t)ev->serial; + xcb_err.major_code = ev->request_code; + xcb_err.minor_code = ev->minor_code; + xcb_err.error_code = ev->error_code; + x_handle_error(&ps_g->c, &xcb_err); + return 0; +} + /// Initialize x_connection struct from an Xlib Display. /// /// Note this function doesn't take ownership of the Display, the caller is still @@ -212,10 +231,17 @@ void x_connection_init(struct x_connection *c, Display *dpy) { c->dpy = dpy; c->c = XGetXCBConnection(dpy); list_init_head(&c->pending_x_errors); + list_init_head(&c->pending_x_requests); c->previous_xerror_handler = XSetErrorHandler(xerror); c->screen = DefaultScreen(dpy); c->screen_info = xcb_aux_get_screen(c->c, c->screen); + c->message_on_hold = NULL; + + // Do a round trip to fetch the current sequence number + auto cookie = xcb_get_input_focus(c->c); + free(xcb_get_input_focus_reply(c->c, cookie, NULL)); + c->last_sequence = cookie.sequence; } /** @@ -909,3 +935,167 @@ void x_free_monitor_info(struct x_monitors *m) { } m->count = 0; } + +static uint32_t x_get_full_sequence(struct x_connection *c, uint16_t sequence) { + auto last_sequence_low = c->last_sequence & 0xffff; + // sequence < last_sequence16 means the lower 16 bits overflowed, which should + // carry to the higher 16 bits + auto sequence_high = c->last_sequence & 0xffff0000; + if (sequence < last_sequence_low) { + sequence_high += 0x10000; + } + return sequence_high | sequence; +} + +static int64_t x_compare_sequence(struct x_connection *c, uint32_t a, uint32_t b) { + bool a_overflown = a < c->last_sequence, b_overflown = b < c->last_sequence; + if (a_overflown == b_overflown) { + return (int64_t)a - (int64_t)b; + } + return a_overflown ? 1 : -1; +} + +static xcb_raw_generic_event_t * +x_poll_for_event_impl(struct x_connection *c, struct x_async_request_base **out_req) { + struct x_async_request_base *first_pending_request = NULL; + if (!list_is_empty(&c->pending_x_requests)) { + first_pending_request = list_entry(c->pending_x_requests.next, + struct x_async_request_base, siblings); + } + + bool on_hold_is_reply; + if (c->message_on_hold == NULL) { + // Nothing on hold, we need to read new information from the X connection. + // We must only read from the X connection once in this function to keep + // things consistent. The only way to do that is reading the connection + // with `xcb_poll_for_reply`, and then check for events with + // `xcb_poll_for_queued_event`, because there is no + // `xcb_poll_for_queued_reply`. Unless we are not waiting for any replies, + // in which case a simple `xcb_poll_for_event` is enough. + if (first_pending_request != NULL) { + xcb_generic_error_t *err = NULL; + on_hold_is_reply = + xcb_poll_for_reply(c->c, first_pending_request->sequence, + (void **)&c->message_on_hold, &err) == 1; + if (err != NULL) { + c->message_on_hold = (xcb_raw_generic_event_t *)err; + } + if (!on_hold_is_reply) { + // We didn't get a reply, but did we get an event? + c->message_on_hold = + (xcb_raw_generic_event_t *)xcb_poll_for_queued_event(c->c); + } + } else { + c->message_on_hold = + (xcb_raw_generic_event_t *)xcb_poll_for_event(c->c); + on_hold_is_reply = false; + } + } else if (first_pending_request != NULL) { + // response_type 0 is error, 1 is reply. + on_hold_is_reply = c->message_on_hold->response_type < 2 && + x_get_full_sequence(c, c->message_on_hold->sequence) == + first_pending_request->sequence; + } else { + on_hold_is_reply = false; + } + if (c->message_on_hold == NULL) { + // Didn't get any new information from the X connection, nothing to + // return. + return NULL; + } + + // From this point, no more reading from the X connection is allowed. + xcb_generic_event_t *next_event = NULL; + if (on_hold_is_reply) { + next_event = xcb_poll_for_queued_event(c->c); + assert(next_event == NULL || next_event->response_type != 1); + } else { + next_event = (xcb_generic_event_t *)c->message_on_hold; + } + + // `next_event == c->message_on_hold` iff `on_hold_is_reply` is false. + + bool should_return_event = false; + if (first_pending_request == NULL) { + // Here `on_hold_is_reply` must be false, therefore `next_event == + // c->message_on_hold` must be true, therefore `next_event` cannot be + // NULL. + should_return_event = true; + } else if (next_event != NULL) { + auto ordering = x_compare_sequence(c, next_event->full_sequence, + first_pending_request->sequence); + // If next_event is a true event, it might share a sequence number with a + // reply. But if it's an error (i.e. response_type == 0), its sequence + // number must be different from any reply. + assert(next_event->response_type != 0 || ordering != 0); + should_return_event = ordering < 0; + } + + if (should_return_event) { + x_discard_pending_errors(c, next_event->full_sequence); + c->last_sequence = next_event->full_sequence; + if (!on_hold_is_reply) { + c->message_on_hold = NULL; + } + return (xcb_raw_generic_event_t *)next_event; + } + + // We should return the reply to the first pending request. + xcb_raw_generic_event_t *ret = NULL; + if (!on_hold_is_reply) { + xcb_generic_error_t *err = NULL; + // This is a very special case. Because we have already received an event + // with a greater or equal sequence number than the reply, we _know_ the + // reply must also have already arrived. We can safely call + // `xcb_poll_for_reply` here because we know it will not read from the X + // connection again. + BUG_ON(xcb_poll_for_reply(c->c, first_pending_request->sequence, + (void **)&ret, &err) == 0); + if (err != NULL) { + ret = (xcb_raw_generic_event_t *)err; + } + } else { + ret = c->message_on_hold; + c->message_on_hold = (xcb_raw_generic_event_t *)next_event; + } + + x_discard_pending_errors(c, first_pending_request->sequence + 1); + c->last_sequence = first_pending_request->sequence; + *out_req = first_pending_request; + list_remove(&first_pending_request->siblings); + return ret; +} + +xcb_generic_event_t *x_poll_for_event(struct x_connection *c) { + xcb_raw_generic_event_t *ret = NULL; + while (true) { + struct x_async_request_base *req = NULL; + ret = x_poll_for_event_impl(c, &req); + if (ret == NULL) { + break; + } + + if (req != NULL) { + req->callback(c, req, ret); + } else if (ret->response_type == 0) { + x_handle_error(c, (xcb_generic_error_t *)ret); + } else { + break; + } + free(ret); + } + return (xcb_generic_event_t *)ret; +} + +void x_cancel_request(struct x_connection *c, struct x_async_request_base *req) { + list_remove(&req->siblings); + if (c->message_on_hold == NULL) { + return; + } + if (c->message_on_hold->response_type >= 2 || + x_get_full_sequence(c, c->message_on_hold->sequence) != req->sequence) { + return; + } + free(c->message_on_hold); + c->message_on_hold = NULL; +} diff --git a/src/x.h b/src/x.h index caaf4b4e65..6afc5ddcab 100644 --- a/src/x.h +++ b/src/x.h @@ -78,6 +78,17 @@ struct x_connection { // Private fields /// The error handling list. struct list_node pending_x_errors; + /// The list of pending async requests that we have + /// yet to receive a reply for. + struct list_node pending_x_requests; + /// A message, either an event or a reply, that is currently being held, because + /// there are messages of the opposite type with lower sequence numbers that we + /// need to return first. + xcb_raw_generic_event_t *message_on_hold; + /// The sequence number of the last message returned by + /// `x_poll_for_message`. Used for sequence number overflow + /// detection. + uint32_t last_sequence; /// Previous handler of X errors XErrorHandler previous_xerror_handler; /// Information about the default screen @@ -178,11 +189,25 @@ x_set_error_action(struct x_connection *c, uint32_t sequence, enum x_error_actio ((void)(cookie)) #endif +struct x_async_request_base { + struct list_node siblings; + /// The callback function to call when the reply is received. If `reply_or_error` + /// is NULL, it means the X connection is closed while waiting for the reply. + void (*callback)(struct x_connection *, struct x_async_request_base *, + xcb_raw_generic_event_t *reply_or_error); + /// The sequence number of the X request. + unsigned int sequence; +}; + static inline void attr_unused free_x_connection(struct x_connection *c) { list_foreach_safe(struct pending_x_error, i, &c->pending_x_errors, siblings) { list_remove(&i->siblings); free(i); } + list_foreach_safe(struct x_async_request_base, i, &c->pending_x_requests, siblings) { + list_remove(&i->siblings); + i->callback(c, i, NULL); + } XSetErrorHandler(c->previous_xerror_handler); } @@ -193,18 +218,6 @@ static inline void attr_unused free_x_connection(struct x_connection *c) { /// responsible for closing it after `free_x_connection` is called. void x_connection_init(struct x_connection *c, Display *dpy); -/// Discard queued pending replies. -/// -/// We have received reply with sequence number `sequence`, which means all pending -/// replies with sequence number less than `sequence` will never be received. So discard -/// them. -void x_discard_pending_errors(struct x_connection *c, uint32_t sequence); - -/// Handle X errors. -/// -/// This function logs X errors, or aborts the program based on severity of the error. -void x_handle_error(struct x_connection *c, xcb_generic_error_t *ev); - /** * Get a specific attribute of a window. * @@ -415,3 +428,24 @@ uint32_t attr_deprecated xcb_generate_id(xcb_connection_t *c); /// Ask X server to send us a notification for the next end of vblank. void x_request_vblank_event(struct x_connection *c, xcb_window_t window, uint64_t msc); + +/// Register an X request as async request. Its reply will be processed as part of the +/// event stream. i.e. the registered callback will only be called when all preceding +/// events have been retrieved via `x_poll_for_event`. +/// `req` store information about the request, including the callback. The callback is +/// responsible for freeing `req`. +static inline void x_await_request(struct x_connection *c, struct x_async_request_base *req) { + list_insert_before(&c->pending_x_requests, &req->siblings); +} + +/// Cancel an async request. +void x_cancel_request(struct x_connection *c, struct x_async_request_base *req); + +/// Poll for the next X event. This is like `xcb_poll_for_event`, but also includes +/// machinery for handling async replies. Calling `xcb_poll_for_event` directly will +/// cause replies to async requests to be lost, so that should never be called. +xcb_generic_event_t *x_poll_for_event(struct x_connection *c); + +static inline bool x_has_pending_requests(struct x_connection *c) { + return !list_is_empty(&c->pending_x_requests); +}