From b50263f740e601654ef8cd417d6189dd07bb27d6 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 3 Jan 2005 19:07:25 +0000 Subject: [PATCH] More work on task #43: fix race conditions on multithreaded (win32) servers. svn:r3251 --- src/common/compat.c | 6 ++++++ src/or/cpuworker.c | 22 ++++++++++++++-------- src/or/dns.c | 28 ++++++++++++++++++++-------- 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/src/common/compat.c b/src/common/compat.c index b2be40312f..e9f0b23371 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -331,6 +331,7 @@ tor_socketpair(int family, int type, int protocol, int fd[2]) } fd[0] = connector; fd[1] = acceptor; + return 0; abort_tidy_up_and_fail: @@ -557,6 +558,11 @@ get_uname(void) /** Minimalist interface to run a void function in the background. On * unix calls fork, on win32 calls beginthread. Returns -1 on failure. * func should not return, but rather should call spawn_exit. + * + * NOTE: if data is used, it should not be allocated on the stack, + * since in a multithreaded environment, there is no way to be sure that + * the caller's stack will still be around when the called function is + * running. */ int spawn_func(int (*func)(void *), void *data) diff --git a/src/or/cpuworker.c b/src/or/cpuworker.c index c9ac3ebee4..6222300312 100644 --- a/src/or/cpuworker.c +++ b/src/or/cpuworker.c @@ -213,8 +213,9 @@ static int cpuworker_main(void *data) { #ifndef TOR_IS_MULTITHREADED tor_close_socket(fdarray[0]); /* this is the side of the socketpair the parent uses */ connection_free_all(); /* so the child doesn't hold the parent's fd's open */ -#endif handle_signals(0); /* ignore interrupts from the keyboard, etc */ +#endif + tor_free(data); dup_onion_keys(&onion_key, &last_onion_key); @@ -270,33 +271,38 @@ static int cpuworker_main(void *data) { /** Launch a new cpuworker. */ static int spawn_cpuworker(void) { - int fd[2]; + int *fdarray; + int fd; connection_t *conn; - if (tor_socketpair(AF_UNIX, SOCK_STREAM, 0, fd) < 0) { + fdarray = tor_malloc(sizeof(int)*2); + if (tor_socketpair(AF_UNIX, SOCK_STREAM, 0, fdarray) < 0) { log(LOG_ERR, "Couldn't construct socketpair: %s", tor_socket_strerror(tor_socket_errno(-1))); tor_cleanup(); + tor_free(fdarray); exit(1); } - spawn_func(cpuworker_main, (void*)fd); + fd = fdarray[0]; + spawn_func(cpuworker_main, (void*)fdarray); log_fn(LOG_DEBUG,"just spawned a worker."); #ifndef TOR_IS_MULTITHREADED - tor_close_socket(fd[1]); /* we don't need the worker's side of the pipe */ + tor_close_socket(fdarray[1]); /* we don't need the worker's side of the pipe */ + tor_free(fdarray); #endif conn = connection_new(CONN_TYPE_CPUWORKER); - set_socket_nonblocking(fd[0]); + set_socket_nonblocking(fd); /* set up conn so it's got all the data we need to remember */ - conn->s = fd[0]; + conn->s = fd; conn->address = tor_strdup("localhost"); if (connection_add(conn) < 0) { /* no space, forget it */ log_fn(LOG_WARN,"connection_add failed. Giving up."); - connection_free(conn); /* this closes fd[0] */ + connection_free(conn); /* this closes fd */ return -1; } diff --git a/src/or/dns.c b/src/or/dns.c index afc777dd7e..75a45170cf 100644 --- a/src/or/dns.c +++ b/src/or/dns.c @@ -618,6 +618,7 @@ int connection_dns_process_inbuf(connection_t *conn) { void dnsworkers_rotate(void) { connection_t *dnsconn; + log_fn(LOG_INFO, "Rotating DNS workers."); while ((dnsconn = connection_get_by_type_state(CONN_TYPE_DNSWORKER, DNSWORKER_STATE_IDLE))) { connection_mark_for_close(dnsconn); @@ -653,17 +654,21 @@ static int dnsworker_main(void *data) { int fd; int result; + /* log_fn(LOG_NOTICE,"After spawn: fdarray @%d has %d:%d", (int)fdarray, fdarray[0],fdarray[1]); */ + fd = fdarray[1]; /* this side is ours */ #ifndef TOR_IS_MULTITHREADED tor_close_socket(fdarray[0]); /* this is the side of the socketpair the parent uses */ connection_free_all(); /* so the child doesn't hold the parent's fd's open */ -#endif handle_signals(0); /* ignore interrupts from the keyboard, etc */ +#endif + tor_free(data); for (;;) { if (recv(fd, &address_len, 1, 0) != 1) { log_fn(LOG_INFO,"dnsworker exiting because tor process closed connection (either pruned idle dnsworker or died)."); + log_fn(LOG_INFO,"Error on %d was %s", fd, tor_socket_strerror(tor_socket_errno(fd))); spawn_exit(); } @@ -704,33 +709,40 @@ static int dnsworker_main(void *data) { /** Launch a new DNS worker; return 0 on success, -1 on failure. */ static int spawn_dnsworker(void) { - int fd[2]; + int *fdarray; + int fd; connection_t *conn; - if (tor_socketpair(AF_UNIX, SOCK_STREAM, 0, fd) < 0) { + fdarray = tor_malloc(sizeof(int)*2); + if (tor_socketpair(AF_UNIX, SOCK_STREAM, 0, fdarray) < 0) { log(LOG_ERR, "Couldn't construct socketpair: %s", tor_socket_strerror(tor_socket_errno(-1))); tor_cleanup(); + tor_free(fdarray); exit(1); } - spawn_func(dnsworker_main, (void*)fd); + /* log_fn(LOG_NOTICE,"Before spawn: fdarray @%d has %d:%d", (int)fdarray, fdarray[0],fdarray[1]); */ + + fd = fdarray[0]; /* We copy this out here, since dnsworker_main may free fdarray */ + spawn_func(dnsworker_main, (void*)fdarray); log_fn(LOG_DEBUG,"just spawned a worker."); #ifndef TOR_IS_MULTITHREADED - tor_close_socket(fd[1]); /* we don't need the worker's side of the pipe */ + tor_close_socket(fdarray[1]); /* we don't need the worker's side of the pipe */ + tor_free(fdarray); #endif conn = connection_new(CONN_TYPE_DNSWORKER); - set_socket_nonblocking(fd[0]); + set_socket_nonblocking(fd); /* set up conn so it's got all the data we need to remember */ - conn->s = fd[0]; + conn->s = fd; conn->address = tor_strdup(""); if (connection_add(conn) < 0) { /* no space, forget it */ log_fn(LOG_WARN,"connection_add failed. Giving up."); - connection_free(conn); /* this closes fd[0] */ + connection_free(conn); /* this closes fd */ return -1; }