Add a VBM bitmap for keeping track of which filedescriptors the worker

process should inherit, and close all other fd's in the child.

Rename variables used for the various fd's to more understandable names.

git-svn-id: svn+ssh://projects.linpro.no/svn/varnish/trunk@2771 d4fa192b-c00b-0410-8231-f00ffab90ce4

diff --git a/varnish-cache/bin/varnishd/cache_cli.c b/varnish-cache/bin/varnishd/cache_cli.c
index aa4741c58a7643f66aa07ee91c55c8ab39824e6b..0b2e8194ae5dba7cde71cfe5b45fb917f504c0f1 100644 (file)
--- a/varnish-cache/bin/varnishd/cache_cli.c
+++ b/varnish-cache/bin/varnishd/cache_cli.c
@@ -118,7 +118,7 @@ cli_vlu(void *priv, const char *p)
        UNLOCK(&cli_mtx);
        vsb_finish(cli->sb);
        AZ(vsb_overflowed(cli->sb));
-       i = cli_writeres(heritage.fds[1], cli);
+       i = cli_writeres(heritage.cli_out, cli);
        if (i)
                VSL(SLT_Error, 0, "CLI write failed (errno=%d)", errno);
        else
@@ -148,7 +148,7 @@ CLI_Run(void)
        XXXAN(vlu);
        printf("Ready\n");
        while (1) {
-               pfd[0].fd = heritage.fds[2];
+               pfd[0].fd = heritage.cli_in;
                pfd[0].events = POLLIN;
                i = poll(pfd, 1, 5000);
                if (i == 0) {
@@ -160,7 +160,7 @@ CLI_Run(void)
                            "EOF on CLI connection, exiting\n");
                        break;
                }
-               i = VLU_Fd(heritage.fds[2], vlu);
+               i = VLU_Fd(heritage.cli_in, vlu);
                if (i) {
                        fprintf(stderr,
                            "Error on CLI connection, exiting "

diff --git a/varnish-cache/bin/varnishd/common.h b/varnish-cache/bin/varnishd/common.h
index 252b0727f97c26c79f4ac48b2df4f8d06c64e7f0..efca146ce055f94c258a04bf54810858097bb388 100644 (file)
--- a/varnish-cache/bin/varnishd/common.h
+++ b/varnish-cache/bin/varnishd/common.h
@@ -51,3 +51,6 @@ int TCP_connect(int s, const struct sockaddr *name, socklen_t namelen, int msec)
 #endif
 
 #define TRUST_ME(ptr)  ((void*)(uintptr_t)(ptr))
+
+/* Really belongs in mgt.h, but storage_file chokes on both */
+void mgt_child_inherit(int fd, const char *what);

diff --git a/varnish-cache/bin/varnishd/heritage.h b/varnish-cache/bin/varnishd/heritage.h
index 10970a929d1053d40aeda14be98b87874d20d7eb..6eb16a164e5b26ee89a9c6c7efca920c01c28c1a 100644 (file)
--- a/varnish-cache/bin/varnishd/heritage.h
+++ b/varnish-cache/bin/varnishd/heritage.h
@@ -46,11 +46,12 @@ VTAILQ_HEAD(listen_sock_head, listen_sock);
 
 struct heritage {
 
-       /*
-        * Two pipe(2)'s for CLI connection between cache and mgt.
-        * cache reads [2] and writes [1].  Mgt reads [0] and writes [3].
-        */
-       int                             fds[4];
+       /* Two pipe(2)'s for CLI connection between cache and mgt.  */
+       int                             cli_in;
+       int                             cli_out;
+
+       /* File descriptor for stdout/stderr */
+       int                             std_fd;
 
        /* Sockets from which to accept connections */
        struct listen_sock_head         socks;

diff --git a/varnish-cache/bin/varnishd/mgt_child.c b/varnish-cache/bin/varnishd/mgt_child.c
index c052efc0135a606cbf12396d35735e378c558a8c..17b8e8d0d472e67ceb37c375bac1b2c799930b1f 100644 (file)
--- a/varnish-cache/bin/varnishd/mgt_child.c
+++ b/varnish-cache/bin/varnishd/mgt_child.c
@@ -57,11 +57,17 @@
 #include "mgt_event.h"
 #include "vlu.h"
 #include "vss.h"
+#include "vbm.h"
 
 pid_t          mgt_pid;
 pid_t          child_pid = -1;
 
-static int             child_fds[2];
+static struct vbitmap  *fd_map;
+
+static int             child_cli_in = -1;
+static int             child_cli_out = -1;
+static int             child_output = -1;
+
 static enum {
        CH_STOPPED = 0,
        CH_STARTING = 1,
@@ -83,6 +89,26 @@ static struct ev     *ev_poker;
 static struct ev       *ev_listen;
 static struct vlu      *vlu;
 
+/*--------------------------------------------------------------------
+ * Keep track of which filedescriptors the child should inherit and
+ * which should be closed after fork()
+ */
+
+void
+mgt_child_inherit(int fd, const char *what)
+{
+
+       printf("Inherit %d %s\n", fd, what);
+       assert(fd >= 0);
+       if(fd_map == NULL)
+               fd_map = vbit_init(128);
+       AN(fd_map);
+       if (what != NULL)
+               vbit_set(fd_map, fd);
+       else
+               vbit_clr(fd_map, fd);
+}
+
 /*--------------------------------------------------------------------*/
 
 static int
@@ -107,7 +133,7 @@ child_listener(const struct ev *e, int what)
                ev_listen = NULL;
                return (1);
        }
-       if (VLU_Fd(child_fds[0], vlu)) {
+       if (VLU_Fd(child_output, vlu)) {
                ev_listen = NULL;
                return (1);
        }
@@ -148,13 +174,15 @@ open_sockets(void)
                if (ls->sock < 0) 
                        continue;
 
+               mgt_child_inherit(ls->sock, "sock");
+
                /*
                 * Set nonblocking mode to avoid a race where a client
                 * closes before we call accept(2) and nobody else are in
                 * the listen queue to release us.
                 */
                TCP_nonblocking(ls->sock);
-               TCP_filter_http(ls->sock);
+               (void)TCP_filter_http(ls->sock);
                good++;
        }
        if (!good)
@@ -172,6 +200,7 @@ close_sockets(void)
        VTAILQ_FOREACH(ls, &heritage.socks, list) {
                if (ls->sock < 0)
                        continue;
+               mgt_child_inherit(ls->sock, NULL);
                AZ(close(ls->sock));
                ls->sock = -1;
        }
@@ -186,6 +215,7 @@ start_child(void)
        unsigned u;
        char *p;
        struct ev *e;
+       int i, cp[2];
 
        if (child_state != CH_STOPPED && child_state != CH_DIED)
                return;
@@ -197,31 +227,53 @@ start_child(void)
 
        child_state = CH_STARTING;
 
-       AZ(pipe(&heritage.fds[0]));
-       AZ(pipe(&heritage.fds[2]));
-       AZ(pipe(child_fds));
+       /* Open pipe for mgr->child CLI */
+       AZ(pipe(cp));
+       heritage.cli_in = cp[0];
+       mgt_child_inherit(heritage.cli_in, "cli_in");
+       child_cli_out = cp[1];
+
+       /* Open pipe for child->mgr CLI */
+       AZ(pipe(cp));
+       heritage.cli_out = cp[1];
+       mgt_child_inherit(heritage.cli_out, "cli_out");
+       child_cli_in = cp[0];
+
+       /*
+        * Open pipe for child stdout/err 
+        * NB: not inherited, because we dup2() it to stdout/stderr in child
+        */
+       AZ(pipe(cp));
+       heritage.std_fd = cp[1];
+       child_output = cp[0];
+
        MCF_ParamSync();
        if ((pid = fork()) < 0) {
                perror("Could not fork child");
                exit(1);
        }
        if (pid == 0) {
-               /* XXX: We should close things like syslog & telnet sockets */
                if (geteuid() == 0) {
                        XXXAZ(setgid(params->gid));
                        XXXAZ(setuid(params->uid));
                }
 
                /* Redirect stdin/out/err */
-               AZ(close(0));
-               assert(open("/dev/null", O_RDONLY) == 0);
-               assert(dup2(child_fds[1], 1) == 1);
-               assert(dup2(child_fds[1], 2) == 2);
-               AZ(close(child_fds[0]));
-               AZ(close(child_fds[1]));
-
-               AZ(close(heritage.fds[0]));
-               AZ(close(heritage.fds[3]));
+               AZ(close(STDIN_FILENO));
+               assert(open("/dev/null", O_RDONLY) == STDIN_FILENO);
+               assert(dup2(heritage.std_fd, STDOUT_FILENO) == STDOUT_FILENO);
+               assert(dup2(heritage.std_fd, STDERR_FILENO) == STDERR_FILENO);
+
+               /* Close anything we shouldn't know about */
+               closelog();
+               printf("Closed fds:");
+               for (i = STDERR_FILENO + 1; i < getdtablesize(); i++) {
+                       if (vbit_test(fd_map, i))
+                               continue;
+                       if (close(i) == 0)
+                               printf(" %d", i);
+               }
+               printf("\n");
 
                setproctitle("Varnish-Chld %s", heritage.name);
 
@@ -231,13 +283,21 @@ start_child(void)
 
                exit(1);
        }
-
        fprintf(stderr, "start child pid %jd\n", (intmax_t)pid);
 
-       close_sockets();
+       /* Close stuff the child got */
+       AZ(close(heritage.std_fd));
+       heritage.std_fd = -1;
+
+       mgt_child_inherit(heritage.cli_in, NULL);
+       AZ(close(heritage.cli_in));
+       heritage.cli_in = -1;
 
-       AZ(close(child_fds[1]));
-       child_fds[1] = -1;
+       mgt_child_inherit(heritage.cli_out, NULL);
+       AZ(close(heritage.cli_out));
+       heritage.cli_out = -1;
+
+       close_sockets();
 
        vlu = VLU_New(NULL, child_line, 0);
        AN(vlu);
@@ -245,7 +305,7 @@ start_child(void)
        AZ(ev_listen);
        e = ev_new();
        XXXAN(e);
-       e->fd = child_fds[0];
+       e->fd = child_output;
        e->fd_flags = EV_RD;
        e->name = "Child listener";
        e->callback = child_listener;
@@ -263,17 +323,13 @@ start_child(void)
                ev_poker = e;
        }
 
-       mgt_cli_start_child(heritage.fds[0], heritage.fds[3]);
-       AZ(close(heritage.fds[1]));
-       heritage.fds[1] = -1;
-       AZ(close(heritage.fds[2]));
-       heritage.fds[2] = -1;
+       mgt_cli_start_child(child_cli_in, child_cli_out);
        child_pid = pid;
        if (mgt_push_vcls_and_start(&u, &p)) {
                fprintf(stderr, "Pushing vcls failed:\n%s\n", p);
                free(p);
                /* Pick up any stuff lingering on stdout/stderr */
-               child_listener(NULL, EV_RD);
+               (void)child_listener(NULL, EV_RD);
                exit(2);
        }
        child_state = CH_RUNNING;
@@ -300,10 +356,10 @@ stop_child(void)
        mgt_cli_stop_child();
 
        /* We tell the child to die gracefully by closing the CLI */
-       AZ(close(heritage.fds[0]));
-       heritage.fds[0] = -1;
-       AZ(close(heritage.fds[3]));
-       heritage.fds[3] = -1;
+       AZ(close(child_cli_out));
+       child_cli_out= -1;
+       AZ(close(child_cli_in));
+       child_cli_in = -1;
 
        fprintf(stderr, "Child stopping\n");
 }
@@ -333,7 +389,7 @@ mgt_sigchld(const struct ev *e, int what)
        child_pid = -1;
 
        /* Pick up any stuff lingering on stdout/stderr */
-       child_listener(NULL, EV_RD);
+       (void)child_listener(NULL, EV_RD);
 
        if (child_state == CH_RUNNING) {
                child_state = CH_DIED;
@@ -341,10 +397,10 @@ mgt_sigchld(const struct ev *e, int what)
                mgt_cli_stop_child();
 
                /* We tell the child to die gracefully by closing the CLI */
-               AZ(close(heritage.fds[0]));
-               heritage.fds[0] = -1;
-               AZ(close(heritage.fds[3]));
-               heritage.fds[3] = -1;
+               AZ(close(child_cli_out));
+               child_cli_out = -1;
+               AZ(close(child_cli_in));
+               child_cli_in = -1;
        }
 
        if (ev_listen != NULL) {
@@ -353,8 +409,8 @@ mgt_sigchld(const struct ev *e, int what)
        }
        ev_listen = NULL;
 
-       AZ(close(child_fds[0]));
-       child_fds[0] = -1;
+       AZ(close(child_output));
+       child_output = -1;
        fprintf(stderr, "Child cleaned\n");
 
        if (child_state == CH_DIED && params->auto_restart)

diff --git a/varnish-cache/bin/varnishd/storage_file.c b/varnish-cache/bin/varnishd/storage_file.c
index b93c8425d06e294acfca34e35941fc944cc1355b..c6d34c53371e40c3ba1f0f32e2fd4a54944cc83e 100644 (file)
--- a/varnish-cache/bin/varnishd/storage_file.c
+++ b/varnish-cache/bin/varnishd/storage_file.c
@@ -329,6 +329,7 @@ smf_init(struct stevedore *parent, const char *spec)
        XXXAN(sc->filename);
        free(q);
        smf_initfile(sc, size, 1);
+       mgt_child_inherit(sc->fd, "storage_file");
        free(p);
 }