BUG/MINOR: listeners: properly close listener FDs

The code dealing with zombie proxies in soft_stop() is bogus, it uses
close() instead of fd_delete(), leaving a live entry in the fdtab with
a dangling pointer to a free memory location. The FD might be reassigned
for an outgoing connection for the time it takes the proxy to completely
stop, or could be dumped on the CLI's "show fd" command. In addition,
the listener's FD was not even reset, leaving doubts about whether or
not it will happen again in deinit().

And in deinit(), the loop in charge of closing zombie FDs is particularly
unsafe because it closes the fd then calls unbind_listener() then
delete_listener() hoping none of them will touch it again. Since it
requires some mental efforts to figure what's done there, let's correctly
reset the fd here as well and close it using fd_delete() to eliminate any
remaining doubts.

It's uncertain whether this should be backported. Zombie proxies are rare
and the situations capable of triggering such issues are not trivial to
setup. However it's easy to imagine how things could go wrong if backported
too far. Better wait for any matching report if at all (this code has been
there since 1.8 without anobody noticing).

diff --git a/src/haproxy.c b/src/haproxy.c
index 22095175acc633b2d051e53b51516806d4089dac..7458dee363bfa9382207335178d291df3f273df7 100644 (file)
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2599,7 +2599,8 @@ void deinit(void)
                         * Close it and give the listener its real state.
                         */
                        if (p->state == PR_STSTOPPED && l->state >= LI_ZOMBIE) {
-                               close(l->rx.fd);
+                               fd_delete(l->rx.fd);
+                               l->rx.fd = -1;
                                l->state = LI_INIT;
                        }
                        unbind_listener(l);

diff --git a/src/proxy.c b/src/proxy.c
index 21d11ee247cdeb412cce7f0f727c091422d669d8..18cdf426e0c86e1c217ca41c3b5ef7ccf71b06aa 100644 (file)
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -1244,8 +1244,10 @@ void soft_stop(void)
                    struct listener *, by_fe)->state > LI_ASSIGNED) {
                        struct listener *l;
                        list_for_each_entry(l, &p->conf.listeners, by_fe) {
-                               if (l->state > LI_ASSIGNED)
-                                       close(l->rx.fd);
+                               if (l->state > LI_ASSIGNED) {
+                                       fd_delete(l->rx.fd);
+                                       l->rx.fd = -1;
+                               }
                                l->state = LI_INIT;
                        }
                }