Remove dead subscribers early

Fix bug in which "dead" subscribers would not be removed right away and would
consume resources *and* file descriptors which could lead to the bspwm process
running out of available file descriptors and not being able to accept new
incoming connections.

When a process subscribes to bspwm events (i.e. `bspc subscribe <event>`), bspwm
keeps some state about this subscriber such as the file descriptor in which to
write the events and which events are requested by the subscriber.

When a subscriber dies (i.e. the `bspc subscribe ...` process is killed) bspwm
does not know it right away. Instead, on the next event being logged, if bspwm
encounters an error when trying to write() into the file descriptor associated
to a subscriber bspwm will consider this subscriber dead and remove it from the
linked list (which will also close() the file descriptor of the subscriber).
This is done in the put_status() function defined in subscribe.c.

This means that bspwm keeps file descriptors on potentially closed streams for
some time.
Normally this is ok, because events are generated often (when focusing nodes,
switching desktop, etc ...), therefore those "dead" file descriptors are not
kept around for too long.
However, during a period of inactivity (i.e. the user is not actively using the
computer or never switching nodes or desktops) those file descriptors can be
kept for a long time.
This is especially problematic in the following scenario:
    1. User leaves the machine idle.
    2. A "churn" of subscribers happens, where subscribers come and go.
    3. Since the machine is idle, no events are happening and the file
       descriptor of all dead subscribers are not closed.
    4. bspwm consumes more and more file descriptors as new subscribers are
       created.
    5. At some point bspwm has too many opened file descriptors and cannot
       accepts new incoming connections. The accept() syscall in the mainloop
       fails (returning EMFILE) and bspwm is stuck in an infinite loop, not
       being able to serve new request. At this point bspwm needs to be killed
       and restarted.

While this could be deemed as unlikely (the maximum number of opened files being
1024 AFAIK (Arch Linux)), it turns out that this exact scenario can happen with
polybar (https://github.com/polybar/polybar) and the bspwm module. When a
laptop connected to an external monitor becomes idle, and the screen saver kicks
in, polybar periodically reloads itself (because of xrandr events) and therefore
periodically kill and re-create a subscriber. Since there is no activity on the
bspwm process, all dead instances of the polybar subscriber are not removed and
bspwm eventually runs out of file descriptors and gets stuck in the infinite
loop while consuming 100% cpu.
(Note: This is not a polybar issue, polybar does the right thing here).

The fix is as follows: In the main loop of bspwm, check for any dead subscribers
and remove them (if any). This is accomplished by a new function declared in
subscribe.h: prune_dead_subscribers().
prune_dead_subscribers() will iterate through the linked list of subscribers and
for each of them will try to call write() on their file descriptor with an empty
buffer (i.e. size == 0). If the write() fails (returns -1) then the subscriber
is declared dead and this function can remove it from the list and close the
associated file descriptor.

diff --git a/src/bspwm.c b/src/bspwm.c
index cda9482e9b2b56f290cf946088d119e33f25bcbb..88c48e94b99671bd21914739ab0af9a54cd1d1ac 100644 (file)
--- a/src/bspwm.c
+++ b/src/bspwm.c
@@ -215,6 +215,8 @@ int main(int argc, char *argv[])
                if (!check_connection(dpy)) {
                        running = false;
                }
+
+               prune_dead_subscribers();
        }
 
        if (restart) {

diff --git a/src/subscribe.c b/src/subscribe.c
index ebf121acc468fd245a0d4968c9f99737a5732130..90a4bd5d6b0e18135881a602ab96163cca411ffc 100644 (file)
--- a/src/subscribe.c
+++ b/src/subscribe.c
@@ -163,3 +163,20 @@ void put_status(subscriber_mask_t mask, ...)
                sb = next;
        }
 }
+
+void prune_dead_subscribers(void)
+{
+       subscriber_list_t *sb = subscribe_head;
+       while (sb != NULL) {
+               subscriber_list_t *next = sb->next;
+               // To check if a subscriber's stream is still open and writable call
+               // write with an empty buffer and check the returned value. If the
+               // stream is not writable anymore (i.e. it has been closed because the
+               // process associated to this subscriber no longer exists) then write()
+               // will return -1.
+               if (write(fileno(sb->stream), 0, 0) == -1) {
+                       remove_subscriber(sb);
+               }
+               sb = next;
+       }
+}

diff --git a/src/subscribe.h b/src/subscribe.h
index cb90a862f0ef43ef490b39b8b49106e32298664b..e3e5ba8e141ebb8d77dcd19a154d65becdb53419 100644 (file)
--- a/src/subscribe.h
+++ b/src/subscribe.h
@@ -68,4 +68,8 @@ void add_subscriber(subscriber_list_t *sb);
 int print_report(FILE *stream);
 void put_status(subscriber_mask_t mask, ...);
 
+/* Remove any subscriber for which the stream has been closed and is no longer
+ * writable. */
+void prune_dead_subscribers(void);
+
 #endif