* [PATCH v3 1/2] session: store vtab both in struct and registry
2018-12-21 16:48 [PATCH v3 0/2] Outdate binary session on disconnect Vladislav Shpilevoy
@ 2018-12-21 16:48 ` Vladislav Shpilevoy
2018-12-21 16:48 ` [PATCH v3 2/2] session: outdate a session of a closed connection Vladislav Shpilevoy
2018-12-24 10:07 ` [PATCH v3 0/2] Outdate binary session on disconnect Vladimir Davydov
2 siblings, 0 replies; 4+ messages in thread
From: Vladislav Shpilevoy @ 2018-12-21 16:48 UTC (permalink / raw)
To: tarantool-patches; +Cc: vdavydov.dev
Before the patch vtabs were stored in a global array
only, called registry and accessed by session.type to
call a virtual method. But it does not allow to change
session vtab without affecting session type. This
feature is necessary to be able to outdate binary
sessions whose socket is closed.
Outdated, closed, sessions will return errors from all
its methods.
Needed for #3859
---
src/box/applier.cc | 5 ++++-
src/box/lua/session.c | 3 ++-
src/box/session.cc | 10 +++++++++-
src/box/session.h | 12 +++++++++---
4 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/src/box/applier.cc b/src/box/applier.cc
index ff4af95e5..21d2e6bcb 100644
--- a/src/box/applier.cc
+++ b/src/box/applier.cc
@@ -587,7 +587,10 @@ applier_f(va_list ap)
* Set correct session type for use in on_replace()
* triggers.
*/
- current_session()->type = SESSION_TYPE_APPLIER;
+ struct session *session = session_create_on_demand();
+ if (session == NULL)
+ return -1;
+ session_set_type(session, SESSION_TYPE_APPLIER);
/* Re-connect loop */
while (!fiber_is_cancelled()) {
diff --git a/src/box/lua/session.c b/src/box/lua/session.c
index d3d27643f..a4ddb201c 100644
--- a/src/box/lua/session.c
+++ b/src/box/lua/session.c
@@ -57,7 +57,8 @@ lbox_session_create(struct lua_State *L)
session->meta.fd = luaL_optinteger(L, 1, -1);
}
/* If a session already exists, simply reset its type */
- session->type = STR2ENUM(session_type, luaL_optstring(L, 2, "console"));
+ session_set_type(session, STR2ENUM(session_type,
+ luaL_optstring(L, 2, "console")));
lua_pushnumber(L, session->id);
return 1;
diff --git a/src/box/session.cc b/src/box/session.cc
index 64714cdcf..0ec118bca 100644
--- a/src/box/session.cc
+++ b/src/box/session.cc
@@ -92,6 +92,14 @@ session_on_stop(struct trigger *trigger, void * /* event */)
session_destroy(fiber_get_session(fiber()));
}
+void
+session_set_type(struct session *session, enum session_type type)
+{
+ assert(type < session_type_MAX);
+ session->type = type;
+ session->vtab = &session_vtab_registry[type];
+}
+
struct session *
session_create(enum session_type type)
{
@@ -105,7 +113,7 @@ session_create(enum session_type type)
session->id = sid_max();
memset(&session->meta, 0, sizeof(session->meta));
- session->type = type;
+ session_set_type(session, type);
session->sql_flags = default_flags;
session->sql_default_engine = SQL_STORAGE_ENGINE_MEMTX;
diff --git a/src/box/session.h b/src/box/session.h
index df1dcbc62..d86939e48 100644
--- a/src/box/session.h
+++ b/src/box/session.h
@@ -97,6 +97,8 @@ struct session {
/** SQL Connection flag for current user session */
uint32_t sql_flags;
enum session_type type;
+ /** Session virtual methods. */
+ const struct session_vtab *vtab;
/** Session metadata. */
union session_meta meta;
/** Session user id and global grants */
@@ -140,6 +142,10 @@ struct session_vtab {
extern struct session_vtab session_vtab_registry[];
+/** Change session type and vtab. */
+void
+session_set_type(struct session *session, enum session_type type);
+
/**
* Find a session by id.
*/
@@ -302,19 +308,19 @@ access_check_universe(user_access_t access);
static inline int
session_push(struct session *session, uint64_t sync, struct port *port)
{
- return session_vtab_registry[session->type].push(session, sync, port);
+ return session->vtab->push(session, sync, port);
}
static inline int
session_fd(struct session *session)
{
- return session_vtab_registry[session->type].fd(session);
+ return session->vtab->fd(session);
}
static inline int
session_sync(struct session *session)
{
- return session_vtab_registry[session->type].sync(session);
+ return session->vtab->sync(session);
}
/**
--
2.17.2 (Apple Git-113)
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH v3 2/2] session: outdate a session of a closed connection
2018-12-21 16:48 [PATCH v3 0/2] Outdate binary session on disconnect Vladislav Shpilevoy
2018-12-21 16:48 ` [PATCH v3 1/2] session: store vtab both in struct and registry Vladislav Shpilevoy
@ 2018-12-21 16:48 ` Vladislav Shpilevoy
2018-12-24 10:07 ` [PATCH v3 0/2] Outdate binary session on disconnect Vladimir Davydov
2 siblings, 0 replies; 4+ messages in thread
From: Vladislav Shpilevoy @ 2018-12-21 16:48 UTC (permalink / raw)
To: tarantool-patches; +Cc: vdavydov.dev
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)
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v3 0/2] Outdate binary session on disconnect
2018-12-21 16:48 [PATCH v3 0/2] Outdate binary session on disconnect Vladislav Shpilevoy
2018-12-21 16:48 ` [PATCH v3 1/2] session: store vtab both in struct and registry Vladislav Shpilevoy
2018-12-21 16:48 ` [PATCH v3 2/2] session: outdate a session of a closed connection Vladislav Shpilevoy
@ 2018-12-24 10:07 ` Vladimir Davydov
2 siblings, 0 replies; 4+ messages in thread
From: Vladimir Davydov @ 2018-12-24 10:07 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches
On Fri, Dec 21, 2018 at 07:48:39PM +0300, Vladislav Shpilevoy wrote:
> Vladislav Shpilevoy (2):
> session: store vtab both in struct and registry
> session: outdate a session of a closed connection
Pushed to 2.1 and 1.10.
^ permalink raw reply [flat|nested] 4+ messages in thread