Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Yield from a GC hook from a fiber serving an iproto request leads to assertion fail #6647

Closed
Totktonada opened this issue Nov 25, 2021 · 0 comments · Fixed by #8048
Closed
Assignees
Labels
1.10 Target is 1.10 and all newer release/master branches bug Something isn't working crash

Comments

@Totktonada
Copy link
Member

Totktonada commented Nov 25, 2021

1.10.11-46-g5c8c025da

-- box.cfg, grants, requires, create net.box connection.
tarantool> box.cfg({listen = 3301})
tarantool> box.schema.user.grant('guest', 'super')
tarantool> ffi = require('ffi')
tarantool> fiber = require('fiber')
tarantool> net_box = require('net.box')
tarantool> c = net_box.connect(3301)

-- Reproducer itself.
tarantool> f = function() local t = {__gh_hook = ffi.gc(ffi.new('void *'), fiber.yield)} t = nil collectgarbage() end
tarantool> c:call('f')
tarantool: /home/alex/p/tarantool-meta/1.10/src/lua/utils.c:730: cord_on_yield: Assertion `L != NULL' failed.
Aborted

AFAIR, we can see an empty lua state in the fibr local storage on master too in some cases. It should be revisited carefully as for master as well as for 1.10.

Info for inspiration:

9be50ae
b4e186f
ec9b544
tarantool/tuple-merger#18

@Totktonada Totktonada added bug Something isn't working teamL crash labels Nov 25, 2021
@kyukhin kyukhin added this to the 1.10.14 milestone Jun 16, 2022
@Totktonada Totktonada removed this from the 1.10.14 milestone Sep 7, 2022
@igormunkin igormunkin removed the teamL label Sep 15, 2022
@igormunkin igormunkin added the 1.10 Target is 1.10 and all newer release/master branches label Oct 2, 2022
@mkokryashkin mkokryashkin self-assigned this Oct 5, 2022
mkokryashkin pushed a commit to mkokryashkin/tarantool that referenced this issue Nov 26, 2022
There is code that may save some time and resources for creating a new
Lua state when it is present in the fiber storage of a current fiber.
There are not so much of them: running a Lua trigger and construction of
a next tuple in a merge source.

Before the patch, fiber->storage.lua.stack is filled only for the main
fiber and fibers created from Lua using fiber.create() or fiber.new(),
but not for background fibers (which serve binary protocol requests).

This patch fills fiber->storage.lua.stack for background fibers that
serve a Lua call or eval: we already have this state and nothing
prevents us from exposing it via the fiber storage.

Closes tarantool#6647
Follows up tarantool#4954

NO_DOC=bugfix

(cherry-picked from commit ec9b544)
mkokryashkin added a commit to mkokryashkin/tarantool that referenced this issue Dec 12, 2022
Before the patch, fiber->storage.lua.stack is used for
`panic` calls. However, some fibers don't have any Lua
state saved in their storage (for example, space
triggers).

After the patch, the Lua state pointed by `cur_L` is
used to make those calls, as it is always present.

Closes tarantool#6647
NO_DOC=bugfix
mkokryashkin added a commit to mkokryashkin/tarantool that referenced this issue Dec 12, 2022
Before the patch, fiber->storage.lua.stack is used for
`panic` calls. However, some fibers don't have any Lua
state saved in their storage (for example, space
triggers).

After the patch, the Lua state pointed by `cur_L` is
used to make those calls, as it is always present.

Closes tarantool#6647
NO_DOC=bugfix
mkokryashkin pushed a commit to mkokryashkin/tarantool that referenced this issue Dec 13, 2022
There is code that may save some time and resources for creating a new
Lua state when it is present in the fiber storage of a current fiber.
There are not so much of them: running a Lua trigger and construction of
a next tuple in a merge source.

Before the patch, fiber->storage.lua.stack is filled only for the main
fiber and fibers created from Lua using fiber.create() or fiber.new(),
but not for background fibers (which serve binary protocol requests).

This patch fills fiber->storage.lua.stack for background fibers that
serve a Lua call or eval: we already have this state and nothing
prevents us from exposing it via the fiber storage.

Part of tarantool#6647
Follows up tarantool#4954

NO_DOC=bugfix

(cherry-picked from commit ec9b544)
mkokryashkin added a commit to mkokryashkin/tarantool that referenced this issue Dec 13, 2022
Before the patch, fiber->storage.lua.stack is used for
`panic` calls. However, some fibers don't have any Lua
state saved in their storage (for example, space
triggers).

After the patch, the Lua state pointed by `cur_L` is
used to make those calls, as it is always present.

Closes tarantool#6647
NO_DOC=bugfix
NO_CHANGELOG=already added in the previous patch
mkokryashkin pushed a commit to mkokryashkin/tarantool that referenced this issue Dec 13, 2022
There is code that may save some time and resources for creating a new
Lua state when it is present in the fiber storage of a current fiber.
There are not so much of them: running a Lua trigger and construction of
a next tuple in a merge source.

Before the patch, fiber->storage.lua.stack is filled only for the main
fiber and fibers created from Lua using fiber.create() or fiber.new(),
but not for background fibers (which serve binary protocol requests).

This patch fills fiber->storage.lua.stack for background fibers that
serve a Lua call or eval: we already have this state and nothing
prevents us from exposing it via the fiber storage.

Part of tarantool#6647
Follows up tarantool#4954

NO_DOC=bugfix

(cherry-picked from commit ec9b544)
mkokryashkin added a commit to mkokryashkin/tarantool that referenced this issue Dec 13, 2022
Before the patch, fiber->storage.lua.stack is used for
`panic` calls. However, some fibers don't have any Lua
state saved in their storage (for example, space
triggers).

After the patch, the Lua state pointed by `cur_L` is
used to make those calls, as it is always present.

Closes tarantool#6647
NO_DOC=bugfix
NO_CHANGELOG=already added in the previous patch
mkokryashkin pushed a commit to mkokryashkin/tarantool that referenced this issue Dec 13, 2022
There is code that may save some time and resources for creating a new
Lua state when it is present in the fiber storage of a current fiber.
There are not so much of them: running a Lua trigger and construction of
a next tuple in a merge source.

Before the patch, fiber->storage.lua.stack is filled only for the main
fiber and fibers created from Lua using fiber.create() or fiber.new(),
but not for background fibers (which serve binary protocol requests).

This patch fills fiber->storage.lua.stack for background fibers that
serve a Lua call or eval: we already have this state and nothing
prevents us from exposing it via the fiber storage.

Part of tarantool#6647
Follows up tarantool#4954

NO_DOC=bugfix

(cherry-picked from commit ec9b544)
mkokryashkin added a commit to mkokryashkin/tarantool that referenced this issue Dec 13, 2022
Before the patch, fiber->storage.lua.stack is used for
`panic` calls. However, some fibers don't have any Lua
state saved in their storage (for example, space
triggers).

After the patch, the Lua state pointed by `cur_L` is
used to make those calls, as it is always present.

Closes tarantool#6647
NO_DOC=bugfix
NO_CHANGELOG=already added in the previous patch
mkokryashkin pushed a commit to mkokryashkin/tarantool that referenced this issue Dec 13, 2022
There is code that may save some time and resources for creating a new
Lua state when it is present in the fiber storage of a current fiber.
There are not so much of them: running a Lua trigger and construction of
a next tuple in a merge source.

Before the patch, fiber->storage.lua.stack is filled only for the main
fiber and fibers created from Lua using fiber.create() or fiber.new(),
but not for background fibers (which serve binary protocol requests).

This patch fills fiber->storage.lua.stack for background fibers that
serve a Lua call or eval: we already have this state and nothing
prevents us from exposing it via the fiber storage.

Part of tarantool#6647
Follows up tarantool#4954

NO_DOC=bugfix

(cherry-picked from commit ec9b544)
mkokryashkin added a commit to mkokryashkin/tarantool that referenced this issue Dec 13, 2022
Before the patch, fiber->storage.lua.stack is used for
`panic` calls. However, some fibers don't have any Lua
state saved in their storage (for example, space
triggers).

After the patch, the Lua state pointed by `cur_L` is
used to make those calls, as it is always present.

Closes tarantool#6647
NO_DOC=bugfix
NO_CHANGELOG=already added in the previous patch
mkokryashkin added a commit to mkokryashkin/tarantool that referenced this issue Dec 13, 2022
Before the patch, fiber->storage.lua.stack is used for
`panic` calls. However, some fibers don't have any Lua
state saved in their storage (for example, space
triggers).

After the patch, the Lua state pointed by `cur_L` is
used to make those calls, as it is always present.

Closes tarantool#6647
NO_DOC=bugfix
mkokryashkin added a commit to mkokryashkin/tarantool that referenced this issue Dec 15, 2022
Before the patch, fiber->storage.lua.stack is used for
`panic` calls. However, some fibers don't have any Lua
state saved in their storage (for example, space
triggers).

After the patch, the Lua state pointed by `cur_L` is
used to make those calls, as it is always present.

Closes tarantool#6647
NO_DOC=bugfix
mkokryashkin added a commit to mkokryashkin/tarantool that referenced this issue Dec 15, 2022
Before the patch, fiber->storage.lua.stack is used for
`panic` calls. However, some fibers don't have any Lua
state saved in their storage (for example, space
triggers).

After the patch, the Lua state pointed by `cur_L` is
used to make those calls, as it is always present.

Closes tarantool#6647
NO_DOC=bugfix
mkokryashkin added a commit to mkokryashkin/tarantool that referenced this issue Dec 15, 2022
Before the patch, fiber->storage.lua.stack is used for
`panic` calls. However, some fibers don't have any Lua
state saved in their storage (for example, space
triggers).

After the patch, the Lua state pointed by `cur_L` is
used to make those calls, as it is always present.

Closes tarantool#6647
NO_DOC=bugfix
mkokryashkin added a commit to mkokryashkin/tarantool that referenced this issue Dec 15, 2022
Before the patch, fiber->storage.lua.stack is used for
`panic` calls. However, some fibers don't have any Lua
state saved in their storage (for example, space
triggers).

After the patch, the Lua state pointed by `cur_L` is
used to make those calls, as it is always present.

Closes tarantool#6647
NO_DOC=bugfix
mkokryashkin added a commit to mkokryashkin/tarantool that referenced this issue Dec 15, 2022
Before the patch, fiber->storage.lua.stack is used for
`panic` calls. However, some fibers don't have any Lua
state saved in their storage (for example, space
triggers).

After the patch, the Lua state pointed by `cur_L` is
used to make those calls, as it is always present.

Closes tarantool#6647
NO_DOC=bugfix
mkokryashkin added a commit to mkokryashkin/tarantool that referenced this issue Dec 15, 2022
Before the patch, fiber->storage.lua.stack is used for
`panic` calls. However, some fibers don't have any Lua
state saved in their storage (for example, space
triggers).

After the patch, the Lua state pointed by `cur_L` is
used to make those calls, as it is always present.

Closes tarantool#6647
NO_DOC=bugfix
mkokryashkin added a commit to mkokryashkin/tarantool that referenced this issue Dec 15, 2022
Before the patch, fiber->storage.lua.stack is used for
`panic` calls. However, some fibers don't have any Lua
state saved in their storage (for example, space
triggers).

After the patch, the Lua state pointed by `cur_L` is
used to make those calls, as it is always present.

Closes tarantool#6647
NO_DOC=bugfix
mkokryashkin added a commit to mkokryashkin/tarantool that referenced this issue Dec 15, 2022
Before the patch, fiber->storage.lua.stack is used for
`panic` calls. However, some fibers don't have any Lua
state saved in their storage (for example, space
triggers).

After the patch, the Lua state pointed by `cur_L` is
used to make those calls, as it is always present.

Closes tarantool#6647
NO_DOC=bugfix
mkokryashkin added a commit to mkokryashkin/tarantool that referenced this issue Dec 16, 2022
Before the patch, fiber->storage.lua.stack is used for
`panic` calls. However, some fibers don't have any Lua
state saved in their storage (for example, space
triggers).

After the patch, the Lua state pointed by `cur_L` is
used to make those calls, as it is always present.

Closes tarantool#6647
NO_DOC=bugfix
igormunkin pushed a commit that referenced this issue Dec 16, 2022
Before the patch, fiber->storage.lua.stack is used for
`panic` calls. However, some fibers don't have any Lua
state saved in their storage (for example, space
triggers).

After the patch, the Lua state pointed by `cur_L` is
used to make those calls, as it is always present.

Closes #6647
NO_DOC=bugfix

(cherry picked from commit dfe79b9)
igormunkin pushed a commit that referenced this issue Dec 16, 2022
Before the patch, fiber->storage.lua.stack is used for
`panic` calls. However, some fibers don't have any Lua
state saved in their storage (for example, space
triggers).

After the patch, the Lua state pointed by `cur_L` is
used to make those calls, as it is always present.

Closes #6647
NO_DOC=bugfix

(cherry picked from commit dfe79b9)
@igormunkin igormunkin linked a pull request Dec 21, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.10 Target is 1.10 and all newer release/master branches bug Something isn't working crash
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants