Re: [Tarantool-patches] [PATCH 00/16] Cord buffer, static alloc, and Lua GC bug

[Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

20.03.2021 03:42, Vladislav Shpilevoy пишет:
> The patch attempts to fix most of the easy to face issues with the global
> resources not having proper ownership in Lua code and therefore not protected
> again being suddenly reused during Lua GC.
>
> There are many such resources: tarantool_lua_ibuf/IBUF_SHARED, static alloc,
> errno, diag/box.error, box_tuple_last, port_c, and others.
>
> The most easy to screw - ibuf and static alloc. They are fixed in this
> patchset.
>
> The solution is dubious, but there were not found any better alternatives.
>
> A patch for 1.10 will be sent later in a separate CL.
>
> Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-5632-global-buf-crash-2x
> Issue: https://github.com/tarantool/tarantool/issues/5632

Hi! Thanks for the patchset!
Generally LGTM, with one question in reply to patch 06.

>
> Vladislav Shpilevoy (16):
>    fio: don't use shared buffer in pread()
>    test: don't use IBUF_SHARED in the tests
>    tuple: pass global ibuf explicitly where possible
>    iconv: take errno before reseting the context
>    cord_buf: introduce cord_buf API
>    cord_buf: introduce ownership management
>    buffer: implement ffi stash
>    uuid: replace static_alloc with ffi stash
>    uuid: drop tt_uuid_str() from Lua
>    uri: replace static_alloc with ffi stash and ibuf
>    buffer: remove static_alloc() from Lua
>    lua: use lua_pushfstring() instead of tt_sprintf()
>    sio: rework sio_strfaddr()
>    sio: increase SERVICE_NAME_MAXLEN size
>    sio: introduce and use sio_snprintf()
>    buffer: remove Lua registers
>
>   changelogs/unreleased/fix-ibuf-static.md      |   7 +
>   src/CMakeLists.txt                            |   1 -
>   src/box/iproto.cc                             |   8 +-
>   src/box/iproto.h                              |   2 +-
>   src/box/lua/info.c                            |   5 +-
>   src/box/lua/net_box.lua                       |   4 +-
>   src/box/lua/schema.lua                        |  56 ++++--
>   src/box/lua/session.c                         |   7 +-
>   src/box/lua/space.cc                          |   6 +-
>   src/box/lua/tuple.c                           |  80 +++++----
>   src/box/lua/tuple.lua                         |  14 +-
>   src/exports.h                                 |   7 +-
>   src/lib/core/CMakeLists.txt                   |   1 +
>   src/lib/core/cord_buf.c                       | 167 ++++++++++++++++++
>   src/lib/core/cord_buf.h                       |  54 ++++++
>   src/lib/core/sio.c                            |  72 ++++----
>   src/lib/core/sio.h                            |  15 +-
>   src/lua/buffer.c                              |  42 -----
>   src/lua/buffer.lua                            | 156 +++++++++-------
>   src/lua/crypto.lua                            |  19 +-
>   src/lua/digest.lua                            |  25 ++-
>   src/lua/fiber.c                               |   2 +-
>   src/lua/fio.lua                               |  12 +-
>   src/lua/iconv.lua                             |  27 +--
>   src/lua/init.c                                |   3 -
>   src/lua/msgpack.c                             |   6 +-
>   src/lua/msgpackffi.lua                        |  57 +++---
>   src/lua/socket.lua                            |  54 +++---
>   src/lua/string.lua                            |  26 ++-
>   src/lua/swim.lua                              |  19 +-
>   src/lua/uri.lua                               |  21 ++-
>   src/lua/utf8.c                                |  21 ++-
>   src/lua/utils.h                               |   1 -
>   src/lua/uuid.lua                              | 115 +++++++-----
>   .../test/static-build/exports.test.lua        |   4 +-
>   test/app-tap/buffer.test.lua                  |  59 +++++++
>   test/app-tap/gh-5632-gc-buf-reuse.test.lua    | 151 ++++++++++++++++
>   test/app-tap/module_api.test.lua              |  10 +-
>   test/app-tap/uri.test.lua                     |  23 +--
>   test/app/buffer.result                        |  53 ------
>   test/app/buffer.test.lua                      |  23 ---
>   test/app/fio.result                           | 160 +++++++++--------
>   test/app/fio.test.lua                         |  67 ++++---
>   test/app/msgpack.result                       |   7 +-
>   test/app/msgpack.test.lua                     |   5 +-
>   test/box/varbinary_type.result                |   3 +-
>   test/box/varbinary_type.test.lua              |   3 +-
>   test/unit/luaT_tuple_new.c                    |   4 -
>   48 files changed, 1092 insertions(+), 592 deletions(-)
>   create mode 100644 changelogs/unreleased/fix-ibuf-static.md
>   create mode 100644 src/lib/core/cord_buf.c
>   create mode 100644 src/lib/core/cord_buf.h
>   delete mode 100644 src/lua/buffer.c
>   create mode 100755 test/app-tap/buffer.test.lua
>   create mode 100755 test/app-tap/gh-5632-gc-buf-reuse.test.lua
>   delete mode 100644 test/app/buffer.result
>   delete mode 100644 test/app/buffer.test.lua
>

-- 
Serge Petrenko