From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id F31D86EC56; Sat, 20 Mar 2021 03:47:56 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org F31D86EC56 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1616201277; bh=t/3MaWogNQe0WSHCk91bL2zLkTHe/yvMmE6dtzLcEto=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=jq4p+b9hFBmm+jUe/0yKlmzAtSQEljwNiDGGZZocFqU/Zdo4bHZ/QvPKaRz72m4wP uCKK5CjC9oqGypqpE7jkj94KQzorubMtROLo1irCKp6RLaxxLwwlkxZM/mQLnmnJvs JPU9zScPkiVMW59+sKkIiS+bLl9SpUW5kotMk8tI= Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 7DA5A6FC8C for ; Sat, 20 Mar 2021 03:43:02 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 7DA5A6FC8C Received: by smtpng3.m.smailru.net with esmtpa (envelope-from ) id 1lNPhl-00076U-NT; Sat, 20 Mar 2021 03:43:02 +0300 To: tarantool-patches@dev.tarantool.org, gorcunov@gmail.com, sergepetrenko@tarantool.org Date: Sat, 20 Mar 2021 01:42:40 +0100 Message-Id: <9dc377c5bb5ecc0b15484541ce8d414e6539fbc4.1616200860.git.v.shpilevoy@tarantool.org> X-Mailer: git-send-email 2.24.3 (Apple Git-128) In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD95D6E7CC48CB1F5F10D3016C09B407F8B1E2E766A3410B623182A05F5380850404981A060FCE0E4B20A9382A74F9906CAF72867F03FD433ACB80AA4DDD8BB1D31 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE798E808258C20928CEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006378D70459436292EC88638F802B75D45FF914D58D5BE9E6BC131B5C99E7648C95C81A23F326053F8274C7C5A6F4312956931609DE0AF7F3B94A471835C12D1D9774AD6D5ED66289B5278DA827A17800CE767883B903EA3BAEA9FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C390BCC82C2C62A6D1117882F4460429728AD0CFFFB425014E868A13BD56FB6657A7F4EDE966BC389F9E8FC8737B5C2249A1DCCEB63E2F10FB089D37D7C0E48F6CCF19DD082D7633A0E7DDDDC251EA7DABAAAE862A0553A39223F8577A6DFFEA7C46CC729E650C927943847C11F186F3C5E7DDDDC251EA7DABCC89B49CDF41148F90DBEB212AEC08F22623479134186CDE6BA297DBC24807EABDAD6C7F3747799A X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8186998911F362727C4B5F30041107ECCAEB459AD25B48836B76E15E3BE9FD72F9AC6CDE5D1141D2B1C6329739450BAD0CD46552BF075DB6569CC911E156B48ECE0AD91A466A1DEF99B296C473AB1E14218DA371E42557AFC06296C473AB1E142186B023E84F73EF47C6DABF04D5057A81F728CF7B057D10C700A9EC8C3488E7643B936CB490224F2464EEA7BD89490CAC0EDDA962BC3F61961 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D343F22CE0A71FDB37EA14B80B0A68E2E20832E40BBA6C6148A48FE1D7D8D2C02E62398F2A6BAE5F9C31D7E09C32AA3244CC3CED0B1978AFC1BC0E60ECD1B8074B9F165894D92D62706FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj8xHC0Ak4ylYU3Dn8D1DLEA== X-Mailru-Sender: 689FA8AB762F73936BC43F508A063822E436295B5FB94E57C208FC3ABCF984A53841015FED1DE5223CC9A89AB576DD93FB559BB5D741EB963CF37A108A312F5C27E8A8C3839CE0E267EA787935ED9F1B X-Mras: Ok Subject: [Tarantool-patches] [PATCH 03/16] tuple: pass global ibuf explicitly where possible X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Vladislav Shpilevoy via Tarantool-patches Reply-To: Vladislav Shpilevoy Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Code in lua/tuple.c used global tarantool_lua_ibuf in many places relying on it never being changed and not reused by other code until a yield. But it is not so. In fact, as it was discovered in #5632, in any Lua function may be started GC. Any GC handler might touch some API also using tarantool_lua_ibuf inside. This makes the first usage in lua/tuple.c invalid - the buffer could be reset or reallocated or its wpos/rpos could change during GC. In order to fix this, first of all there should be clear points where the buffer is taken, and where it becomes not needed anymore. The patch makes code in lua/tuple.c take tarantool_lua_ibuf when it is needed first time. Not during usage. The same is done for the fiber region for the API symmetry. Part of #5632 --- src/box/lua/tuple.c | 60 ++++++++++++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c index 3e6f043b4..ffd01d899 100644 --- a/src/box/lua/tuple.c +++ b/src/box/lua/tuple.c @@ -135,10 +135,8 @@ luaT_istuple(struct lua_State *L, int narg) * Helper for (). */ static int -luaT_tuple_encode_values(struct lua_State *L) +luaT_tuple_encode_values(struct lua_State *L, struct ibuf *buf) { - struct ibuf *buf = tarantool_lua_ibuf; - ibuf_reset(buf); struct mpstream stream; mpstream_init(&stream, buf, ibuf_reserve_cb, ibuf_alloc_cb, luamp_error, L); @@ -151,22 +149,31 @@ luaT_tuple_encode_values(struct lua_State *L) return 0; } -typedef void luaT_mpstream_init_f(struct mpstream *stream, struct lua_State *L); +typedef void luaT_mpstream_init_f(struct mpstream *stream, struct lua_State *L, + void *buffer); static void -luaT_mpstream_init_lua_ibuf(struct mpstream *stream, struct lua_State *L) +luaT_mpstream_init_lua_ibuf(struct mpstream *stream, struct lua_State *L, + void *buffer) { - mpstream_init(stream, tarantool_lua_ibuf, ibuf_reserve_cb, + mpstream_init(stream, (struct ibuf *)buffer, ibuf_reserve_cb, ibuf_alloc_cb, luamp_error, L); } static void -luaT_mpstream_init_box_region(struct mpstream *stream, struct lua_State *L) +luaT_mpstream_init_box_region(struct mpstream *stream, struct lua_State *L, + void *buffer) { - mpstream_init(stream, &fiber()->gc, region_reserve_cb, region_alloc_cb, - luamp_error, L); + mpstream_init(stream, (struct region *)buffer, region_reserve_cb, + region_alloc_cb, luamp_error, L); } +/** Helper to pass parameters into luaT_tuple_encode_table via the Lua stack. */ +struct luaT_tuple_encode_ctx { + luaT_mpstream_init_f *mpstream_init_f; + void *buffer; +}; + /** * Encode a Lua table or a tuple as MsgPack. * @@ -178,8 +185,8 @@ static int luaT_tuple_encode_table(struct lua_State *L) { struct mpstream stream; - luaT_mpstream_init_f *luaT_mpstream_init_f = lua_topointer(L, 1); - luaT_mpstream_init_f(&stream, L); + const struct luaT_tuple_encode_ctx *ctx = lua_topointer(L, 1); + ctx->mpstream_init_f(&stream, L, ctx->buffer); luamp_encode_tuple(L, &tuple_serializer, &stream, 2); mpstream_flush(&stream); return 0; @@ -190,7 +197,8 @@ luaT_tuple_encode_table(struct lua_State *L) */ static int luaT_tuple_encode_on_mpstream(struct lua_State *L, int idx, - luaT_mpstream_init_f *luaT_mpstream_init_f) + luaT_mpstream_init_f *luaT_mpstream_init_f, + void *buffer) { assert(idx != 0); if (!lua_istable(L, idx) && !luaT_istuple(L, idx)) { @@ -213,7 +221,11 @@ luaT_tuple_encode_on_mpstream(struct lua_State *L, int idx, lua_rawgeti(L, LUA_REGISTRYINDEX, luaT_tuple_encode_table_ref); assert(lua_isfunction(L, -1)); - lua_pushlightuserdata(L, luaT_mpstream_init_f); + struct luaT_tuple_encode_ctx ctx = { + .mpstream_init_f = luaT_mpstream_init_f, + .buffer = buffer, + }; + lua_pushlightuserdata(L, &ctx); lua_pushvalue(L, idx); int rc = luaT_call(L, 2, 0); @@ -226,12 +238,11 @@ luaT_tuple_encode_on_mpstream(struct lua_State *L, int idx, */ static char * luaT_tuple_encode_on_lua_ibuf(struct lua_State *L, int idx, - size_t *tuple_len_ptr) + size_t *tuple_len_ptr, struct ibuf *buf) { - struct ibuf *buf = tarantool_lua_ibuf; - ibuf_reset(buf); if (luaT_tuple_encode_on_mpstream(L, idx, - luaT_mpstream_init_lua_ibuf) != 0) + luaT_mpstream_init_lua_ibuf, + buf) != 0) return NULL; if (tuple_len_ptr != NULL) *tuple_len_ptr = ibuf_used(buf); @@ -247,7 +258,8 @@ luaT_tuple_encode(struct lua_State *L, int idx, size_t *tuple_len_ptr) struct region *region = &fiber()->gc; size_t region_svp = region_used(region); if (luaT_tuple_encode_on_mpstream(L, idx, - luaT_mpstream_init_box_region) != 0) { + luaT_mpstream_init_box_region, + region) != 0) { region_truncate(region, region_svp); return NULL; } @@ -268,15 +280,16 @@ luaT_tuple_encode(struct lua_State *L, int idx, size_t *tuple_len_ptr) box_tuple_t * luaT_tuple_new(struct lua_State *L, int idx, box_tuple_format_t *format) { + struct ibuf *ibuf = tarantool_lua_ibuf; + ibuf_reset(ibuf); size_t tuple_len; - char *tuple_data = luaT_tuple_encode_on_lua_ibuf(L, idx, &tuple_len); + char *tuple_data = luaT_tuple_encode_on_lua_ibuf(L, idx, &tuple_len, + ibuf); if (tuple_data == NULL) return NULL; box_tuple_t *tuple = box_tuple_new(format, tuple_data, tuple_data + tuple_len); - if (tuple == NULL) - return NULL; - ibuf_reinit(tarantool_lua_ibuf); + ibuf_reinit(ibuf); return tuple; } @@ -296,7 +309,8 @@ lbox_tuple_new(lua_State *L) box_tuple_format_t *fmt = box_tuple_format_default(); if (argc != 1 || (!lua_istable(L, 1) && !luaT_istuple(L, 1))) { struct ibuf *buf = tarantool_lua_ibuf; - luaT_tuple_encode_values(L); /* may raise */ + ibuf_reset(buf); + luaT_tuple_encode_values(L, buf); /* may raise */ struct tuple *tuple = box_tuple_new(fmt, buf->buf, buf->buf + ibuf_used(buf)); ibuf_reinit(buf); -- 2.24.3 (Apple Git-128)