Commit 902bdf49 authored by Poul-Henning Kamp's avatar Poul-Henning Kamp Committed by Tollef Fog Heen

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.
parent bc568b80
...@@ -64,7 +64,7 @@ extern pthread_t vtc_thread; ...@@ -64,7 +64,7 @@ extern pthread_t vtc_thread;
void init_sema(void); 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); void cmd_server_genvcl(struct vsb *vsb);
......
...@@ -103,7 +103,7 @@ client_thread(void *priv) ...@@ -103,7 +103,7 @@ client_thread(void *priv)
VTCP_myname(fd, mabuf, sizeof mabuf, mpbuf, sizeof mpbuf); VTCP_myname(fd, mabuf, sizeof mabuf, mpbuf, sizeof mpbuf);
vtc_log(vl, 3, "connected fd %d from %s %s to %s", vtc_log(vl, 3, "connected fd %d from %s %s to %s",
fd, mabuf, mpbuf, VSB_data(vsb)); 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); vtc_log(vl, 3, "closing fd %d", fd);
VTCP_close(&fd); VTCP_close(&fd);
} }
......
...@@ -54,7 +54,7 @@ struct http { ...@@ -54,7 +54,7 @@ struct http {
unsigned magic; unsigned magic;
#define HTTP_MAGIC 0x2f02169c #define HTTP_MAGIC 0x2f02169c
int fd; int fd;
int sfd; int *sfd;
int timeout; int timeout;
struct vtclog *vl; struct vtclog *vl;
...@@ -77,14 +77,14 @@ struct http { ...@@ -77,14 +77,14 @@ struct http {
#define ONLY_CLIENT(hp, av) \ #define ONLY_CLIENT(hp, av) \
do { \ do { \
if (hp->sfd >= 0) \ if (hp->sfd != NULL) \
vtc_log(hp->vl, 0, \ vtc_log(hp->vl, 0, \
"\"%s\" only possible in client", av[0]); \ "\"%s\" only possible in client", av[0]); \
} while (0) } while (0)
#define ONLY_SERVER(hp, av) \ #define ONLY_SERVER(hp, av) \
do { \ do { \
if (hp->sfd < 0) \ if (hp->sfd == NULL) \
vtc_log(hp->vl, 0, \ vtc_log(hp->vl, 0, \
"\"%s\" only possible in server", av[0]); \ "\"%s\" only possible in server", av[0]); \
} while (0) } while (0)
...@@ -344,19 +344,22 @@ http_rxchar_eof(struct http *hp, int n) ...@@ -344,19 +344,22 @@ http_rxchar_eof(struct http *hp, int n)
pfd[0].revents = 0; pfd[0].revents = 0;
i = poll(pfd, 1, hp->timeout); i = poll(pfd, 1, hp->timeout);
if (i < 0) if (i < 0)
vtc_log(hp->vl, 0, "HTTP rx timeout (%u ms)", vtc_log(hp->vl, 0, "HTTP rx timeout (fd:%d %u ms)",
hp->timeout); hp->fd, hp->timeout);
if (i < 0) if (i < 0)
vtc_log(hp->vl, 0, "HTTP rx failed (poll: %s)", vtc_log(hp->vl, 0, "HTTP rx failed (fd:%d poll: %s)",
strerror(errno)); hp->fd, strerror(errno));
assert(i > 0); 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); assert(hp->prxbuf + n < hp->nrxbuf);
i = read(hp->fd, hp->rxbuf + hp->prxbuf, n); i = read(hp->fd, hp->rxbuf + hp->prxbuf, n);
if (i == 0) if (i == 0)
return (i); return (i);
if (i <= 0) if (i <= 0)
vtc_log(hp->vl, 0, "HTTP rx failed (read: %s)", vtc_log(hp->vl, 0, "HTTP rx failed (fd:%d read: %s)",
strerror(errno)); hp->fd, strerror(errno));
hp->prxbuf += i; hp->prxbuf += i;
hp->rxbuf[hp->prxbuf] = '\0'; hp->rxbuf[hp->prxbuf] = '\0';
n -= i; n -= i;
...@@ -1036,7 +1039,7 @@ cmd_http_expect_close(CMD_ARGS) ...@@ -1036,7 +1039,7 @@ cmd_http_expect_close(CMD_ARGS)
(void)vl; (void)vl;
CAST_OBJ_NOTNULL(hp, priv, HTTP_MAGIC); CAST_OBJ_NOTNULL(hp, priv, HTTP_MAGIC);
AZ(av[1]); AZ(av[1]);
assert(hp->sfd >= 0); assert(hp->sfd != NULL);
vtc_log(vl, 4, "Expecting close (fd = %d)", hp->fd); vtc_log(vl, 4, "Expecting close (fd = %d)", hp->fd);
while (1) { while (1) {
...@@ -1074,10 +1077,11 @@ cmd_http_accept(CMD_ARGS) ...@@ -1074,10 +1077,11 @@ cmd_http_accept(CMD_ARGS)
(void)vl; (void)vl;
CAST_OBJ_NOTNULL(hp, priv, HTTP_MAGIC); CAST_OBJ_NOTNULL(hp, priv, HTTP_MAGIC);
AZ(av[1]); AZ(av[1]);
assert(hp->sfd != NULL);
assert(hp->sfd >= 0); assert(hp->sfd >= 0);
VTCP_close(&hp->fd); VTCP_close(&hp->fd);
vtc_log(vl, 4, "Accepting"); vtc_log(vl, 4, "Accepting");
hp->fd = accept(hp->sfd, NULL, NULL); hp->fd = accept(*hp->sfd, NULL, NULL);
if (hp->fd < 0) if (hp->fd < 0)
vtc_log(vl, 0, "Accepted failed: %s", strerror(errno)); vtc_log(vl, 0, "Accepted failed: %s", strerror(errno));
vtc_log(vl, 3, "Accepted socket fd is %d", hp->fd); vtc_log(vl, 3, "Accepted socket fd is %d", hp->fd);
...@@ -1136,11 +1140,12 @@ static const struct cmds http_cmds[] = { ...@@ -1136,11 +1140,12 @@ static const struct cmds http_cmds[] = {
{ NULL, NULL } { NULL, NULL }
}; };
void int
http_process(struct vtclog *vl, const char *spec, int sock, int sfd) http_process(struct vtclog *vl, const char *spec, int sock, int *sfd)
{ {
struct http *hp; struct http *hp;
char *s, *q; char *s, *q;
int retval;
(void)sfd; (void)sfd;
ALLOC_OBJ(hp, HTTP_MAGIC); ALLOC_OBJ(hp, HTTP_MAGIC);
...@@ -1162,9 +1167,11 @@ http_process(struct vtclog *vl, const char *spec, int sock, int sfd) ...@@ -1162,9 +1167,11 @@ http_process(struct vtclog *vl, const char *spec, int sock, int sfd)
assert(q > s); assert(q > s);
AN(s); AN(s);
parse_string(s, http_cmds, hp, vl); parse_string(s, http_cmds, hp, vl);
retval = hp->fd;
VSB_delete(hp->vsb); VSB_delete(hp->vsb);
free(hp->rxbuf); free(hp->rxbuf);
free(hp); free(hp);
return (retval);
} }
/********************************************************************** /**********************************************************************
......
...@@ -100,7 +100,7 @@ server_thread(void *priv) ...@@ -100,7 +100,7 @@ server_thread(void *priv)
if (fd < 0) if (fd < 0)
vtc_log(vl, 0, "Accepted failed: %s", strerror(errno)); vtc_log(vl, 0, "Accepted failed: %s", strerror(errno));
vtc_log(vl, 3, "accepted fd %d", fd); 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); vtc_log(vl, 3, "shutting fd %d", fd);
j = shutdown(fd, SHUT_WR); j = shutdown(fd, SHUT_WR);
if (!VTCP_Check(j)) if (!VTCP_Check(j))
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment