[PATCH v3 2/2] session: outdate a session of a closed connection

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Fri Dec 21 19:48:41 MSK 2018


Once a connection is closed, a long-running user
request can not learn about this occasion. Even
box.sesion.push() still works.

This patch makes such disconnected session 'rotten'.
So a user can determine if a connection is closed by
looking at session.fd() == -1, or checking for
errors from box.session.push().

Closes #3859
---
 src/box/errcode.h      |  2 +-
 src/box/iproto.cc      |  9 +++++---
 src/box/session.cc     | 23 +++++++++++++++++++
 src/box/session.h      |  7 ++++++
 test/box/misc.result   |  1 +
 test/box/push.result   | 50 ++++++++++++++++++++++++++++++++++++++++++
 test/box/push.test.lua | 22 +++++++++++++++++++
 7 files changed, 110 insertions(+), 4 deletions(-)

diff --git a/src/box/errcode.h b/src/box/errcode.h
index 73359ebdf..9d3e89f85 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -138,7 +138,7 @@ struct errcode_record {
 	/* 83 */_(ER_ROLE_EXISTS,		"Role '%s' already exists") \
 	/* 84 */_(ER_CREATE_ROLE,		"Failed to create role '%s': %s") \
 	/* 85 */_(ER_INDEX_EXISTS,		"Index '%s' already exists") \
-	/* 86 */_(ER_UNUSED6,			"") \
+	/* 86 */_(ER_SESSION_CLOSED,		"Session is closed") \
 	/* 87 */_(ER_ROLE_LOOP,			"Granting role '%s' to role '%s' would create a loop") \
 	/* 88 */_(ER_GRANT,			"Incorrect grant arguments: %s") \
 	/* 89 */_(ER_PRIV_GRANTED,		"User '%s' already has %s access on %s '%s'") \
diff --git a/src/box/iproto.cc b/src/box/iproto.cc
index 3edfd8f28..1debc3c84 100644
--- a/src/box/iproto.cc
+++ b/src/box/iproto.cc
@@ -1246,9 +1246,12 @@ 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);
+	if (con->session != NULL) {
+		session_close(con->session);
+		if (! rlist_empty(&session_on_disconnect)) {
+			tx_fiber_init(con->session, 0);
+			session_run_on_disconnect_triggers(con->session);
+		}
 	}
 }
 
diff --git a/src/box/session.cc b/src/box/session.cc
index 0ec118bca..70822e8fd 100644
--- a/src/box/session.cc
+++ b/src/box/session.cc
@@ -92,10 +92,33 @@ session_on_stop(struct trigger *trigger, void * /* event */)
 	session_destroy(fiber_get_session(fiber()));
 }
 
+static int
+closed_session_push(struct session *session, uint64_t sync, struct port *port)
+{
+	(void) session;
+	(void) sync;
+	(void) port;
+	diag_set(ClientError, ER_SESSION_CLOSED);
+	return -1;
+}
+
+static struct session_vtab closed_session_vtab = {
+	/* .push = */ closed_session_push,
+	/* .fd = */ generic_session_fd,
+	/* .sync = */ generic_session_sync,
+};
+
+void
+session_close(struct session *session)
+{
+	session->vtab = &closed_session_vtab;
+}
+
 void
 session_set_type(struct session *session, enum session_type type)
 {
 	assert(type < session_type_MAX);
+	assert(session->vtab != &closed_session_vtab);
 	session->type = type;
 	session->vtab = &session_vtab_registry[type];
 }
diff --git a/src/box/session.h b/src/box/session.h
index d86939e48..3a7397146 100644
--- a/src/box/session.h
+++ b/src/box/session.h
@@ -146,6 +146,13 @@ extern struct session_vtab session_vtab_registry[];
 void
 session_set_type(struct session *session, enum session_type type);
 
+/**
+ * Close a session. It will return errors from all virtual methods
+ * and its type is fixed.
+ */
+void
+session_close(struct session *session);
+
 /**
  * Find a session by id.
  */
diff --git a/test/box/misc.result b/test/box/misc.result
index d266bb334..9fecbce76 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -415,6 +415,7 @@ t;
   83: box.error.ROLE_EXISTS
   84: box.error.CREATE_ROLE
   85: box.error.INDEX_EXISTS
+  86: box.error.SESSION_CLOSED
   87: box.error.ROLE_LOOP
   88: box.error.GRANT
   89: box.error.PRIV_GRANTED
diff --git a/test/box/push.result b/test/box/push.result
index af730c1a7..8919a3f5b 100644
--- a/test/box/push.result
+++ b/test/box/push.result
@@ -497,6 +497,56 @@ box.schema.func.drop('do_pushes')
 s:drop()
 ---
 ...
+--
+-- gh-3859: box.session.push() succeeds even after the connection
+-- is closed.
+--
+chan_push = fiber.channel()
+---
+...
+chan_disconnected = fiber.channel()
+---
+...
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+function do_long_and_push()
+    box.session.on_disconnect(function() chan_disconnected:put(true) end)
+    chan_push:get()
+    ok, err = box.session.push(100)
+    chan_push:put(err)
+end;
+---
+...
+test_run:cmd("setopt delimiter ''");
+---
+- true
+...
+box.schema.func.create('do_long_and_push')
+---
+...
+box.schema.user.grant('guest', 'execute', 'function', 'do_long_and_push')
+---
+...
+f = fiber.create(function() c:call('do_long_and_push') end)
+---
+...
 c:close()
 ---
 ...
+chan_disconnected:get()
+---
+- true
+...
+chan_push:put(true)
+---
+- true
+...
+chan_push:get()
+---
+- Session is closed
+...
+box.schema.func.drop('do_long_and_push')
+---
+...
diff --git a/test/box/push.test.lua b/test/box/push.test.lua
index 893cf0153..514c08b3e 100644
--- a/test/box/push.test.lua
+++ b/test/box/push.test.lua
@@ -237,4 +237,26 @@ keys
 box.schema.func.drop('do_push_and_duplicate')
 box.schema.func.drop('do_pushes')
 s:drop()
+
+--
+-- gh-3859: box.session.push() succeeds even after the connection
+-- is closed.
+--
+chan_push = fiber.channel()
+chan_disconnected = fiber.channel()
+test_run:cmd("setopt delimiter ';'")
+function do_long_and_push()
+    box.session.on_disconnect(function() chan_disconnected:put(true) end)
+    chan_push:get()
+    ok, err = box.session.push(100)
+    chan_push:put(err)
+end;
+test_run:cmd("setopt delimiter ''");
+box.schema.func.create('do_long_and_push')
+box.schema.user.grant('guest', 'execute', 'function', 'do_long_and_push')
+f = fiber.create(function() c:call('do_long_and_push') end)
 c:close()
+chan_disconnected:get()
+chan_push:put(true)
+chan_push:get()
+box.schema.func.drop('do_long_and_push')
-- 
2.17.2 (Apple Git-113)




More information about the Tarantool-patches mailing list