Merge pull request h2o#976 from h2o/kazuho/issues/966

Fix assertion failure: `sock->super._cb.write != ((void *)0)' failed`
ksss · Jul 6, 2016 · 2fb5c81 · 2fb5c81
2 parents 72f5bd7 + ea8b400
commit 2fb5c81
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 32 deletions.
diff --git a/include/h2o/socket/evloop.h b/include/h2o/socket/evloop.h
@@ -26,11 +26,11 @@
 
 #define H2O_SOCKET_FLAG_IS_DISPOSED 0x1
 #define H2O_SOCKET_FLAG_IS_READ_READY 0x2
-#define H2O_SOCKET_FLAG_IS_WRITE_ERROR 0x4
+#define H2O_SOCKET_FLAG_IS_WRITE_NOTIFY 0x4
 #define H2O_SOCKET_FLAG_IS_POLLED_FOR_READ 0x8
 #define H2O_SOCKET_FLAG_IS_POLLED_FOR_WRITE 0x10
 #define H2O_SOCKET_FLAG_DONT_READ 0x20
-#define H2O_SOCKET_FLAG_DONT_WRITE 0x40
+#define H2O_SOCKET_FLAG_IS_CONNECTING 0x40
 #define H2O_SOCKET_FLAG_IS_ACCEPTED_CONNECTION 0x80
 #define H2O_SOCKET_FLAG__EPOLL_IS_REGISTERED 0x1000
 

diff --git a/lib/common/socket/evloop.c.h b/lib/common/socket/evloop.c.h
@@ -192,13 +192,9 @@ void write_pending(struct st_h2o_evloop_socket_t *sock)
 {
     assert(sock->super._cb.write != NULL);
 
-    if ((sock->_flags & H2O_SOCKET_FLAG_DONT_WRITE) != 0) {
-        /* connection complete */
-        assert(sock->_wreq.cnt == 0);
+    /* DONT_WRITE poll */
+    if (sock->_wreq.cnt == 0)
         goto Complete;
-    }
-
-    assert(sock->_wreq.cnt != 0);
 
     /* write */
     if (write_core(sock->fd, &sock->_wreq.bufs, &sock->_wreq.cnt) == 0 && sock->_wreq.cnt != 0) {
@@ -208,13 +204,9 @@ void write_pending(struct st_h2o_evloop_socket_t *sock)
 
     /* either completed or failed */
     wreq_free_buffer_if_allocated(sock);
-    if (sock->_wreq.cnt != 0) {
-        /* pending data exists -> was an error */
-        sock->_wreq.cnt = 0; /* clear it ! */
-        sock->_flags |= H2O_SOCKET_FLAG_IS_WRITE_ERROR;
-    }
 
 Complete:
+    sock->_flags |= H2O_SOCKET_FLAG_IS_WRITE_NOTIFY;
     link_to_pending(sock);
     link_to_statechanged(sock); /* might need to disable the write polling */
 }
@@ -270,12 +262,17 @@ void do_write(h2o_socket_t *_sock, h2o_iovec_t *_bufs, size_t bufcnt, h2o_socket
 
     /* try to write now */
     if (write_core(sock->fd, &bufs, &bufcnt) != 0) {
-        sock->_flags |= H2O_SOCKET_FLAG_IS_WRITE_ERROR;
+        /* fill in _wreq.bufs with fake data to indicate error */
+        sock->_wreq.bufs = sock->_wreq.smallbufs;
+        sock->_wreq.cnt = 1;
+        *sock->_wreq.bufs = h2o_iovec_init(H2O_STRLIT("deadbeef"));
+        sock->_flags |= H2O_SOCKET_FLAG_IS_WRITE_NOTIFY;
         link_to_pending(sock);
         return;
     }
     if (bufcnt == 0) {
         /* write complete, schedule the callback */
+        sock->_flags |= H2O_SOCKET_FLAG_IS_WRITE_NOTIFY;
         link_to_pending(sock);
         return;
     }
@@ -434,9 +431,8 @@ h2o_socket_t *h2o_socket_connect(h2o_loop_t *loop, struct sockaddr *addr, sockle
         return NULL;
     }
 
-    sock = create_socket_set_nodelay(loop, fd, H2O_SOCKET_FLAG_DONT_WRITE);
-    sock->super._cb.write = cb;
-    link_to_statechanged(sock);
+    sock = create_socket_set_nodelay(loop, fd, H2O_SOCKET_FLAG_IS_CONNECTING);
+    h2o_socket_notify_write(&sock->super, cb);
     return &sock->super;
 }
 
@@ -481,9 +477,9 @@ void h2o_socket_notify_write(h2o_socket_t *_sock, h2o_socket_cb cb)
 {
     struct st_h2o_evloop_socket_t *sock = (struct st_h2o_evloop_socket_t *)_sock;
     assert(sock->super._cb.write == NULL);
+    assert(sock->_wreq.cnt == 0);
 
     sock->super._cb.write = cb;
-    sock->_flags |= H2O_SOCKET_FLAG_DONT_WRITE;
     link_to_statechanged(sock);
 }
 
@@ -499,21 +495,23 @@ static void run_socket(struct st_h2o_evloop_socket_t *sock)
         read_on_ready(sock);
     }
 
-    if (sock->super._cb.write != NULL && sock->_wreq.cnt == 0) {
+    if ((sock->_flags & H2O_SOCKET_FLAG_IS_WRITE_NOTIFY) != 0) {
         const char *err = NULL;
-        if ((sock->_flags & H2O_SOCKET_FLAG_DONT_WRITE) != 0) {
+        assert(sock->super._cb.write != NULL);
+        sock->_flags &= ~H2O_SOCKET_FLAG_IS_WRITE_NOTIFY;
+        if (sock->_wreq.cnt != 0) {
+            /* error */
+            err = h2o_socket_error_io;
+            sock->_wreq.cnt = 0;
+        } else if ((sock->_flags & H2O_SOCKET_FLAG_IS_CONNECTING) != 0) {
+            sock->_flags &= ~H2O_SOCKET_FLAG_IS_CONNECTING;
             int so_err = 0;
             socklen_t l = sizeof(so_err);
             so_err = 0;
-            getsockopt(sock->fd, SOL_SOCKET, SO_ERROR, &so_err, &l);
-            if (so_err != 0) {
+            if (getsockopt(sock->fd, SOL_SOCKET, SO_ERROR, &so_err, &l) != 0 || so_err != 0) {
                 /* FIXME lookup the error table */
                 err = h2o_socket_error_conn_fail;
             }
-            sock->_flags &= ~H2O_SOCKET_FLAG_DONT_WRITE;
-        } else {
-            if ((sock->_flags & H2O_SOCKET_FLAG_IS_WRITE_ERROR) != 0)
-                err = h2o_socket_error_io;
         }
         on_write_complete(&sock->super, err);
     }

diff --git a/lib/common/socket/evloop/epoll.c.h b/lib/common/socket/evloop/epoll.c.h
@@ -61,8 +61,7 @@ static int update_status(struct st_h2o_evloop_epoll_t *loop)
                     changed = 1;
                 }
             }
-            if (h2o_socket_is_writing(&sock->super) &&
-                (sock->_wreq.cnt != 0 || (sock->_flags & H2O_SOCKET_FLAG_DONT_WRITE) != 0)) {
+            if (h2o_socket_is_writing(&sock->super)) {
                 ev.events |= EPOLLOUT;
                 if ((sock->_flags & H2O_SOCKET_FLAG_IS_POLLED_FOR_WRITE) == 0) {
                     sock->_flags |= H2O_SOCKET_FLAG_IS_POLLED_FOR_WRITE;

diff --git a/lib/common/socket/evloop/kqueue.c.h b/lib/common/socket/evloop/kqueue.c.h
@@ -84,8 +84,7 @@ static int collect_status(struct st_h2o_socket_loop_kqueue_t *loop, struct keven
                     SET_AND_UPDATE(EVFILT_READ, EV_DELETE);
                 }
             }
-            if (h2o_socket_is_writing(&sock->super) &&
-                (sock->_wreq.cnt != 0 || (sock->_flags & H2O_SOCKET_FLAG_DONT_WRITE) != 0)) {
+            if (h2o_socket_is_writing(&sock->super)) {
                 if ((sock->_flags & H2O_SOCKET_FLAG_IS_POLLED_FOR_WRITE) == 0) {
                     sock->_flags |= H2O_SOCKET_FLAG_IS_POLLED_FOR_WRITE;
                     SET_AND_UPDATE(EVFILT_WRITE, EV_ADD);

diff --git a/lib/common/socket/evloop/poll.c.h b/lib/common/socket/evloop/poll.c.h
@@ -59,8 +59,7 @@ static void update_socks(struct st_h2o_evloop_poll_t *loop)
                 DEBUG_LOG("clearing READ for fd: %d\n", sock->fd);
                 sock->_flags &= ~H2O_SOCKET_FLAG_IS_POLLED_FOR_READ;
             }
-            if (h2o_socket_is_writing(&sock->super) &&
-                (sock->_wreq.cnt != 0 || (sock->_flags & H2O_SOCKET_FLAG_DONT_WRITE) != 0)) {
+            if (h2o_socket_is_writing(&sock->super)) {
                 DEBUG_LOG("setting WRITE for fd: %d\n", sock->fd);
                 sock->_flags |= H2O_SOCKET_FLAG_IS_POLLED_FOR_WRITE;
             } else {