[master] 4351233 Fix a long-standing bug in varnishtest:

Poul-Henning Kamp phk at varnish-cache.org
Tue Sep 20 22:29:45 CEST 2011


commit 43512331ff01fad7f5078cda5339ba169bd1d81f
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Tue Sep 20 20:22:09 2011 +0000

    Fix a long-standing bug in varnishtest:
    
    It used to be that we'd call http processing on a filedescriptor and
    then that was that.  Then we added keywords to do new accepts in the
    server and suddenly the calling code closes a wrong filedesc which
    belongs to somebody else and things get really inconvenient fast.
    
    This made all test-cases which use the "accept" server directive flakey.

diff --git a/bin/varnishtest/vtc.h b/bin/varnishtest/vtc.h
index eb9a02a..5e5cce7 100644
--- a/bin/varnishtest/vtc.h
+++ b/bin/varnishtest/vtc.h
@@ -64,7 +64,7 @@ extern pthread_t	vtc_thread;
 
 void init_sema(void);
 
-void http_process(struct vtclog *vl, const char *spec, int sock, int sfd);
+int http_process(struct vtclog *vl, const char *spec, int sock, int *sfd);
 
 void cmd_server_genvcl(struct vsb *vsb);
 
diff --git a/bin/varnishtest/vtc_client.c b/bin/varnishtest/vtc_client.c
index 20b152b..3bd430b 100644
--- a/bin/varnishtest/vtc_client.c
+++ b/bin/varnishtest/vtc_client.c
@@ -103,7 +103,7 @@ client_thread(void *priv)
 		VTCP_myname(fd, mabuf, sizeof mabuf, mpbuf, sizeof mpbuf);
 		vtc_log(vl, 3, "connected fd %d from %s %s to %s",
 		    fd, mabuf, mpbuf, VSB_data(vsb));
-		http_process(vl, c->spec, fd, -1);
+		fd = http_process(vl, c->spec, fd, NULL);
 		vtc_log(vl, 3, "closing fd %d", fd);
 		VTCP_close(&fd);
 	}
diff --git a/bin/varnishtest/vtc_http.c b/bin/varnishtest/vtc_http.c
index 7dad4ed..32c7e0c 100644
--- a/bin/varnishtest/vtc_http.c
+++ b/bin/varnishtest/vtc_http.c
@@ -54,7 +54,7 @@ struct http {
 	unsigned		magic;
 #define HTTP_MAGIC		0x2f02169c
 	int			fd;
-	int			sfd;
+	int			*sfd;
 	int			timeout;
 	struct vtclog		*vl;
 
@@ -77,14 +77,14 @@ struct http {
 
 #define ONLY_CLIENT(hp, av)						\
 	do {								\
-		if (hp->sfd >= 0)					\
+		if (hp->sfd != NULL)					\
 			vtc_log(hp->vl, 0,				\
 			    "\"%s\" only possible in client", av[0]);	\
 	} while (0)
 
 #define ONLY_SERVER(hp, av)						\
 	do {								\
-		if (hp->sfd < 0)					\
+		if (hp->sfd == NULL)					\
 			vtc_log(hp->vl, 0,				\
 			    "\"%s\" only possible in server", av[0]);	\
 	} while (0)
@@ -344,19 +344,22 @@ http_rxchar_eof(struct http *hp, int n)
 		pfd[0].revents = 0;
 		i = poll(pfd, 1, hp->timeout);
 		if (i < 0)
-			vtc_log(hp->vl, 0, "HTTP rx timeout (%u ms)",
-			    hp->timeout);
+			vtc_log(hp->vl, 0, "HTTP rx timeout (fd:%d %u ms)",
+			    hp->fd, hp->timeout);
 		if (i < 0)
-			vtc_log(hp->vl, 0, "HTTP rx failed (poll: %s)",
-			    strerror(errno));
+			vtc_log(hp->vl, 0, "HTTP rx failed (fd:%d poll: %s)",
+			    hp->fd, strerror(errno));
 		assert(i > 0);
+		if (pfd[0].revents & ~POLLIN)
+			vtc_log(hp->vl, 4, "HTTP rx poll (fd:%d revents: %x)",
+			    hp->fd, pfd[0].revents);
 		assert(hp->prxbuf + n < hp->nrxbuf);
 		i = read(hp->fd, hp->rxbuf + hp->prxbuf, n);
 		if (i == 0)
 			return (i);
 		if (i <= 0)
-			vtc_log(hp->vl, 0, "HTTP rx failed (read: %s)",
-			    strerror(errno));
+			vtc_log(hp->vl, 0, "HTTP rx failed (fd:%d read: %s)",
+			    hp->fd, strerror(errno));
 		hp->prxbuf += i;
 		hp->rxbuf[hp->prxbuf] = '\0';
 		n -= i;
@@ -1036,7 +1039,7 @@ cmd_http_expect_close(CMD_ARGS)
 	(void)vl;
 	CAST_OBJ_NOTNULL(hp, priv, HTTP_MAGIC);
 	AZ(av[1]);
-	assert(hp->sfd >= 0);
+	assert(hp->sfd != NULL);
 
 	vtc_log(vl, 4, "Expecting close (fd = %d)", hp->fd);
 	while (1) {
@@ -1074,10 +1077,11 @@ cmd_http_accept(CMD_ARGS)
 	(void)vl;
 	CAST_OBJ_NOTNULL(hp, priv, HTTP_MAGIC);
 	AZ(av[1]);
+	assert(hp->sfd != NULL);
 	assert(hp->sfd >= 0);
 	VTCP_close(&hp->fd);
 	vtc_log(vl, 4, "Accepting");
-	hp->fd = accept(hp->sfd, NULL, NULL);
+	hp->fd = accept(*hp->sfd, NULL, NULL);
 	if (hp->fd < 0)
 		vtc_log(vl, 0, "Accepted failed: %s", strerror(errno));
 	vtc_log(vl, 3, "Accepted socket fd is %d", hp->fd);
@@ -1136,11 +1140,12 @@ static const struct cmds http_cmds[] = {
 	{ NULL,			NULL }
 };
 
-void
-http_process(struct vtclog *vl, const char *spec, int sock, int sfd)
+int
+http_process(struct vtclog *vl, const char *spec, int sock, int *sfd)
 {
 	struct http *hp;
 	char *s, *q;
+	int retval;
 
 	(void)sfd;
 	ALLOC_OBJ(hp, HTTP_MAGIC);
@@ -1162,9 +1167,11 @@ http_process(struct vtclog *vl, const char *spec, int sock, int sfd)
 	assert(q > s);
 	AN(s);
 	parse_string(s, http_cmds, hp, vl);
+	retval = hp->fd;
 	VSB_delete(hp->vsb);
 	free(hp->rxbuf);
 	free(hp);
+	return (retval);
 }
 
 /**********************************************************************
diff --git a/bin/varnishtest/vtc_server.c b/bin/varnishtest/vtc_server.c
index 542a266..d1787b1 100644
--- a/bin/varnishtest/vtc_server.c
+++ b/bin/varnishtest/vtc_server.c
@@ -100,7 +100,7 @@ server_thread(void *priv)
 		if (fd < 0)
 			vtc_log(vl, 0, "Accepted failed: %s", strerror(errno));
 		vtc_log(vl, 3, "accepted fd %d", fd);
-		http_process(vl, s->spec, fd, s->sock);
+		fd = http_process(vl, s->spec, fd, &s->sock);
 		vtc_log(vl, 3, "shutting fd %d", fd);
 		j = shutdown(fd, SHUT_WR);
 		if (!VTCP_Check(j))



More information about the varnish-commit mailing list