Browse code

tcp: more consistent IO_FD_CLOSING usage

- extra safety checks before using IO_FD_CLOSING in io_watch_del()
calls (must be used only when the fd will be close() imediately
afterwards).
- add IO_FD_CLOSING when POLLERR or POLLHUP is detected in
tcp_main and the socket receive buffer is empty (in this case
the fd will be shortly closed)

Andrei Pelinescu-Onciul authored on 18/06/2010 22:21:58
Showing 1 changed files
... ...
@@ -3099,18 +3099,31 @@ inline static int handle_tcp_child(struct tcp_child* tcp_c, int fd_i)
3099 3099
 		case CONN_RELEASE:
3100 3100
 			tcp_c->busy--;
3101 3101
 			if (unlikely(tcpconn_put(tcpconn))){
3102
+				/* if refcnt was 1 => it was used only in the
3103
+				   tcp reader => it's not hashed or watched for IO
3104
+				   anymore => no need to io_watch_del() */
3102 3105
 				tcpconn_destroy(tcpconn);
3103 3106
 				break;
3104 3107
 			}
3105 3108
 			if (unlikely(tcpconn->state==S_CONN_BAD)){ 
3109
+				if (tcpconn_try_unhash(tcpconn)) {
3106 3110
 #ifdef TCP_ASYNC
3107
-				if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){
3108
-					io_watch_del(&io_h, tcpconn->s, -1, IO_FD_CLOSING);
3111
+					if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){
3112
+						io_watch_del(&io_h, tcpconn->s, -1, IO_FD_CLOSING);
3113
+						tcpconn->flags &= ~F_CONN_WRITE_W;
3114
+					}
3115
+#endif /* TCP_ASYNC */
3116
+					tcpconn_put_destroy(tcpconn);
3117
+				}
3118
+#ifdef TCP_ASYNC
3119
+				 else if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){
3120
+					/* should never happen: if it's already unhashed, it
3121
+					   should not be watched for IO */
3122
+					BUG("unhashed connection watched for write\n");
3123
+					io_watch_del(&io_h, tcpconn->s, -1, 0);
3109 3124
 					tcpconn->flags &= ~F_CONN_WRITE_W;
3110 3125
 				}
3111 3126
 #endif /* TCP_ASYNC */
3112
-				if (tcpconn_try_unhash(tcpconn))
3113
-					tcpconn_put_destroy(tcpconn);
3114 3127
 				break;
3115 3128
 			}
3116 3129
 			/* update the timeout*/
... ...
@@ -3147,12 +3160,17 @@ inline static int handle_tcp_child(struct tcp_child* tcp_c, int fd_i)
3147 3147
 						TCP_EV_SEND_TIMEOUT(0, &tcpconn->rcv);
3148 3148
 						TCP_STATS_SEND_TIMEOUT();
3149 3149
 					}
3150
-					if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){
3151
-						io_watch_del(&io_h, tcpconn->s, -1, IO_FD_CLOSING);
3150
+					if (tcpconn_try_unhash(tcpconn)) {
3151
+						if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){
3152
+							io_watch_del(&io_h, tcpconn->s, -1, IO_FD_CLOSING);
3153
+							tcpconn->flags&=~F_CONN_WRITE_W;
3154
+						}
3155
+						tcpconn_put_destroy(tcpconn);
3156
+					} else if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){
3157
+						BUG("unhashed connection watched for write\n");
3158
+						io_watch_del(&io_h, tcpconn->s, -1, 0);
3152 3159
 						tcpconn->flags&=~F_CONN_WRITE_W;
3153 3160
 					}
3154
-					if (tcpconn_try_unhash(tcpconn))
3155
-						tcpconn_put_destroy(tcpconn);
3156 3161
 					break;
3157 3162
 				}else{
3158 3163
 					crt_timeout=MIN_unsigned(con_lifetime,
... ...
@@ -3177,14 +3195,22 @@ inline static int handle_tcp_child(struct tcp_child* tcp_c, int fd_i)
3177 3177
 				LOG(L_CRIT, "ERROR: tcp_main: handle_tcp_child: failed to add"
3178 3178
 						" new socket to the fd list\n");
3179 3179
 				tcpconn->flags&=~F_CONN_READ_W;
3180
+				if (tcpconn_try_unhash(tcpconn)) {
3180 3181
 #ifdef TCP_ASYNC
3181
-				if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){
3182
-					io_watch_del(&io_h, tcpconn->s, -1, IO_FD_CLOSING);
3182
+					if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){
3183
+						io_watch_del(&io_h, tcpconn->s, -1, IO_FD_CLOSING);
3184
+						tcpconn->flags&=~F_CONN_WRITE_W;
3185
+					}
3186
+#endif /* TCP_ASYNC */
3187
+					tcpconn_put_destroy(tcpconn);
3188
+				}
3189
+#ifdef TCP_ASYNC
3190
+				 else if (unlikely(tcpconn->flags & F_CONN_WRITE_W)) {
3191
+					BUG("unhashed connection watched for write\n");
3192
+					io_watch_del(&io_h, tcpconn->s, -1, 0);
3183 3193
 					tcpconn->flags&=~F_CONN_WRITE_W;
3184 3194
 				}
3185 3195
 #endif /* TCP_ASYNC */
3186
-				if (tcpconn_try_unhash(tcpconn))
3187
-					tcpconn_put_destroy(tcpconn);
3188 3196
 				break;
3189 3197
 			}
3190 3198
 			DBG("handle_tcp_child: CONN_RELEASE  %p refcnt= %d\n", 
... ...
@@ -3457,10 +3483,16 @@ inline static int handle_ser_child(struct process_table* p, int fd_i)
3457 3457
 													POLLIN|POLLOUT, -1)<0)){
3458 3458
 							LOG(L_CRIT, "ERROR: tcp_main: handle_ser_child:"
3459 3459
 									" failed to change socket watch events\n");
3460
-							io_watch_del(&io_h, tcpconn->s, -1, IO_FD_CLOSING);
3461
-							tcpconn->flags&=~F_CONN_READ_W;
3462
-							if (tcpconn_try_unhash(tcpconn))
3460
+							if (tcpconn_try_unhash(tcpconn)) {
3461
+								io_watch_del(&io_h, tcpconn->s, -1,
3462
+												IO_FD_CLOSING);
3463
+								tcpconn->flags&=~F_CONN_READ_W;
3463 3464
 								tcpconn_put_destroy(tcpconn);
3465
+							} else {
3466
+								BUG("unhashed connection watched for IO\n");
3467
+								io_watch_del(&io_h, tcpconn->s, -1, 0);
3468
+								tcpconn->flags&=~F_CONN_READ_W;
3469
+							}
3464 3470
 							break;
3465 3471
 						}
3466 3472
 					}
... ...
@@ -3782,10 +3814,6 @@ inline static int handle_tcpconn_ev(struct tcp_connection* tcpconn, short ev,
3782 3782
 					(wbufq_run(tcpconn->s, tcpconn, &empty_q)<0) ||
3783 3783
 					(empty_q && tcpconn_close_after_send(tcpconn))
3784 3784
 			)){
3785
-			if (unlikely(io_watch_del(&io_h, tcpconn->s, fd_i, 0)<0)){
3786
-				LOG(L_ERR, "ERROR: handle_tcpconn_ev: io_watch_del(1) failed:"
3787
-							" for %p, fd %d\n", tcpconn, tcpconn->s);
3788
-			}
3789 3785
 			if ((tcpconn->flags & F_CONN_READ_W) && (ev & POLLIN)){
3790 3786
 				/* connection is watched for read and there is a read event
3791 3787
 				 * (unfortunately if we have POLLIN here we don't know if 
... ...
@@ -3798,6 +3826,11 @@ inline static int handle_tcpconn_ev(struct tcp_connection* tcpconn, short ev,
3798 3798
 				 * conn.  to a a child only if needed (another syscall + at 
3799 3799
 				 * least 2 * syscalls in the reader + ...) */
3800 3800
 				if ((ioctl(tcpconn->s, FIONREAD, &bytes)>=0) && (bytes>0)){
3801
+					if (unlikely(io_watch_del(&io_h, tcpconn->s, fd_i, 0)<0)){
3802
+						LOG(L_ERR, "ERROR: handle_tcpconn_ev: io_watch_del(1)"
3803
+								" failed: for %p, fd %d\n",
3804
+								tcpconn, tcpconn->s);
3805
+					}
3801 3806
 					tcpconn->flags&=~(F_CONN_WRITE_W|F_CONN_READ_W|
3802 3807
 										F_CONN_WANTS_RD|F_CONN_WANTS_WR);
3803 3808
 					tcpconn->flags|=F_CONN_FORCE_EOF|F_CONN_WR_ERROR;
... ...
@@ -3805,6 +3838,11 @@ inline static int handle_tcpconn_ev(struct tcp_connection* tcpconn, short ev,
3805 3805
 				}
3806 3806
 				/* if bytes==0 or ioctl failed, destroy the connection now */
3807 3807
 			}
3808
+			if (unlikely(io_watch_del(&io_h, tcpconn->s, fd_i,
3809
+											IO_FD_CLOSING) < 0)){
3810
+				LOG(L_ERR, "ERROR: handle_tcpconn_ev: io_watch_del() failed:"
3811
+							" for %p, fd %d\n", tcpconn, tcpconn->s);
3812
+			}
3808 3813
 			tcpconn->flags&=~(F_CONN_WRITE_W|F_CONN_READ_W|
3809 3814
 								F_CONN_WANTS_RD|F_CONN_WANTS_WR);
3810 3815
 			if (unlikely(ev & POLLERR)){
... ...
@@ -3898,7 +3936,8 @@ send_to_child:
3898 3898
 			tcpconn->flags&=~F_CONN_READER;
3899 3899
 #ifdef TCP_ASYNC
3900 3900
 			if (tcpconn->flags & F_CONN_WRITE_W){
3901
-				if (unlikely(io_watch_del(&io_h, tcpconn->s, fd_i, 0)<0)){
3901
+				if (unlikely(io_watch_del(&io_h, tcpconn->s, fd_i,
3902
+														IO_FD_CLOSING) < 0)){
3902 3903
 					LOG(L_ERR, "ERROR: handle_tcpconn_ev: io_watch_del(4)"
3903 3904
 							" failed:" " for %p, fd %d\n",
3904 3905
 							tcpconn, tcpconn->s);