From 10ba9600f693cfc592d0987923820a9d213fc28a Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Thu, 13 Mar 2025 18:06:27 +0000 Subject: [PATCH] misc: add a xcb patch to help detect freeze conditions Patch xcb to expose the internal buffer of a xcb_connection_t, so picom can assert that it's empty before it goes to sleep. With nix its easy to setup a dev environment that builds libxcb with this patch. Also rename the `useClangProfile` devShell to `useClangDebuggable` which I think is a better name. (Pretty sure I am the only person that uses this anyways). Signed-off-by: Yuxuan Shui --- flake.nix | 15 ++++- nix/patches/xcb-add-debug-functions.patch | 82 +++++++++++++++++++++++ src/meson.build | 4 ++ src/picom.c | 24 +++++++ 4 files changed, 122 insertions(+), 3 deletions(-) create mode 100644 nix/patches/xcb-add-debug-functions.patch diff --git a/flake.nix b/flake.nix index aac76c78..f15b049f 100644 --- a/flake.nix +++ b/flake.nix @@ -26,9 +26,18 @@ inherit system overlays; config.allowBroken = true; }; - profilePkgs = import nixpkgs { + debuggablePkgs = import nixpkgs { inherit system; overlays = overlays ++ [ + (final: prev: { + xorg = prev.xorg.overrideScope (_: xprev: { + libxcb = final.enableDebugging (xprev.libxcb.overrideAttrs (o: { + patches = (o.patches or []) ++ [ + ./nix/patches/xcb-add-debug-functions.patch + ]; + })); + }); + }) (final: prev: { stdenv = prev.withCFlags "-fno-omit-frame-pointer" prev.stdenv; }) @@ -75,8 +84,8 @@ }; # build picom and all dependencies with frame pointer, making profiling/debugging easier. # WARNING! many many rebuilds - devShells.useClangProfile = (mkDevShell (profilePkgs.picom.override { devShell = true; })).override { - stdenv = profilePkgs.withCFlags "-fno-omit-frame-pointer" profilePkgs.llvmPackages_18.stdenv; + devShells.useClangDebuggable = (mkDevShell (debuggablePkgs.picom.override { devShell = true; })).override { + stdenv = debuggablePkgs.withCFlags "-fno-omit-frame-pointer" debuggablePkgs.llvmPackages_18.stdenv; }; }); } diff --git a/nix/patches/xcb-add-debug-functions.patch b/nix/patches/xcb-add-debug-functions.patch new file mode 100644 index 00000000..3d7e1fd2 --- /dev/null +++ b/nix/patches/xcb-add-debug-functions.patch @@ -0,0 +1,82 @@ +diff --git i/src/xcb.h w/src/xcb.h +index 8d1b456..f37561d 100644 +--- i/src/xcb.h ++++ w/src/xcb.h +@@ -386,6 +386,11 @@ xcb_special_event_t *xcb_register_for_special_xge(xcb_connection_t *c, + void xcb_unregister_for_special_event(xcb_connection_t *c, + xcb_special_event_t *se); + ++/** ++ * @brief Returns an arbitrary message from the connection's internal buffer if any. ++ */ ++xcb_generic_event_t *xcb_picom_peek_message(xcb_connection_t *c); ++ + /** + * @brief Return the error for a request, or NULL if none can ever arrive. + * @param c The connection to the X server. +diff --git i/src/xcb_in.c w/src/xcb_in.c +index 62526f6..7c13022 100644 +--- i/src/xcb_in.c ++++ w/src/xcb_in.c +@@ -25,6 +25,7 @@ + + /* Stuff that reads stuff from the server. */ + ++#include + #ifdef HAVE_CONFIG_H + #include "config.h" + #endif +@@ -913,6 +914,25 @@ xcb_unregister_for_special_event(xcb_connection_t *c, + pthread_mutex_unlock(&c->iolock); + } + ++xcb_generic_event_t *xcb_picom_peek_message(xcb_connection_t *c) { ++ if (c->in.events) { ++ return c->in.events->event; ++ } ++ if (c->in.current_reply) { ++ return c->in.current_reply->reply; ++ } ++ if (_xcb_map_head(c->in.replies)) { ++ struct reply_list *list = _xcb_map_head(c->in.replies); ++ return list->reply; ++ } ++ for (xcb_special_event_t *s, **prev = &c->in.special_events; (s = *prev) != NULL; prev = &(s->next)) { ++ if (s->events) { ++ return s->events->event; ++ } ++ } ++ return NULL; ++} ++ + /* Private interface */ + + int _xcb_in_init(_xcb_in *in) +diff --git i/src/xcb_list.c w/src/xcb_list.c +index bdd2d43..9dfaaac 100644 +--- i/src/xcb_list.c ++++ w/src/xcb_list.c +@@ -103,3 +103,11 @@ void *_xcb_map_remove(_xcb_map *list, uint64_t key) + } + return 0; + } ++ ++void *_xcb_map_head(_xcb_map *list) { ++ if (!list || !list->head) { ++ return NULL; ++ } ++ return list->head->data; ++} ++ +diff --git i/src/xcbint.h w/src/xcbint.h +index 9836def..ba2d27c 100644 +--- i/src/xcbint.h ++++ w/src/xcbint.h +@@ -85,6 +85,7 @@ _xcb_map *_xcb_map_new(void); + void _xcb_map_delete(_xcb_map *q, xcb_list_free_func_t do_free); + int _xcb_map_put(_xcb_map *q, uint64_t key, void *data); + void *_xcb_map_remove(_xcb_map *q, uint64_t key); ++void *_xcb_map_head(_xcb_map *list); + + + /* xcb_out.c */ diff --git a/src/meson.build b/src/meson.build index 873eedc2..bdf66385 100644 --- a/src/meson.build +++ b/src/meson.build @@ -57,6 +57,10 @@ foreach i : required_xcb_packages base_deps += [dependency(i, version: '>=1.12.0', required: true)] endforeach +if cc.has_function('xcb_picom_peek_message', prefix: '#include ', dependencies: base_deps) + cflags += ['-DHAS_XCB_PICOM_DEBUG_FUNCTIONS'] +endif + libconfig_dep = dependency('libconfig', version: '>=1.7', required: false) if not libconfig_dep.found() diff --git a/src/picom.c b/src/picom.c index 1b0595d1..4056f755 100644 --- a/src/picom.c +++ b/src/picom.c @@ -1281,6 +1281,28 @@ static void unredirect(session_t *ps) { log_debug("Screen unredirected."); } +/// Returns true if there is no queued messages in xcb_connection_t's buffer. +static bool check_xcb_buffer_is_empty(xcb_connection_t *c) { +#ifdef HAS_XCB_PICOM_DEBUG_FUNCTIONS + xcb_generic_event_t *buffered = xcb_picom_peek_message(c); + if (buffered != NULL) { + if (buffered->response_type != 1) { + // not a reply, full_sequence is valid + log_fatal("Going into sleep with messages in connection's " + "buffer, seq: %x", + buffered->full_sequence); + } else { + log_fatal("Going into sleep with replies in connection's buffer, " + "seq: %x", + buffered->sequence); + } + + return false; + } +#endif + return true; +} + /// Handle queued events before we go to sleep. /// /// This function is called by ev_prepare watcher, which is called just before @@ -1333,6 +1355,8 @@ static void handle_x_events(struct session *ps) { ps->pending_updates = true; queue_redraw(ps); } + + assert(check_xcb_buffer_is_empty(ps->c.c)); } static void handle_x_events_ev(EV_P attr_unused, ev_prepare *w, int revents attr_unused) {