IPC patch "memory leak"

[bakkeby]

I am raising this for reference in case someone else should end up complaining about the same issue. Feel free to close.

I am referring to "memory leak" in quotes as the IPC code itself appears to be rock solid.

How to replicate

1. apply the IPC patch on a bare dwm
2. start dwm and spawn a few windows / terminals
3. monitor memory usage of the dwm process

$ watch -n 0.5 "ps -q $(pidof dwm) aux"

(note the RSS value)

4. spam run commands via dwm-msg

$ for I in $(seq 9999); do dwm-msg run_command focusstack 1 2>&1 >/dev/null; done

It doesn't really matter which command is being run, as long as it ultimately results in the bar being updated (which should apply to most of them).

5. observe the RSS value increasing

The expected behaviour is of course that the memory does not increase.

Cause

The source of the leak has been tracked down to this function in drw.c:

void
drw_map(Drw *drw, Window win, int x, int y, unsigned int w, unsigned int h)
{
    if (!drw)
        return;

    XCopyArea(drw->dpy, drw->drawable, win, drw->gc, x, y, w, h, x, y);
    XSync(drw->dpy, False);
}

This 20 year old post suggests that performing a series of XCopyArea operations without calling XNextEvent will result in a memory leak: https://root.cern.ch/root/roottalk/roottalk01/1040.html

The XSync call flushes the output buffer and then waits until all requests have been received and processed by the X server.

The XNextEvent just returns the next event in the queue.

How is this related to the IPC patch?

I believe that this has to do with that the IPC patch allows for triggering updates that are outside of the event loop, which means that it is possible to trigger more than one dwm-msg run_command call between X server events happening.

This way we can end up making multiple calls to drawbar > drw_map > XCopyArea before a call to XNextEvent is made, resulting in the obscure memory leak within the X libraries.

Solution

I found that simply removing the XSync call solves the issue, but if the flushing of the output buffer is necessary for whatever reason (I could not find any side effect of not including it) then XFlush can be used instead.

 void
 drw_map(Drw *drw, Window win, int x, int y, unsigned int w, unsigned int h)
 {
     if (!drw)
         return;

     XCopyArea(drw->dpy, drw->drawable, win, drw->gc, x, y, w, h, x, y);
-    XSync(drw->dpy, False);
+    XFlush(drw->dpy);
 }

Due to the nature of the issue I do not think the above change should be part of the IPC patch itself.

As such I am leaving this here as reading material for those particularly interested.