[Tarantool-patches] [PATCH] Move rollback to savepoint from ffi to C-API
Alexander Turenko
alexander.turenko at tarantool.org
Wed Feb 5 19:53:09 MSK 2020
We agreed on the following wording with Igor and Leonid:
commit 34234427a3cc1f181ef6583c36ed7e4193bafde7
Author: Leonid Vasiliev <lvasiliev at tarantool.org>
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 <struct txn>.id starts from 1, because we lean on this fact
to use luaL_toint64(), which does not distinguish an unexpected Lua type
and cdata<int64_t> 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 <alexander.turenko at tarantool.org>
Reviewed-by: Igor Munkin <imun at tarantool.org>
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 <struct txn>.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<int64_t> 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<alexander.turenko at tarantool.org>
> Co-authored-by: Leonid Vasiliev<lvasiliev at tarantool.org>
More information about the Tarantool-patches
mailing list