BUG/MEDIUM: mux-fcgi: ignore conn->owner when deciding if a connection is dead

fcgi_conn_is_dead() reports a connection as dead when it has no stream left
and conn->owner is NULL, on the assumption that an ownerless connection has
nobody left to take care of it.

That assumption no longer holds. Idle backend connections are stored in the
server, reachable through a hash, so they are no longer attached to a
session and may legitimately have conn->owner NULL while still being
perfectly usable. This is exactly the situation mux-h2 hit: the first stream
on a fresh connection sets conn->owner, but a later stream picking that
connection from the idle pool does not, so the connection is destroyed the
next time its last stream detaches instead of being returned to the
pool. Commit a0541f5d2 ("BUG/MEDIUM: mux-h2: ignore conn->owner when
deciding if a connection is dead") removed the test from h2c_is_dead() for
that reason; do the same here.

The test was also actively harmful in fcgi_detach(): the non-private branch
clears conn->owner and then calls fcgi_conn_is_dead(), so the predicate was
true by construction at that point, which would have prevented returning the
connection to the server's idle pool at all.  It only worked because
detach() tests the flags and FCGI_CF_KEEP_CONN first and never reaches the
tail for such a connection.

Note that mux-fcgi has no reverse-connection support, so unlike mux-h2 there
is no conn_is_reverse() companion test to remove.

This should be backported wherever a0541f5d2 was backported, so as far as 2.8.

diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c
index 2d4d6253c3cddad1a7583d4e4515335dfe5f7635..d83858b2cb3289e57e63ccb5ceb67aa645123d6c 100644 (file)
--- a/src/mux_fcgi.c
+++ b/src/mux_fcgi.c
@@ -894,7 +894,6 @@ static inline int fcgi_conn_is_dead(struct fcgi_conn *fconn)
             (fconn->flags & FCGI_CF_ERR_PENDING &&
              fconn->state < FCGI_CS_RECORD_H) || /* early error during connect */
             (fconn->state == FCGI_CS_CLOSED && !fconn->task) ||/* a timeout stroke earlier */
-            (!(fconn->conn->owner)) ||                         /* Nobody's left to take care of the connection, drop it now */
             (!br_data(fconn->mbuf) &&                          /* mux buffer empty, also process clean events below */
              (fconn->flags & FCGI_CF_EOS))))
              return 1;