BUG/MEDIUM: poller: use fd_delete() to release the poller pipes

The poller pipes needed to communicate between multiple threads are
allocated in init_pollers_per_thread() and released in
deinit_pollers_per_thread(). The former adds them via fd_insert()
so that they are known, but the former only closes them using a
regular close().

This asymmetry represents a problem, because we have in the fdtab[]
an entry for something that may disappear when one thread leaves, and
since these FD numbers are very low, there is a very high likelihood
that they are immediately reassigned to another thread trying to
connect() to a server or just sending a health check. In this case,
the other thread is going to fd_insert() the fd and the recently
added consistency checks will notive that ->owner is not NULL and
will crash. We just need to use fd_delete() here to match fd_insert().

Note that this test was added in 2.7-dev2 by commit 36d9097cf
("MINOR: fd: Add BUG_ON checks on fd_insert()") which was backported
to 2.4 as a safety measure (since it allowed to catch particularly
serious issues). The patch in itself isn't wrong, it just revealed
a long-dormant bug (been there since 1.9-dev1, 4 years ago). As such
the current patch needs to be backported wherever the commit above
is backported.

Many thanks to Christian Ruppert for providing detailed traces in
github issue #1807 and Cedric Paillet for bringing his complementary
analysis that helped to understand the required conditions for this
issue to happen (fast health checks @100ms + randomly long connections
~7s + fast reloads every second + hard-stop-after 5s were necessary
on the dev's machine to trigger it from time to time).

diff --git a/src/fd.c b/src/fd.c
index ba864deed83f385c8c247017c773991d03fcca58..65036c74f38a83d3bac5cb7b9d89957869f511f9 100644 (file)
--- a/src/fd.c
+++ b/src/fd.c
@@ -974,9 +974,9 @@ static void deinit_pollers_per_thread()
        /* rd and wr are init at the same place, but only rd is init to -1, so
          we rely to rd to close.   */
        if (poller_rd_pipe > -1) {
-               close(poller_rd_pipe);
+               fd_delete(poller_rd_pipe);
                poller_rd_pipe = -1;
-               close(poller_wr_pipe[tid]);
+               fd_delete(poller_wr_pipe[tid]);
                poller_wr_pipe[tid] = -1;
        }
 }