Browse Source

- fix: tcp fd cache: don't cache own fd in a tcp_reader
- various cleanups/better error messages

Andrei Pelinescu-Onciul 17 years ago
parent
commit
b4fa727e63
2 changed files with 28 additions and 15 deletions
  1. 8 4
      tcp_main.c
  2. 20 11
      tcp_read.c

+ 8 - 4
tcp_main.c

@@ -1448,7 +1448,9 @@ int tcp_send(struct dest_info* dst, union sockaddr_union* from,
 #endif /* TCP_BUF_WRITE */
 #endif /* TCP_BUF_WRITE */
 #ifdef TCP_FD_CACHE
 #ifdef TCP_FD_CACHE
 	struct fd_cache_entry* fd_cache_e;
 	struct fd_cache_entry* fd_cache_e;
+	int use_fd_cache;
 	
 	
+	use_fd_cache=tcp_options.fd_cache;
 	fd_cache_e=0;
 	fd_cache_e=0;
 #endif /* TCP_FD_CACHE */
 #endif /* TCP_FD_CACHE */
 	do_close_fd=1; /* close the fd on exit */
 	do_close_fd=1; /* close the fd on exit */
@@ -1650,7 +1652,11 @@ get_fd:
 			fd=c->fd;
 			fd=c->fd;
 			do_close_fd=0; /* don't close the fd on exit, it's in use */
 			do_close_fd=0; /* don't close the fd on exit, it's in use */
 #ifdef TCP_FD_CACHE
 #ifdef TCP_FD_CACHE
-		}else if (likely(tcp_options.fd_cache && 
+			use_fd_cache=0; /* don't cache: problems would arise due to the
+							   close() on cache eviction (if the fd is still 
+							   used). If it has to be cached then dup() _must_ 
+							   be used */
+		}else if (likely(use_fd_cache && 
 							((fd_cache_e=tcp_fd_cache_get(c))!=0))){
 							((fd_cache_e=tcp_fd_cache_get(c))!=0))){
 			fd=fd_cache_e->fd;
 			fd=fd_cache_e->fd;
 			do_close_fd=0;
 			do_close_fd=0;
@@ -1797,7 +1803,7 @@ error:
 #endif /* TCP_BUF_WRITE */
 #endif /* TCP_BUF_WRITE */
 end:
 end:
 #ifdef TCP_FD_CACHE
 #ifdef TCP_FD_CACHE
-	if (unlikely((fd_cache_e==0) && tcp_options.fd_cache)){
+	if (unlikely((fd_cache_e==0) && use_fd_cache)){
 		tcp_fd_cache_add(c, fd);
 		tcp_fd_cache_add(c, fd);
 	}else
 	}else
 #endif /* TCP_FD_CACHE */
 #endif /* TCP_FD_CACHE */
@@ -1815,8 +1821,6 @@ conn_wait_error:
 	 * tcp_main receives a CONN_ERROR it*/
 	 * tcp_main receives a CONN_ERROR it*/
 	c->state=S_CONN_BAD;
 	c->state=S_CONN_BAD;
 	TCPCONN_LOCK;
 	TCPCONN_LOCK;
-		/* FIXME: race: what if CONN_ERROR is sent by another tcp_send() to
-		 * tcp_main ? */
 		if (c->flags & F_CONN_HASHED){
 		if (c->flags & F_CONN_HASHED){
 			/* if some other parallel tcp_send did send CONN_ERROR to
 			/* if some other parallel tcp_send did send CONN_ERROR to
 			 * tcp_main, the connection might be already detached */
 			 * tcp_main, the connection might be already detached */

+ 20 - 11
tcp_read.c

@@ -674,7 +674,10 @@ void release_tcpconn(struct tcp_connection* c, long state, int unix_sock)
 		DBG(" extra_data %p\n", c->extra_data);
 		DBG(" extra_data %p\n", c->extra_data);
 		/* release req & signal the parent */
 		/* release req & signal the parent */
 		c->reader_pid=0; /* reset it */
 		c->reader_pid=0; /* reset it */
-		if (c->fd!=-1) close(c->fd);
+		if (c->fd!=-1){
+			close(c->fd);
+			c->fd=-1;
+		}
 		/* errno==EINTR, EWOULDBLOCK a.s.o todo */
 		/* errno==EINTR, EWOULDBLOCK a.s.o todo */
 		response[0]=(long)c;
 		response[0]=(long)c;
 		response[1]=state;
 		response[1]=state;
@@ -698,8 +701,9 @@ static ticks_t tcpconn_read_timeout(ticks_t t, struct timer_ln* tl, void* data)
 	}
 	}
 	/* if conn->state is ERROR or BAD => force timeout too */
 	/* if conn->state is ERROR or BAD => force timeout too */
 	if (unlikely(io_watch_del(&io_w, c->fd, -1, IO_FD_CLOSING)<0)){
 	if (unlikely(io_watch_del(&io_w, c->fd, -1, IO_FD_CLOSING)<0)){
-		LOG(L_ERR, "ERROR; tcpconn_read_timeout: io_watch_del failed for %p"
-					" id %d fd %d, state %d\n", c, c->id, c->fd, c->state);
+		LOG(L_ERR, "ERROR: tcpconn_read_timeout: io_watch_del failed for %p"
+					" id %d fd %d, state %d, flags %x, main fd %d\n",
+					c, c->id, c->fd, c->state, c->flags, c->s);
 	}
 	}
 	tcpconn_listrm(tcp_conn_lst, c, c_next, c_prev);
 	tcpconn_listrm(tcp_conn_lst, c, c_next, c_prev);
 	release_tcpconn(c, (c->state<0)?CONN_ERROR:CONN_RELEASE, tcpmain_sock);
 	release_tcpconn(c, (c->state<0)?CONN_ERROR:CONN_RELEASE, tcpmain_sock);
@@ -791,10 +795,12 @@ again:
 			local_timer_reinit(&con->timer);
 			local_timer_reinit(&con->timer);
 			local_timer_add(&tcp_reader_ltimer, &con->timer,
 			local_timer_add(&tcp_reader_ltimer, &con->timer,
 								S_TO_TICKS(TCP_CHILD_TIMEOUT), t);
 								S_TO_TICKS(TCP_CHILD_TIMEOUT), t);
-			if (unlikely(io_watch_add(&io_w, s, POLLIN, F_TCPCONN, con))<0){
-				LOG(L_CRIT, "ERROR; tcpconn_receive: handle_io: io_watch_add "
-							"failed for %p id %d fd %d, state %d\n",
-							con, con->id, con->fd, con->state);
+			if (unlikely(io_watch_add(&io_w, s, POLLIN, F_TCPCONN, con)<0)){
+				LOG(L_CRIT, "ERROR: tcpconn_receive: handle_io: io_watch_add "
+							"failed for %p id %d fd %d, state %d, flags %x,"
+							" main fd %d, refcnt %d\n",
+							con, con->id, con->fd, con->state, con->flags,
+							con->s, atomic_get(&con->refcnt));
 				tcpconn_listrm(tcp_conn_lst, con, c_next, c_prev);
 				tcpconn_listrm(tcp_conn_lst, con, c_next, c_prev);
 				local_timer_del(&tcp_reader_ltimer, &con->timer);
 				local_timer_del(&tcp_reader_ltimer, &con->timer);
 				goto con_error;
 				goto con_error;
@@ -815,13 +821,16 @@ read_error:
 				ret=-1; /* some error occured */
 				ret=-1; /* some error occured */
 				if (unlikely(io_watch_del(&io_w, con->fd, idx,
 				if (unlikely(io_watch_del(&io_w, con->fd, idx,
 											IO_FD_CLOSING) < 0)){
 											IO_FD_CLOSING) < 0)){
-				LOG(L_CRIT, "ERROR; tcpconn_receive: handle_io: io_watch_del "
-							"failed for %p id %d fd %d, state %d\n",
-							con, con->id, con->fd, con->state);
+					LOG(L_CRIT, "ERROR: tcpconn_receive: handle_io: "
+							"io_watch_del failed for %p id %d fd %d,"
+							" state %d, flags %x, main fd %d, refcnt %d\n",
+							con, con->id, con->fd, con->state,
+							con->flags, con->s, atomic_get(&con->refcnt));
 				}
 				}
 				tcpconn_listrm(tcp_conn_lst, con, c_next, c_prev);
 				tcpconn_listrm(tcp_conn_lst, con, c_next, c_prev);
 				local_timer_del(&tcp_reader_ltimer, &con->timer);
 				local_timer_del(&tcp_reader_ltimer, &con->timer);
-				con->state=S_CONN_BAD;
+				if (unlikely(resp!=CONN_EOF))
+					con->state=S_CONN_BAD;
 				release_tcpconn(con, resp, tcpmain_sock);
 				release_tcpconn(con, resp, tcpmain_sock);
 			}else{
 			}else{
 				/* update timeout */
 				/* update timeout */