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).
(cherry picked from commit e6ca435c04a344e2a48b11a64b4faf16c9a3b024)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit b50ae918223fecfca4d116dfecbe830cb118f6fc)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 444d720a7cabcf562219c6d221ba05c0927b0acb)
Signed-off-by: Willy Tarreau <w@1wt.eu>
diff --git a/src/fd.c b/src/fd.c
index b9c8ccc..7fd436a 100644
--- a/src/fd.c
+++ b/src/fd.c
@@ -666,9 +666,9 @@
/* 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;
}
}