Re: stats socket issues with concurrent accesses

From: Willy Tarreau <w#1wt.eu>
Date: Sun, 14 Mar 2010 19:58:08 +0100

Hi Cyril,

just reproduced and found it !
It was not an easy one this time. The issue clearly was because of the shutdown(write) just after the send() in socat. But while the stats code was correct, there was something in the session handling code aborting the establishment of a connection to a stream interface in such a case, and only if there was no analyser (eg: unix_stream or pure TCP). The reason for it to appear more easily with concurrency was because stats tasks are niced and less of them are processed in the same loop, which leaves more time for the second task to gather a full request and the shutdown.

Here's the patch I'm going to merge. I did a bunch of tests on it (both unix and tcp) and it appears OK.

Cheers,
Willy

---

From 296897f2c6b01c3702487eecf5f413321b23528b Mon Sep 17 00:00:00 2001 From: Willy Tarreau <w#1wt.eu>
Date: Sun, 14 Mar 2010 19:21:34 +0100
Subject: [MEDIUM] connect to servers even when the input has already been closed MIME-Version: 1.0
Content-Type: text/plain; charset=latin1 Content-Transfer-Encoding: 8bit

The BF_AUTO_CLOSE flag prevented a connection from establishing on a server if the other side's input channel was already closed. This is wrong because there may be pending data to be sent.

This was causing an issue with stats, as noticed and reported by Cyril Bonté. Since the stats are now handled as a server, sometimes concurrent accesses were causing one of the connections to send the shutdown(write) before the connection to the stats function was established, which aborted it early.

This fix causes the BF_AUTO_CLOSE flag to be checked only when the connection on the outgoing stream interface has reached an established state. That way we're still able to connect, send the request then close.

---
 src/session.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/session.c b/src/session.c
index 515a275..aa79457 100644
--- a/src/session.c
+++ b/src/session.c
@@ -1331,10 +1331,12 @@ resync_stream_interface:
 


 	/* first, let's check if the request buffer needs to shutdown(write), which may

 	 * happen either because the input is closed or because we want to force a close
-	 * once the server has begun to respond.
+	 * once the server has begun to respond. Note that we only apply it once we're
+	 * connected, so that we still support queuing of a request with input already
+	 * closed.
 	 */
 	if (unlikely((s->req->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_HIJACK|BF_AUTO_CLOSE|BF_SHUTR)) ==
-		     (BF_AUTO_CLOSE|BF_SHUTR)))
+		     (BF_AUTO_CLOSE|BF_SHUTR) && s->req->cons->state >= SI_ST_EST))
 			buffer_shutw_now(s->req);
 


 	/* shutdown(write) pending */

-- 
1.6.4.4

Received on 2010/03/14 19:58