[experimental-ims] 4351233 Fix a long-standing bug in varnishtest:
Geoff Simmons
geoff at varnish-cache.org
Mon Jan 9 21:52:00 CET 2012
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