From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id B81642674B for ; Sat, 9 Jun 2018 13:49:51 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ON_6DfM5cXrH for ; Sat, 9 Jun 2018 13:49:51 -0400 (EDT) Received: from smtp54.i.mail.ru (smtp54.i.mail.ru [217.69.128.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 6D73426490 for ; Sat, 9 Jun 2018 13:49:51 -0400 (EDT) From: Vladislav Shpilevoy Subject: [tarantool-patches] [PATCH 2/2] session: fix box.session.sync() Date: Sat, 9 Jun 2018 20:49:49 +0300 Message-Id: <5bba142da513a6224262993a473b57f54e0c8bc3.1528566238.git.v.shpilevoy@tarantool.org> In-Reply-To: References: In-Reply-To: References: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: kostja@tarantool.org 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)