Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@freelists.org
Cc: vdavydov.dev@gmail.com
Subject: [PATCH 2/4] iproto: fire on_disconnect right after disconnect
Date: Fri,  7 Dec 2018 18:46:33 +0300	[thread overview]
Message-ID: <0fd1142f244d285cf74a8e2045f182ade4694020.1544197465.git.v.shpilevoy@tarantool.org> (raw)
In-Reply-To: <cover.1544197465.git.v.shpilevoy@tarantool.org>
In-Reply-To: <cover.1544197465.git.v.shpilevoy@tarantool.org>

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)

  parent reply	other threads:[~2018-12-07 15:46 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-07 15:46 [PATCH 0/4] Outdate disconnected session Vladislav Shpilevoy
2018-12-07 15:46 ` [PATCH 1/4] iproto: rename disconnect cmsg to destroy Vladislav Shpilevoy
2018-12-07 17:36   ` [tarantool-patches] " Konstantin Osipov
2018-12-11 12:54     ` [tarantool-patches] " Vladislav Shpilevoy
2018-12-07 15:46 ` Vladislav Shpilevoy [this message]
2018-12-07 17:37   ` [tarantool-patches] [PATCH 2/4] iproto: fire on_disconnect right after disconnect Konstantin Osipov
2018-12-11 12:55     ` [tarantool-patches] " Vladislav Shpilevoy
2018-12-07 15:46 ` [PATCH 3/4] session: introduce 'dead' type Vladislav Shpilevoy
2018-12-07 17:38   ` [tarantool-patches] " Konstantin Osipov
2018-12-07 20:40     ` [tarantool-patches] " Vladislav Shpilevoy
2018-12-07 22:21       ` Konstantin Osipov
2018-12-07 22:42         ` Vladislav Shpilevoy
2018-12-07 15:46 ` [PATCH 4/4] session: kill a session of a closed connection Vladislav Shpilevoy
2018-12-07 17:35 ` [tarantool-patches] [PATCH 0/4] Outdate disconnected session Konstantin Osipov
2018-12-11 16:12   ` [tarantool-patches] " Vladislav Shpilevoy
2018-12-07 17:41 ` [tarantool-patches] " Konstantin Osipov
2018-12-11 16:13   ` [tarantool-patches] " Vladislav Shpilevoy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0fd1142f244d285cf74a8e2045f182ade4694020.1544197465.git.v.shpilevoy@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=vdavydov.dev@gmail.com \
    --subject='Re: [PATCH 2/4] iproto: fire on_disconnect right after disconnect' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox