[tarantool-patches] [PATCH 2/2] session: fix box.session.sync()

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Sat Jun 9 20:49:49 MSK 2018


Before the patch box.session.sync() is global for the session and
is updated on each new iproto request. When the connection is
multiplexed, box.session.sync() can be changed with no finishing
a current request, if a new one arrives.

The patch makes box.session.push() local for the request,
protecting it from multiplexing mess. Box.session.sync() after
the patch can be safely used inside a request.

Closes #3450

@TarantoolBot document
Title: box.session.sync() became request local
Box.session.sync() was global for a session, so it was unusable
when the connection behind the session is multiplexed. Now
box.session.sync() is request local and can be safely used inside
the request processor.
---
 src/box/iproto.cc             | 11 +++++++----
 src/box/session.h             | 17 ++++++-----------
 src/fiber.h                   | 27 ++++++++++++++++++---------
 test/box-tap/session.test.lua | 29 ++++++++++++++++++++++++++++-
 4 files changed, 59 insertions(+), 25 deletions(-)

diff --git a/src/box/iproto.cc b/src/box/iproto.cc
index 0102aec4b..759945278 100644
--- a/src/box/iproto.cc
+++ b/src/box/iproto.cc
@@ -1171,7 +1171,8 @@ error:
 static void
 tx_fiber_init(struct session *session, uint64_t sync)
 {
-	session->meta.sync = sync;
+	struct fiber *f = fiber();
+	f->storage.net.sync = sync;
 	/*
 	 * We do not cleanup fiber keys at the end of each request.
 	 * This does not lead to privilege escalation as long as
@@ -1182,8 +1183,8 @@ tx_fiber_init(struct session *session, uint64_t sync)
 	 * background tasks clean up their session in on_stop
 	 * trigger as well.
 	 */
-	fiber_set_session(fiber(), session);
-	fiber_set_user(fiber(), &session->credentials);
+	fiber_set_session(f, session);
+	fiber_set_user(f, &session->credentials);
 }
 
 /**
@@ -1818,7 +1819,9 @@ iproto_session_fd(struct session *session)
 int64_t
 iproto_session_sync(struct session *session)
 {
-	return session->meta.sync;
+	(void) session;
+	assert(session == fiber()->storage.session);
+	return fiber()->storage.net.sync;
 }
 
 /** {{{ IPROTO_PUSH implementation. */
diff --git a/src/box/session.h b/src/box/session.h
index 17190ab47..6ed97dc46 100644
--- a/src/box/session.h
+++ b/src/box/session.h
@@ -66,16 +66,11 @@ extern const char *session_type_strs[];
  * types, and allows to do not store attributes in struct session,
  * that are used only by a session of particular type.
  */
-struct session_meta {
-	union {
-		/** IProto connection meta. */
-		struct {
-			uint64_t sync;
-			void *connection;
-		};
-		/** Only by console is used. */
-		int fd;
-	};
+union session_meta {
+	/** IProto connection. */
+	void *connection;
+	/** Console file/socket descriptor. */
+	int fd;
 };
 
 /**
@@ -92,7 +87,7 @@ struct session {
 	uint64_t id;
 	enum session_type type;
 	/** Session metadata. */
-	struct session_meta meta;
+	union session_meta meta;
 	/** Session user id and global grants */
 	struct credentials credentials;
 	/** Trigger for fiber on_stop to cleanup created on-demand session */
diff --git a/src/fiber.h b/src/fiber.h
index b7003aeb6..021929822 100644
--- a/src/fiber.h
+++ b/src/fiber.h
@@ -393,15 +393,24 @@ struct fiber {
 		struct session *session;
 		struct credentials *credentials;
 		struct txn *txn;
-		/**
-		 * Fields used by a fiber created in Lua: Lua
-		 * stack and the optional fiber.storage Lua
-		 * reference.
-		 */
-		struct {
-			struct lua_State *stack;
-			int ref;
-		} lua;
+		union {
+			/**
+			 * Fields used by a fiber created in Lua:
+			 * Lua stack and the optional
+			 * fiber.storage Lua reference.
+			 */
+			struct {
+				struct lua_State *stack;
+				int ref;
+			} lua;
+			/**
+			 * Fields used by a fiber created to
+			 * process an iproto request.
+			 */
+			struct {
+				uint64_t sync;
+			} net;
+		};
 	} storage;
 	/** An object to wait for incoming message or a reader. */
 	struct ipc_wait_pad *wait_pad;
diff --git a/test/box-tap/session.test.lua b/test/box-tap/session.test.lua
index 6fddced3c..c3c07a67c 100755
--- a/test/box-tap/session.test.lua
+++ b/test/box-tap/session.test.lua
@@ -15,7 +15,7 @@ session = box.session
 space = box.schema.space.create('tweedledum')
 index = space:create_index('primary', { type = 'hash' })
 
-test:plan(53)
+test:plan(55)
 
 ---
 --- Check that Tarantool creates ADMIN session for #! script
@@ -183,6 +183,33 @@ local sp = conn:eval("return box.space._space.index.name:get{\"sp1\"}[2]")
 test:is(sp, 1, "effective ddl owner")
 conn:close()
 
+--
+-- gh-3450: box.session.sync() becomes request local.
+--
+cond = fiber.cond()
+local sync1, sync2
+local started = 0
+function f1()
+	started = started + 1
+	cond:wait()
+	sync1 = box.session.sync()
+end
+function f2()
+	started = started + 1
+	sync2 = box.session.sync()
+	cond:signal()
+end
+box.schema.user.grant('guest', 'read,write,execute', 'universe')
+conn = net.box.connect(box.cfg.listen)
+test:ok(conn:ping(), 'connect to self')
+_ = fiber.create(function() conn:call('f1') end)
+while started ~= 1 do fiber.sleep(0.01) end
+_ = fiber.create(function() conn:call('f2') end)
+while started ~= 2 do fiber.sleep(0.01) end
+test:isnt(sync1, sync2, 'session.sync() is request local')
+conn:close()
+box.schema.user.revoke('guest', 'read,write,execute', 'universe')
+
 inspector:cmd('stop server session with cleanup=1')
 session = nil
 os.exit(test:check() == true and 0 or -1)
-- 
2.15.1 (Apple Git-101)





More information about the Tarantool-patches mailing list