diff --git a/hiredis.c b/hiredis.c index 3a7ab11ae..dc741826a 100644 --- a/hiredis.c +++ b/hiredis.c @@ -724,6 +724,47 @@ redisContext *redisConnectBindNonBlockWithReuse(const char *ip, int port, return c; } +redisContext *redisConnectCloseOnExec(const char *ip, int port) { + redisContext *c; + + c = redisContextInit(); + if (c == NULL) { + return NULL; + } + + c->flags |= REDIS_BLOCK; + c->flags |= REDIS_CLOEXEC; + redisContextConnectTcp(c,ip,port,NULL); + return c; +} + +redisContext *redisConnectWithTimeoutCloseOnExec(const char *ip, int port, const struct timeval tv) { + redisContext *c; + + c = redisContextInit(); + if (c == NULL) + return NULL; + + c->flags |= REDIS_BLOCK; + c->flags |= REDIS_CLOEXEC; + redisContextConnectTcp(c,ip,port,&tv); + return c; +} + +redisContext *redisConnectNonBlockCloseOnExec(const char *ip, int port) { + redisContext *c; + + c = redisContextInit(); + if (c == NULL) + return NULL; + + c->flags &= ~REDIS_BLOCK; + c->flags |= REDIS_CLOEXEC; + redisContextConnectTcp(c,ip,port,NULL); + return c; +} + + redisContext *redisConnectUnix(const char *path) { redisContext *c; diff --git a/hiredis.h b/hiredis.h index 77d5797eb..4eea231b8 100644 --- a/hiredis.h +++ b/hiredis.h @@ -74,6 +74,9 @@ /* Flag that is set when we should set SO_REUSEADDR before calling bind() */ #define REDIS_REUSEADDR 0x80 +/* Flag that is set when we should set FD_CLOEXEC when opening the socket */ +#define REDIS_CLOEXEC 0x100 + #define REDIS_KEEPALIVE_INTERVAL 15 /* seconds */ /* number of times we retry to connect in the case of EADDRNOTAVAIL and @@ -167,6 +170,9 @@ redisContext *redisConnectBindNonBlock(const char *ip, int port, const char *source_addr); redisContext *redisConnectBindNonBlockWithReuse(const char *ip, int port, const char *source_addr); +redisContext *redisConnectCloseOnExec(const char *ip, int port); +redisContext *redisConnectWithTimeoutCloseOnExec(const char *ip, int port, const struct timeval tv); +redisContext *redisConnectNonBlockCloseOnExec(const char *ip, int port); redisContext *redisConnectUnix(const char *path); redisContext *redisConnectUnixWithTimeout(const char *path, const struct timeval tv); redisContext *redisConnectUnixNonBlock(const char *path); diff --git a/net.c b/net.c index 33d9a8297..0352119fa 100644 --- a/net.c +++ b/net.c @@ -54,6 +54,14 @@ #include "net.h" #include "sds.h" +#ifdef SOCK_CLOEXEC +/* sock_cloexec is initialized to SOCK_CLOEXEC and cleared to zero if + * socket(2) ever fails with EINVAL when SOCK_CLOEXEC is set. */ +static int sock_cloexec = SOCK_CLOEXEC; +#else +static int sock_cloexec = 0; +#endif + /* Defined in hiredis.c */ void __redisSetError(redisContext *c, int type, const char *str); @@ -270,8 +278,10 @@ static int _redisContextConnectTcp(redisContext *c, const char *addr, int port, struct addrinfo hints, *servinfo, *bservinfo, *p, *b; int blocking = (c->flags & REDIS_BLOCK); int reuseaddr = (c->flags & REDIS_REUSEADDR); + int set_cloexec = (c->flags & REDIS_CLOEXEC); int reuses = 0; long timeout_msec = -1; + int socket_type; servinfo = NULL; c->connection_type = REDIS_CONN_TCP; @@ -336,9 +346,40 @@ static int _redisContextConnectTcp(redisContext *c, const char *addr, int port, } for (p = servinfo; p != NULL; p = p->ai_next) { addrretry: - if ((s = socket(p->ai_family,p->ai_socktype,p->ai_protocol)) == -1) + socket_type = p->ai_socktype; + if (set_cloexec) { + socket_type |= sock_cloexec; + } + + s = socket(p->ai_family, socket_type, p->ai_protocol); + /* If we attempted to open the socket with SOCK_CLOEXEC and + * errno == EINVAL, that means setting it on socket creation + * is not supported. We clear sock_cloexec so it doesn't do + * anything the next time we use it, and try to create it + * without SOCK_CLOEXEC. */ + if (s == -1 && (socket_type & sock_cloexec) && errno == EINVAL) { + socket_type &= ~sock_cloexec; + sock_cloexec = 0; + s = socket(p->ai_family, socket_type, p->ai_protocol); + } + + if (s == -1) continue; + /* If creating the socket with SOCK_CLOEXEC failed, we need to set + * FD_CLOEXEC on it ASAP in order to minimize the possibility that + * another thread in the running program will fork and inherit the + * file descriptor, so we do this right after checking if the socket + * has been created. Note that we're being optmistic here and there's + * no guarantees that we will be able to set it in time. */ + if (set_cloexec && !sock_cloexec) { + if (fcntl(s, F_SETFD, FD_CLOEXEC) == -1) { + __redisSetErrorFromErrno(c, REDIS_ERR_IO, "fcntl(F_SETFD)"); + redisContextCloseFd(c); + return REDIS_ERR; + } + } + c->fd = s; if (redisSetBlocking(c,0) != REDIS_OK) goto error; diff --git a/test.c b/test.c index a23d60676..9b696e00b 100644 --- a/test.c +++ b/test.c @@ -16,7 +16,8 @@ enum connection_type { CONN_TCP, CONN_UNIX, - CONN_FD + CONN_FD, + CONN_TCP_CLOEXEC }; struct config { @@ -106,6 +107,8 @@ static redisContext *connect(struct config config) { printf("Connecting to inherited fd %d\n", fd); c = redisConnectFd(fd); } + } else if (config.type == CONN_TCP_CLOEXEC) { + c = redisConnectCloseOnExec(config.tcp.host, config.tcp.port); } else { assert(NULL); } @@ -647,6 +650,75 @@ static void test_throughput(struct config config) { disconnect(c, 0); } +static void test_close_on_exec(struct config config) { + + redisContext *c; + char command_fmt[] = "echo $$ > /dev/null; lsof -t -R -a -i @%s:%d -p $$"; + + c = connect(config); + { + FILE *pipe_fp; + int status; + + { + int command_size; + char *command; + + command_size = snprintf(NULL, 0, command_fmt, config.tcp.host, config.tcp.port); + command = malloc((command_size + 1) * sizeof(command)); + if (snprintf(command, command_size + 1, command_fmt, config.tcp.host, config.tcp.port) < command_size) { + fprintf(stderr, "Failed to allocate test command\n"); + exit(1); + } + + if ((pipe_fp = popen(command, "r")) == NULL) { + fprintf(stderr, "Failed to popen\n"); + free(command); + exit(1); + } + free(command); + } + + status = pclose(pipe_fp); + + test("Keep socket on fork+exec without setting close-on-exec: "); + test_cond(WIFEXITED(status) && WEXITSTATUS(status) == 0); + } + disconnect(c, 0); + + config.type = CONN_TCP_CLOEXEC; + c = connect(config); + { + FILE *pipe_fp; + int status; + + { + int command_size; + char *command; + + command_size = snprintf(NULL, 0, command_fmt, config.tcp.host, config.tcp.port); + command = malloc((command_size + 1) * sizeof(command)); + if (snprintf(command, command_size + 1, command_fmt, config.tcp.host, config.tcp.port) < command_size) { + fprintf(stderr, "Failed to allocate test command\n"); + exit(1); + } + + if ((pipe_fp = popen(command, "r")) == NULL) { + fprintf(stderr, "Failed to popen\n"); + free(command); + exit(1); + } + free(command); + } + + status = pclose(pipe_fp); + + test("Don't keep socket on fork+exec when setting close-on-exec: "); + test_cond(WIFEXITED(status) && WEXITSTATUS(status) == 1); + } + disconnect(c, 0); +} + // static long __test_callback_flags = 0; // static void __test_callback(redisContext *c, void *privdata) { // ((void)c); @@ -798,6 +870,7 @@ int main(int argc, char **argv) { test_invalid_timeout_errors(cfg); test_append_formatted_commands(cfg); if (throughput) test_throughput(cfg); + test_close_on_exec(cfg); printf("\nTesting against Unix socket connection (%s):\n", cfg.unix_sock.path); cfg.type = CONN_UNIX;