From 774e6962702ec0666f155ed8320f5c9e36591858 Mon Sep 17 00:00:00 2001 From: David Maciejak Date: Mon, 15 Jun 2026 21:40:03 -0400 Subject: [PATCH] wmsetbg: free the previous root pixmap via the ESETROOT pairing Commit 6c107e0c switched duplicatePixmap() to RetainTemporary and removed XKillClient() from setPixmapProperty(). RetainTemporary resources are released only on server reset, so each background change now leaves a full-screen pixmap retained in the X server until logout. In helper mode that is one leak per workspace switch. --- util/wmsetbg.c | 82 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 58 insertions(+), 24 deletions(-) diff --git a/util/wmsetbg.c b/util/wmsetbg.c index 4f697d49..a7e59872 100644 --- a/util/wmsetbg.c +++ b/util/wmsetbg.c @@ -781,12 +781,8 @@ static Pixmap duplicatePixmap(Pixmap pixmap, int width, int height) Display *tmpDpy; Pixmap copyP; - /* Open a separate connection so the pixmap survives our exit. - * RetainTemporary (not RetainPermanent) is used intentionally: - * the pixmap lives until the X session ends, avoiding the need - * for XKillClient() on old pixmaps (which can crash when the - * X server reuses a client ID that now belongs to our own - * connection, causing "X connection broken"). */ + /* must open a new display or the RetainPermanent will + * leave stuff allocated in RContext unallocated after exit */ tmpDpy = XOpenDisplay(display); if (!tmpDpy) { wwarning("could not open display to update background image information"); @@ -799,45 +795,83 @@ static Pixmap duplicatePixmap(Pixmap pixmap, int width, int height) XCopyArea(tmpDpy, pixmap, copyP, DefaultGC(tmpDpy, scr), 0, 0, width, height, 0, 0); XSync(tmpDpy, False); - XSetCloseDownMode(tmpDpy, RetainTemporary); + XSetCloseDownMode(tmpDpy, RetainPermanent); XCloseDisplay(tmpDpy); } return copyP; } +static int dummyErrorHandler(Display * dpy, XErrorEvent * err) +{ + /* Parameter not used, but tell the compiler that it is ok */ + (void) dpy; + (void) err; + + return 0; +} + +static Bool getRootPixmapAtom(Atom prop, Pixmap *ret) +{ + Atom type; + int format; + unsigned long length, after; + unsigned char *data = NULL; + + *ret = None; + if (XGetWindowProperty(dpy, root, prop, 0L, 1L, False, AnyPropertyType, + &type, &format, &length, &after, &data) != Success) + return False; + if (type == XA_PIXMAP && format == 32 && length == 1 && data) { + *ret = *(Pixmap *)data; + XFree(data); + return True; + } + if (data) + XFree(data); + return False; +} static void setPixmapProperty(Pixmap pixmap) { static Atom prop = 0; - Atom type; - int format; - unsigned long length, after; - unsigned char *data; + static Atom eprop = 0; + Pixmap old_root, old_eset; + Bool have_root, have_eset; if (!prop) { prop = XInternAtom(dpy, "_XROOTPMAP_ID", False); + eprop = XInternAtom(dpy, "ESETROOT_PMAP_ID", False); } XGrabServer(dpy); - /* Read and discard the old property data. We no longer call - * XKillClient() on the old pixmap: since duplicatePixmap() uses - * RetainTemporary, old pixmaps are freed automatically when the - * X session ends. Calling XKillClient() here was unsafe because - * the X server reuses client IDs; if the stored resource ID was - * reassigned to our own connection the server would kill us, - * producing "X connection to :0 broken". */ - XGetWindowProperty(dpy, root, prop, 0L, 1L, False, AnyPropertyType, - &type, &format, &length, &after, &data); - if (data) - XFree(data); + /* Free the previous background pixmap only when both _XROOTPMAP_ID + * and ESETROOT_PMAP_ID hold the same XID, that pairing marks a + * RetainPermanent zombie left by a compliant root setter so + * XKillClient on it cannot hit a live connection. A bare + * _XROOTPMAP_ID may come from a tool that closed in Destroy mode, + * in which case the client bits of the stored XID can already be + * recycled to a live client, including our own dpy, and killing + * it produces "X connection broken". */ + have_root = getRootPixmapAtom(prop, &old_root); + have_eset = getRootPixmapAtom(eprop, &old_eset); + if (have_root && have_eset && old_root == old_eset && old_eset != None) { + XSetErrorHandler(dummyErrorHandler); + XKillClient(dpy, old_eset); + XSync(dpy, False); + XSetErrorHandler(NULL); + } - if (pixmap) + if (pixmap) { XChangeProperty(dpy, root, prop, XA_PIXMAP, 32, PropModeReplace, (unsigned char *)&pixmap, 1); - else + XChangeProperty(dpy, root, eprop, XA_PIXMAP, 32, PropModeReplace, + (unsigned char *)&pixmap, 1); + } else { XDeleteProperty(dpy, root, prop); + XDeleteProperty(dpy, root, eprop); + } XUngrabServer(dpy); XFlush(dpy);