From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 3007E46970E for ; Wed, 5 Feb 2020 19:53:17 +0300 (MSK) Date: Wed, 5 Feb 2020 19:53:09 +0300 From: Alexander Turenko Message-ID: <20200205165309.jnvb6p5sbv2zns54@tkn_work_nb> References: <20200116211714.lhwqjfm3o4nhctf7@tkn_work_nb> <9b6e89a0-33f3-6844-4a13-b30303c23f36@tarantool.org> <20200121023332.2gyph5jbr2exm2fs@tkn_work_nb> <20200124143515.GH26983@tarantool.org> <851af63f-4ced-c1de-cac8-eebcd0b6dcd3@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <851af63f-4ced-c1de-cac8-eebcd0b6dcd3@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH] Move rollback to savepoint from ffi to C-API List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Leonid Vasiliev Cc: tarantool-patches@dev.tarantool.org We agreed on the following wording with Igor and Leonid: commit 34234427a3cc1f181ef6583c36ed7e4193bafde7 Author: Leonid Vasiliev Date: Fri Dec 13 10:49:33 2019 +0300 box: rewrite rollback to savepoint to Lua/C LuaJIT records traces while interpreting Lua bytecode (considering it's hot enough) in order to compile the corresponding execution flow to a machine code. A Lua/C call aborts trace recording, but an FFI call does not abort it per se. If code inside an FFI call yields to another fiber while recording a trace and the new current fiber interpreting a Lua bytecode too, then unrelated instructions will be recorded to the current trace. In short, we should not yield a current fiber inside an FFI call. There is another problem. Machine code of a compiled trace may sink a value from a Lua state down to a host register, change it and write back only at trace exit. So the interpreter state may be outdated during the compiled trace execution. A Lua/C call aborts a trace and so the code inside a callee always see an actual interpreter state. An FFI call however can be turned into a single machine's CALL instruction in the compiled code and if the callee accesses a Lua state, then it may see an irrelevant value. In short, we should not access a Lua state directly or reenter to the interpreter from an FFI call. The box.rollback_to_savepoint() function may yield and another fiber will be scheduled for execution. If this fiber touches a Lua state, then it may see an inconsistent state and the behaviour will be undefined. Noted that .id starts from 1, because we lean on this fact to use luaL_toint64(), which does not distinguish an unexpected Lua type and cdata with zero value. It seems that this assumption already exists: the code that prepare arguments for 'on_commit' triggers uses luaL_toint64() too (see lbox_txn_pairs()). Fixes #4427 Co-authored-by: Alexander Turenko Reviewed-by: Igor Munkin Pushed to master, 2.3, 2.2 and 1.10. CCed Kirill. WBR, Alexander Turenko. On Mon, Jan 27, 2020 at 10:24:18AM +0300, Leonid Vasiliev wrote: > The result patch: > > https://github.com/tarantool/tarantool/issues/4427 > https://github.com/tarantool/tarantool/tree/lvasiliev/gh-4427-move-rollback-from-ffi-to-c-api-v2 > > box: rewrite rollback to savepoint to Lua/C > > The fix is prevent an emerge of "FFI sandwich". > See > https://github.com/tarantool/tarantool/issues/4427#issuecomment-524798109 > for more information. > > One of the aims of this change (in addition to the fix) is to increase > locality of highly > coupled code for the rollback to savepoint. > > Noted that .id starts from 1, because I lean on this fact to > use luaL_toint64(), which does not distinguish an unexpected Lua type > case and cdata with zero value. It seems that this assumption > already exists: the code that prepare arguments for 'on_commit' triggers > uses luaL_toint64() too (see lbox_txn_pairs()). > > Fixes #4427 > > Co-authored-by: Alexander Turenko > Co-authored-by: Leonid Vasiliev