diff --git a/src/channel/cc_tcp.c b/src/channel/cc_tcp.c index 0cb1fcc46..325557274 100644 --- a/src/channel/cc_tcp.c +++ b/src/channel/cc_tcp.c @@ -319,33 +319,63 @@ _tcp_accept(struct tcp_conn *sc) { int sd; - ASSERT(sc->sd > 0); - - for (;;) { /* we accept at most one tcp_conn with the 'break' at the end */ + ASSERT(sc->sd >= 0); + + /* How does tcp accept work when a separate thread is used to accept new + * connections? + * + * In general, we want to accept a new connection at a time (on the server + * thread), then hand this connection over to be put on some other event + * loop (e.g. on a worker thread's), and some additional preparation may + * be necessary (e.g. allocating R/W buffers). This is why we break after + * completing a single `accept' successfully. + * + * There are several ways `accept' could "fail", and they need to be + * treated differently. The most common case, which isn't really a failure + * on a non-blocking socket, is receiving EAGAIN/EWOULDBLOCK. This simply + * means there is no new connection to accept and the function should + * return. + * EINTR is another common error which means the call was terminated by + * a signal. This type of interruption is almost always transient, so + * an immediate retry is likely to succeed. + * The rest of the exceptions likely to occur on a SOCK_STREAM socket are + * often due to exhaustion of some resources (e.g. fd, memory), and there + * is no guarantee they will recover immediately. For example, to free + * up another fd requires an existing connection to be closed. In such + * cases, the connection in the backlog will sit there (as fully + * established as far as TCP stack is concerned) until accept in application + * becomes possible again, and any new connections arriving will be added + * to the back of the queue until it's full, at which point the client + * will receive an exception and the connect attempt will fail. + */ + for (;;) { #ifdef CC_ACCEPT4 sd = accept4(sc->sd, NULL, NULL, SOCK_NONBLOCK); #else sd = accept(sc->sd, NULL, NULL); #endif /* CC_ACCEPT4 */ if (sd < 0) { - if (errno == EINTR) { - log_debug("accept on sd %d not ready: eintr", sc->sd); - continue; - } - if (errno == EAGAIN || errno == EWOULDBLOCK) { log_debug("accept on sd %d not ready: eagain", sc->sd); return -1; } + if (errno == EINTR) { + log_debug("accept on sd %d not ready: eintr", sc->sd); + + continue; + } + log_error("accept on sd %d failed: %s", sc->sd, strerror(errno)); INCR(tcp_metrics, tcp_accept_ex); - continue; + + return -1; } break; } + ASSERT(sd >= 0); return sd; } @@ -423,19 +453,21 @@ tcp_reject_all(struct tcp_conn *sc) for (;;) { sd = accept(sc->sd, NULL, NULL); if (sd < 0) { - if (errno == EINTR) { - log_debug("sd %d not ready: eintr", sc->sd); - continue; - } - if (errno == EAGAIN || errno == EWOULDBLOCK) { log_debug("sd %d has no more outstanding connections", sc->sd); return; } + if (errno == EINTR) { + log_debug("accept on sd %d not ready: eintr", sc->sd); + + continue; + } + log_error("accept on sd %d failed: %s", sc->sd, strerror(errno)); INCR(tcp_metrics, tcp_reject_ex); - return; + + return -1; } ret = close(sd); diff --git a/src/event/cc_epoll.c b/src/event/cc_epoll.c index 12cc47c4e..25dbdaeb6 100644 --- a/src/event/cc_epoll.c +++ b/src/event/cc_epoll.c @@ -122,9 +122,6 @@ _event_update(struct event_base *evb, int fd, int op, uint32_t events, void *ptr { struct epoll_event event; - ASSERT(evb != NULL && evb->ep > 0); - ASSERT(fd > 0); - event.events = events; event.data.ptr = ptr; @@ -135,6 +132,9 @@ int event_add_read(struct event_base *evb, int fd, void *data) { int status; + ASSERT(evb != NULL && evb->ep > 0); + ASSERT(fd >= 0); + /* * Note(yao): there have been tests showing EPOLL_CTL_ADD is cheaper than * EPOLL_CTL_MOD, and the only difference is we need to ignore EEXIST @@ -156,6 +156,9 @@ event_add_write(struct event_base *evb, int fd, void *data) { int status; + ASSERT(evb != NULL && evb->ep > 0); + ASSERT(fd >= 0); + status = _event_update(evb, fd, EPOLL_CTL_ADD, EPOLLOUT, data); if (status < 0 && errno != EEXIST) { log_error("ctl (add write) w/ epoll fd %d on fd %d failed: %s", evb->ep, @@ -173,6 +176,9 @@ event_del(struct event_base *evb, int fd) { int status; + ASSERT(evb != NULL && evb->ep > 0); + ASSERT(fd >= 0); + /* event can be NULL in kernel >=2.6.9, here we keep it for compatibility */ status = _event_update(evb, fd, EPOLL_CTL_DEL, 0, NULL); if (status < 0) {