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 6B49E30E5C for ; Fri, 7 Dec 2018 15:42:37 -0500 (EST) 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 g_v1tXWT3HYn for ; Fri, 7 Dec 2018 15:42:37 -0500 (EST) Received: from smtp53.i.mail.ru (smtp53.i.mail.ru [94.100.177.113]) (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 260762C11E for ; Fri, 7 Dec 2018 15:42:37 -0500 (EST) Subject: [tarantool-patches] Re: [PATCH] Fix premature cdata collecting in luaT_pusherror() References: <20181207173204.GA840@chai> From: Vladislav Shpilevoy Message-ID: Date: Fri, 7 Dec 2018 23:42:31 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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: Alexander Turenko , Konstantin Osipov Cc: tarantool-patches@freelists.org On 07/12/2018 21:15, Alexander Turenko wrote: > This is follow up of 28c7e667aee9be8c3288597bcc179d9b4e7b4940 to fix > luaT_pusherror() itself, not only luaT_error(). > > Fixes #1955 (again). > > It is backport of 480868ff8f304ead2fd03e317902a1e8d41c6248 for 1.10. > --- > > https://github.com/tarantool/tarantool/issues/1955 > https://github.com/tarantool/tarantool/tree/Totktonada/gh-1955-fix-luaT_pusherror-1.10 > > src/lua/utils.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/src/lua/utils.c b/src/lua/utils.c > index 27772a9d3..b05d16a73 100644 > --- a/src/lua/utils.c > +++ b/src/lua/utils.c > @@ -898,12 +898,20 @@ luaL_error_gc(struct lua_State *L) > void > luaT_pusherror(struct lua_State *L, struct error *e) > { > + /* > + * gh-1955 luaT_pusherror allocates Lua objects, thus it Sorry, but usually we do not mention issue numbers in comments. Please, reword the comment so as to do not use explicit issue number. > + * may trigger GC. GC may invoke finalizers which are > + * arbitrary Lua code, potentially invalidating last error > + * object, hence error_ref below. > + * > + * It also important to reference the error first and only > + * then set the finalizer. > + */ > + error_ref(e); > assert(CTID_CONST_STRUCT_ERROR_REF != 0); > struct error **ptr = (struct error **) luaL_pushcdata(L, > CTID_CONST_STRUCT_ERROR_REF); > *ptr = e; > - /* The order is important - first reference the error, then set gc */ > - error_ref(e); > lua_pushcfunction(L, luaL_error_gc); > luaL_setcdatagc(L, -2); > } > @@ -913,15 +921,7 @@ luaT_error(lua_State *L) > { > struct error *e = diag_last_error(&fiber()->diag); > assert(e != NULL); > - /* > - * gh-1955 luaT_pusherror allocates Lua objects, thus it may trigger > - * GC. GC may invoke finalizers which are arbitrary Lua code, > - * potentially invalidating last error object, hence error_ref > - * below. > - */ > - error_ref(e); > luaT_pusherror(L, e); > - error_unref(e); > lua_error(L); > unreachable(); > return 0; >