Explorar el Código

- fixes:
- on io_watch_add overwrite error don't delete the previous fd hash
entry + more error debugging info
- return proper len on pending connect (instead of -1)
- tcp_reader: always check if a connection was marked as bad and if
so release it immediately + error checks for io_watch_*

Andrei Pelinescu-Onciul hace 18 años
padre
commit
316130a735
Se han modificado 3 ficheros con 40 adiciones y 10 borrados
  1. 10 4
      io_wait.h
  2. 1 0
      tcp_main.c
  3. 29 6
      tcp_read.c

+ 10 - 4
io_wait.h

@@ -344,8 +344,9 @@ inline static int io_watch_add(	io_wait_h* h,
 	e=get_fd_map(h, fd);
 	e=get_fd_map(h, fd);
 	if (unlikely(e && (e->type!=0 /*F_NONE*/))){
 	if (unlikely(e && (e->type!=0 /*F_NONE*/))){
 		LOG(L_ERR, "ERROR: io_watch_add: trying to overwrite entry %d"
 		LOG(L_ERR, "ERROR: io_watch_add: trying to overwrite entry %d"
-				" in the hash(%d, %d, %p) with (%d, %d, %p)\n",
-				fd, e->fd, e->type, e->data, fd, type, data);
+				" watched for %x in the hash(%d, %d, %p) with (%d, %d, %p)\n",
+				fd, events, e->fd, e->type, e->data, fd, type, data);
+		e=0;
 		goto error;
 		goto error;
 	}
 	}
 	
 	
@@ -442,6 +443,11 @@ again2:
 			}
 			}
 			if (unlikely( events & POLLOUT)){
 			if (unlikely( events & POLLOUT)){
 				if (unlikely(kq_ev_change(h, fd, EVFILT_WRITE, EV_ADD, e)==-1))
 				if (unlikely(kq_ev_change(h, fd, EVFILT_WRITE, EV_ADD, e)==-1))
+				{
+					if (likely(events & POLLIN)){
+						kq_ev_change(h, fd, EVFILT_READ, EV_DELETE, 0);
+					}
+				}
 				goto error;
 				goto error;
 			}
 			}
 			break;
 			break;
@@ -554,8 +560,8 @@ inline static int io_watch_del(io_wait_h* h, int fd, int idx, int flags)
 	}
 	}
 	if (unlikely(e->type==0 /*F_NONE*/)){
 	if (unlikely(e->type==0 /*F_NONE*/)){
 		LOG(L_ERR, "ERROR: io_watch_del: trying to delete already erased"
 		LOG(L_ERR, "ERROR: io_watch_del: trying to delete already erased"
-				" entry %d in the hash(%d, %d, %p) )\n",
-				fd, e->fd, e->type, e->data);
+				" entry %d in the hash(%d, %d, %p) flags %x)\n",
+				fd, e->fd, e->type, e->data, flags);
 		goto error;
 		goto error;
 	}
 	}
 	events=e->events;
 	events=e->events;

+ 1 - 0
tcp_main.c

@@ -1566,6 +1566,7 @@ no_id:
 										c, strerror(errno), errno);
 										c, strerror(errno), errno);
 							goto conn_wait_error;
 							goto conn_wait_error;
 						}
 						}
+						n=len;
 						goto end;
 						goto end;
 					}
 					}
 					/* error: destroy it directly */
 					/* error: destroy it directly */

+ 29 - 6
tcp_read.c

@@ -697,7 +697,10 @@ static ticks_t tcpconn_read_timeout(ticks_t t, struct timer_ln* tl, void* data)
 		return (ticks_t)(c->timeout - t);
 		return (ticks_t)(c->timeout - t);
 	}
 	}
 	/* if conn->state is ERROR or BAD => force timeout too */
 	/* if conn->state is ERROR or BAD => force timeout too */
-	io_watch_del(&io_w, c->fd, -1, IO_FD_CLOSING);
+	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);
+	}
 	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);
 	
 	
@@ -767,9 +770,15 @@ again:
 							" connection received: %p, id %d, fd %d, refcnt %d"
 							" connection received: %p, id %d, fd %d, refcnt %d"
 							" state %d (n=%d)\n", con, con->id, con->fd,
 							" state %d (n=%d)\n", con, con->id, con->fd,
 							atomic_get(&con->refcnt), con->state, n);
 							atomic_get(&con->refcnt), con->state, n);
-				release_tcpconn(con, CONN_ERROR, tcpmain_sock);
+				goto con_error;
 				break; /* try to recover */
 				break; /* try to recover */
 			}
 			}
+			if (unlikely(con->state==S_CONN_BAD)){
+				LOG(L_WARN, "WARNING: tcp_receive: handle_io: received an"
+							" already bad connection: %p id %d refcnt %d\n",
+							con, con->id, atomic_get(&con->refcnt));
+				goto con_error;
+			}
 			/* must be before io_watch_add, io_watch_add might catch some
 			/* must be before io_watch_add, io_watch_add might catch some
 			 * already existing events => might call handle_io and
 			 * already existing events => might call handle_io and
 			 * handle_io might decide to del. the new connection =>
 			 * handle_io might decide to del. the new connection =>
@@ -783,8 +792,9 @@ again:
 			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){
 			if (unlikely(io_watch_add(&io_w, s, POLLIN, F_TCPCONN, con))<0){
-				LOG(L_CRIT, "ERROR: tcp_receive: handle_io: failed to add"
-						" new socket to the fd list\n");
+				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);
 				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;
@@ -792,10 +802,23 @@ again:
 			break;
 			break;
 		case F_TCPCONN:
 		case F_TCPCONN:
 			con=(struct tcp_connection*)fm->data;
 			con=(struct tcp_connection*)fm->data;
+			if (unlikely(con->state==S_CONN_BAD)){
+				resp=CONN_ERROR;
+				LOG(L_WARN, "WARNING: tcp_receive: handle_io: F_TCPCONN"
+							" connection marked as bad: %p id %d refcnt %d\n",
+							con, con->id, atomic_get(&con->refcnt));
+				goto read_error;
+			}
 			resp=tcp_read_req(con, &ret);
 			resp=tcp_read_req(con, &ret);
 			if (unlikely(resp<0)){
 			if (unlikely(resp<0)){
+read_error:
 				ret=-1; /* some error occured */
 				ret=-1; /* some error occured */
-				io_watch_del(&io_w, con->fd, idx, IO_FD_CLOSING);
+				if (unlikely(io_watch_del(&io_w, con->fd, idx,
+											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);
+				}
 				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;
 				con->state=S_CONN_BAD;
@@ -818,7 +841,7 @@ again:
 	return ret;
 	return ret;
 con_error:
 con_error:
 	con->state=S_CONN_BAD;
 	con->state=S_CONN_BAD;
-	release_tcpconn(con, CONN_ERROR, fm->fd);
+	release_tcpconn(con, CONN_ERROR, tcpmain_sock);
 	return ret;
 	return ret;
 error:
 error:
 	return -1;
 	return -1;