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

@@ -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.
+	/* must open a new display or the RetainPermanent will
-	 * RetainTemporary (not RetainPermanent) is used intentionally:
+	 * leave stuff allocated in RContext unallocated after exit */
-	 * 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"). */
 	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;
+	static Atom eprop = 0;
-	int format;
+	Pixmap old_root, old_eset;
-	unsigned long length, after;
+	Bool have_root, have_eset;
-	unsigned char *data;
 
 	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
+	/* Free the previous background pixmap only when both _XROOTPMAP_ID
-	 * XKillClient() on the old pixmap: since duplicatePixmap() uses
+	 * and ESETROOT_PMAP_ID hold the same XID, that pairing marks a
-	 * RetainTemporary, old pixmaps are freed automatically when the
+	 * RetainPermanent zombie left by a compliant root setter so
-	 * X session ends.  Calling XKillClient() here was unsafe because
+	 * XKillClient on it cannot hit a live connection.  A bare
-	 * the X server reuses client IDs; if the stored resource ID was
+	 * _XROOTPMAP_ID may come from a tool that closed in Destroy mode,
-	 * reassigned to our own connection the server would kill us,
+	 * in which case the client bits of the stored XID can already be
-	 * producing "X connection to :0 broken". */
+	 * recycled to a live client, including our own dpy, and killing
-	XGetWindowProperty(dpy, root, prop, 0L, 1L, False, AnyPropertyType,
+	 * it produces "X connection broken". */
-			   &type, &format, &length, &after, &data);
+	have_root = getRootPixmapAtom(prop, &old_root);
-	if (data)
+	have_eset = getRootPixmapAtom(eprop, &old_eset);
-		XFree(data);
+	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);