From 815a486bc9a1b11f36e46c9f146c593e9e2c6637 Mon Sep 17 00:00:00 2001 From: Konstantin Osipov <kostja@tarantool.org> Date: Mon, 29 Jun 2015 18:42:14 +0300 Subject: [PATCH] iproto: fix rollback to the wrong state of the output of obuf In case output is multiplexed, obuf_rollback_to_svp could roll back to a very early in the output buffer, since svp is taken before a possible yield. This is a regression introduced in the patch which removes iproto_port from Lua calls. --- src/box/iproto.cc | 17 ++++++++-- src/box/lua/call.cc | 78 +++++++++++++++++++++++++-------------------- 2 files changed, 58 insertions(+), 37 deletions(-) diff --git a/src/box/iproto.cc b/src/box/iproto.cc index efe20f6d1b..b5f31c2059 100644 --- a/src/box/iproto.cc +++ b/src/box/iproto.cc @@ -666,7 +666,6 @@ iproto_process(struct iproto_task *task) return; task->connection->session->sync = task->header.sync; - struct obuf_svp svp = obuf_create_svp(out); try { switch (task->header.type) { case IPROTO_SELECT: @@ -677,7 +676,20 @@ iproto_process(struct iproto_task *task) assert(task->request.type == task->header.type); struct iproto_port port; iproto_port_init(&port, out, task->header.sync); - box_process(&task->request, (struct port *) &port); + try { + box_process(&task->request, (struct port *) &port); + } catch (Exception *e) { + /* + * This only works if there are no + * yields between the moment the + * port is first used for + * output and is flushed/an error + * occurs. + */ + if (port.found) + obuf_rollback_to_svp(out, &port.svp); + throw; + } break; case IPROTO_CALL: assert(task->request.type == task->header.type); @@ -728,7 +740,6 @@ iproto_process(struct iproto_task *task) (uint32_t) task->header.type); } } catch (Exception *e) { - obuf_rollback_to_svp(out, &svp); iproto_reply_error(out, e, task->header.sync); } } diff --git a/src/box/lua/call.cc b/src/box/lua/call.cc index 4e07896624..3c39533d1a 100644 --- a/src/box/lua/call.cc +++ b/src/box/lua/call.cc @@ -601,40 +601,45 @@ execute_call(lua_State *L, struct request *request, struct obuf *out) uint32_t count = 0; struct obuf_svp svp = iproto_prepare_select(out); - /** Check if we deal with a table of tables. */ - int nrets = lua_gettop(L); - if (nrets == 1 && lua_isarray(L, 1)) { - /* - * The table is not empty and consists of tables - * or tuples. Treat each table element as a tuple, - * and push it. - */ - lua_pushnil(L); - int has_keys = lua_next(L, 1); - if (has_keys && (lua_isarray(L, lua_gettop(L)) || lua_istuple(L, -1))) { - do { - luamp_encode_tuple(L, luaL_msgpack_default, - out, -1); - ++count; + try { + /** Check if we deal with a table of tables. */ + int nrets = lua_gettop(L); + if (nrets == 1 && lua_isarray(L, 1)) { + /* + * The table is not empty and consists of tables + * or tuples. Treat each table element as a tuple, + * and push it. + */ + lua_pushnil(L); + int has_keys = lua_next(L, 1); + if (has_keys && (lua_isarray(L, lua_gettop(L)) || lua_istuple(L, -1))) { + do { + luamp_encode_tuple(L, luaL_msgpack_default, + out, -1); + ++count; + lua_pop(L, 1); + } while (lua_next(L, 1)); + goto done; + } else if (has_keys) { lua_pop(L, 1); - } while (lua_next(L, 1)); - goto done; - } else if (has_keys) { - lua_pop(L, 1); + } } - } - for (int i = 1; i <= nrets; ++i) { - if (lua_isarray(L, i) || lua_istuple(L, i)) { - luamp_encode_tuple(L, luaL_msgpack_default, out, i); - } else { - luamp_encode_array(luaL_msgpack_default, out, 1); - luamp_encode(L, luaL_msgpack_default, out, i); + for (int i = 1; i <= nrets; ++i) { + if (lua_isarray(L, i) || lua_istuple(L, i)) { + luamp_encode_tuple(L, luaL_msgpack_default, out, i); + } else { + luamp_encode_array(luaL_msgpack_default, out, 1); + luamp_encode(L, luaL_msgpack_default, out, i); + } + ++count; } - ++count; - } -done: - iproto_reply_select(out, &svp, request->header->sync, count); + done: + iproto_reply_select(out, &svp, request->header->sync, count); + } catch (...) { + obuf_rollback_to_svp(out, &svp); + throw; + } } void @@ -685,11 +690,16 @@ execute_eval(lua_State *L, struct request *request, struct obuf *out) /* Send results of the called procedure to the client. */ struct obuf_svp svp = iproto_prepare_select(out); - int nrets = lua_gettop(L); - for (int k = 1; k <= nrets; ++k) { - luamp_encode(L, luaL_msgpack_default, out, k); + try { + int nrets = lua_gettop(L); + for (int k = 1; k <= nrets; ++k) { + luamp_encode(L, luaL_msgpack_default, out, k); + } + iproto_reply_select(out, &svp, request->header->sync, nrets); + } catch (...) { + obuf_rollback_to_svp(out, &svp); + throw; } - iproto_reply_select(out, &svp, request->header->sync, nrets); } void -- GitLab