From: Marko Kreen <markokr@gmail.com>
Date: Tue, 12 Jan 2010 10:19:31 +0000 (+0000)
Subject: sbuf: track exact wait state to avoid double event_del()
X-Git-Tag: pgbouncer_1_3_2~8
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=3ad8094aabda3189b089c5d59b4323d4b503b583;p=pgbouncer

sbuf: track exact wait state to avoid double event_del()
---

diff --git a/include/sbuf.h b/include/sbuf.h
index 3ce431e..33af683 100644
--- a/include/sbuf.h
+++ b/include/sbuf.h
@@ -62,7 +62,7 @@ struct SBuf {
 	struct event ev;	/* libevent handle */
 
 	bool is_unix;		/* is it unix socket */
-	bool wait_send;		/* debug var, otherwise useless */
+	uint8_t wait_type;	/* track wait state */
 	uint8_t pkt_action;	/* method for handling current pkt */
 
 	int sock;		/* fd for this socket */
diff --git a/src/sbuf.c b/src/sbuf.c
index fc5acaf..ea7189f 100644
--- a/src/sbuf.c
+++ b/src/sbuf.c
@@ -35,6 +35,12 @@
 #define ACT_SKIP 2
 #define ACT_CALL 3
 
+enum WaitType {
+	W_NONE = 0,
+	W_CONNECT,
+	W_RECV,
+	W_SEND,
+};
 
 #define AssertSanity(sbuf) do { \
 	Assert(iobuf_sane((sbuf)->io)); \
@@ -158,8 +164,10 @@ bool sbuf_connect(SBuf *sbuf, const PgAddr *addr, const char *unix_dir, int time
 		/* tcp socket needs waiting */
 		event_set(&sbuf->ev, sock, EV_WRITE, sbuf_connect_cb, sbuf);
 		res = event_add(&sbuf->ev, &timeout);
-		if (res >= 0)
+		if (res >= 0) {
+			sbuf->wait_type = W_CONNECT;
 			return true;
+		}
 	}
 
 failed:
@@ -176,12 +184,13 @@ failed:
 bool sbuf_pause(SBuf *sbuf)
 {
 	AssertActive(sbuf);
-	Assert(sbuf->wait_send == 0);
+	Assert(sbuf->wait_type == W_RECV);
 
 	if (event_del(&sbuf->ev) < 0) {
 		log_warning("event_del: %s", strerror(errno));
 		return false;
 	}
+	sbuf->wait_type = W_NONE;
 	return true;
 }
 
@@ -233,13 +242,15 @@ bool sbuf_continue_with_callback(SBuf *sbuf, sbuf_libevent_cb user_cb)
 		log_warning("sbuf_continue_with_callback: %s", strerror(errno));
 		return false;
 	}
+	sbuf->wait_type = W_RECV;
 	return true;
 }
 
 /* socket cleanup & close: keeps .handler and .arg values */
 bool sbuf_close(SBuf *sbuf)
 {
-	if (sbuf->sock > 0) {
+	if (sbuf->wait_type) {
+		Assert(sbuf->sock);
 		/* event_del() acts funny occasionally, debug it */
 		errno = 0;
 		if (event_del(&sbuf->ev) < 0) {
@@ -251,12 +262,13 @@ bool sbuf_close(SBuf *sbuf)
 			/* we can retry whole sbuf_close() if needed */
 			/* if (errno == ENOMEM) return false; */
 		}
-		safe_close(sbuf->sock);
 	}
+	if (sbuf->sock > 0)
+		safe_close(sbuf->sock);
 	sbuf->dst = NULL;
 	sbuf->sock = 0;
 	sbuf->pkt_remain = 0;
-	sbuf->pkt_action = sbuf->wait_send = 0;
+	sbuf->pkt_action = sbuf->wait_type = 0;
 	if (sbuf->io) {
 		obj_free(iobuf_cache, sbuf->io);
 		sbuf->io = NULL;
@@ -352,6 +364,7 @@ static bool sbuf_wait_for_data(SBuf *sbuf)
 		log_warning("sbuf_wait_for_data: event_add: %s", strerror(errno));
 		return false;
 	}
+	sbuf->wait_type = W_RECV;
 	return true;
 }
 
@@ -366,10 +379,11 @@ static void sbuf_send_cb(int sock, short flags, void *arg)
 		return;
 
 	AssertSanity(sbuf);
-	Assert(sbuf->wait_send);
+	Assert(sbuf->wait_type == W_SEND);
+
+	sbuf->wait_type = W_NONE;
 
 	/* prepare normal situation for sbuf_main_loop */
-	sbuf->wait_send = 0;
 	res = sbuf_wait_for_data(sbuf);
 	if (res) {
 		/* here we should certainly skip recv() */
@@ -384,11 +398,13 @@ static bool sbuf_queue_send(SBuf *sbuf)
 {
 	int err;
 	AssertActive(sbuf);
+	Assert(sbuf->wait_type == W_RECV);
 
 	/* if false is returned, the socket will be closed later */
 
 	/* stop waiting for read events */
 	err = event_del(&sbuf->ev);
+	sbuf->wait_type = W_NONE; /* make sure its called only once */
 	if (err < 0) {
 		log_warning("sbuf_queue_send: event_del failed: %s", strerror(errno));
 		return false;
@@ -401,8 +417,8 @@ static bool sbuf_queue_send(SBuf *sbuf)
 		log_warning("sbuf_queue_send: event_add failed: %s", strerror(errno));
 		return false;
 	}
+	sbuf->wait_type = W_SEND;
 
-	sbuf->wait_send = 1;
 	return true;
 }
 
@@ -598,7 +614,7 @@ static void sbuf_main_loop(SBuf *sbuf, bool skip_recv)
 		return;
 
 	/* reading should be disabled when waiting */
-	Assert(sbuf->wait_send == 0);
+	Assert(sbuf->wait_type == W_RECV);
 	AssertSanity(sbuf);
 
 	if (!allocate_iobuf(sbuf))
@@ -690,6 +706,9 @@ static void sbuf_connect_cb(int sock, short flags, void *arg)
 {
 	SBuf *sbuf = arg;
 
+	Assert(sbuf->wait_type == W_CONNECT || sbuf->wait_type == W_NONE);
+	sbuf->wait_type = W_NONE;
+
 	if (flags & EV_WRITE) {
 		if (!sbuf_after_connect_check(sbuf))
 			goto failed;