From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladislav Shpilevoy Subject: [PATCH 2/4] iproto: fire on_disconnect right after disconnect Date: Fri, 7 Dec 2018 18:46:33 +0300 Message-Id: <0fd1142f244d285cf74a8e2045f182ade4694020.1544197465.git.v.shpilevoy@tarantool.org> In-Reply-To: References: In-Reply-To: References: To: tarantool-patches@freelists.org Cc: vdavydov.dev@gmail.com List-ID: Before the patch on_disconnect triggers were called only after last outstanding request had finished. It was enough for most goals. But after box.session.push was implemented, it appeared that it is impossible to return an error from push() if a connection is closed. Tx session just does not know that a connection is already closed. The patch splits destroy and disconnect phases of an iproto connection lifecycle. Disconnect calls on_disconnect triggers and resets iproto socket fd. Destroy frees resources. Needed for #3859 --- src/box/iproto.cc | 43 ++++++++++++++++++++++++++++++++------- test/box/net.box.result | 21 ++++++++++++++----- test/box/net.box.test.lua | 13 +++++++++--- 3 files changed, 62 insertions(+), 15 deletions(-) diff --git a/src/box/iproto.cc b/src/box/iproto.cc index 40d456374..19ee67958 100644 --- a/src/box/iproto.cc +++ b/src/box/iproto.cc @@ -281,6 +281,14 @@ static const struct cmsg_hop destroy_route[] = { { net_finish_destroy, NULL }, }; +/** Fire on_disconnect triggers in the tx thread. */ +static void +tx_process_disconnect(struct cmsg *m); + +static const struct cmsg_hop disconnect_route[] = { + { tx_process_disconnect, NULL } +}; + /** * Kharon is in the dead world (iproto). Schedule an event to * flush new obuf as reflected in the fresh wpos. @@ -397,7 +405,19 @@ struct iproto_connection /** Logical session. */ struct session *session; ev_loop *loop; - /** Pre-allocated destroy msg. */ + /** + * Pre-allocated disconnect msg. Is sent right after + * actual disconnect has happened. Does not destroy the + * connection. Used to notify existing requests about the + * occasion. + */ + struct cmsg disconnect_msg; + /** + * Pre-allocated destroy msg. Is sent after disconnect has + * happened and a last request has finished. Firstly + * destroys tx-related resources and then deletes the + * connection. + */ struct cmsg destroy_msg; /** True if destroy message is sent. Debug-only. */ bool is_destroy_sent; @@ -562,6 +582,7 @@ iproto_connection_close(struct iproto_connection *con) * is done only once. */ con->p_ibuf->wpos -= con->parse_size; + cpipe_push(&tx_pipe, &con->disconnect_msg); } /* * If the connection has no outstanding requests in the @@ -993,6 +1014,7 @@ iproto_connection_new(int fd) rlist_create(&con->in_stop_list); /* It may be very awkward to allocate at close. */ cmsg_init(&con->destroy_msg, destroy_route); + cmsg_init(&con->disconnect_msg, disconnect_route); con->is_destroy_sent = false; con->tx.is_push_pending = false; con->tx.is_push_sent = false; @@ -1198,10 +1220,20 @@ tx_fiber_init(struct session *session, uint64_t sync) fiber_set_user(f, &session->credentials); } +static void +tx_process_disconnect(struct cmsg *m) +{ + struct iproto_connection *con = + container_of(m, struct iproto_connection, disconnect_msg); + if (con->session != NULL && !rlist_empty(&session_on_disconnect)) { + tx_fiber_init(con->session, 0); + session_run_on_disconnect_triggers(con->session); + } +} + /** - * Fire on_disconnect triggers in the tx - * thread and destroy the session object, - * as well as output buffers of the connection. + * Destroy the session object, as well as output buffers of the + * connection. */ static void tx_process_destroy(struct cmsg *m) @@ -1209,9 +1241,6 @@ tx_process_destroy(struct cmsg *m) struct iproto_connection *con = container_of(m, struct iproto_connection, destroy_msg); if (con->session) { - tx_fiber_init(con->session, 0); - if (! rlist_empty(&session_on_disconnect)) - session_run_on_disconnect_triggers(con->session); session_destroy(con->session); con->session = NULL; /* safety */ } diff --git a/test/box/net.box.result b/test/box/net.box.result index 6e59d0bc0..62534a545 100644 --- a/test/box/net.box.result +++ b/test/box/net.box.result @@ -2261,6 +2261,11 @@ box.session.on_disconnect(on_disconnect) == on_disconnect --- - true ... +-- +-- gh-3859: on_disconnect is called only after all requests are +-- processed, but should be called right after disconnect and +-- only once. +-- ch1 = fiber.channel(1) --- ... @@ -2283,21 +2288,27 @@ c:close() fiber.sleep(0) --- ... -disconnected -- false +while disconnected == false do fiber.sleep(0.01) end --- -- false ... -ch2:put(true) +disconnected -- true --- - true ... -while disconnected == false do fiber.sleep(0.01) end +disconnected = nil --- ... -disconnected -- true +ch2:put(true) --- - true ... +fiber.sleep(0) +--- +... +disconnected -- nil, on_disconnect is not called second time. +--- +- null +... box.session.on_disconnect(nil, on_disconnect) --- ... diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua index 26773aac9..d34d7affa 100644 --- a/test/box/net.box.test.lua +++ b/test/box/net.box.test.lua @@ -934,6 +934,11 @@ fiber.sleep(0) box.session.on_disconnect(on_disconnect) == on_disconnect +-- +-- gh-3859: on_disconnect is called only after all requests are +-- processed, but should be called right after disconnect and +-- only once. +-- ch1 = fiber.channel(1) ch2 = fiber.channel(1) function wait_signal() ch1:put(true) ch2:get() end @@ -942,11 +947,13 @@ ch1:get() c:close() fiber.sleep(0) -disconnected -- false - -ch2:put(true) while disconnected == false do fiber.sleep(0.01) end disconnected -- true +disconnected = nil + +ch2:put(true) +fiber.sleep(0) +disconnected -- nil, on_disconnect is not called second time. box.session.on_disconnect(nil, on_disconnect) -- 2.17.2 (Apple Git-113)