Investigate if it is possible to make fiber_wakeup() nop on self fiber #5292
Assignees
Labels
bug
Something isn't working
Comments
Gerold103
added a commit
that referenced
this issue
Apr 22, 2021
These new functions are supposed to be used instead of fiber_wakeup whose behaviour sometimes leads to unexpected and hard to catch bugs and to crashes when called on the self fiber, usually accidentally. fiber_touch() allows to wakeup any fiber safely, even self. Because on self it turns into NOP, but it is not any slower than fiber_wakeup(), because uses the same code, but with a bit different flag set. fiber_continue() is like fiber_wakeup() but it has an assertion that it is not called on self allowing to catch self-wakeup bugs early and easy. fiber_wakeup() still exists because it is a part of the public API. In order to make fiber_touch() as cheap as fiber_wakeup() there is a new flag FIBER_IS_RUNNING. It is set for the fiber stored in the cord - the currently running one. The only difference between the wakeup and touch is wakeup skips READY | DEAD fibers, while the touch skips READY | DEAD | RUNNING. Both cost one bitwise OR operation. Part of #5292
Gerold103
added a commit
that referenced
this issue
Apr 22, 2021
fiber_wakeup() is not safe when called on the self fiber - it leads to a skip of the next yield, which leads to hard to catch bugs and might appear much later than the wakeup was called making it hard to trace the source of the bug. The previous commit introduces safer versions of wakeup: touch and continue. This patch uses them everywhere except the tests. This allows to drop 'f != fiber()' checks in some places which could work in the current fiber, and ensures in all the other places the fiber is never the current one. Closes #5292
Gerold103
added a commit
that referenced
this issue
Apr 23, 2021
fiber.wakeup() in Lua and fiber_wakeup() in C could lead to a crash or undefined behaviour when called on the currently running fiber. In particular, if after wakeup was started a new fiber in a blocking way (fiber.create() and fiber_start()) it would crash in debug build, and lead to unknown results in release. If after wakeup was made a sleep with non-zero timeout or an infinite yield (fiber_yield()), the fiber woke up in the same event loop iteration regardless of any timeout or other wakeups. It was a spurious wakeup, which is not expected in most of the places internally. The patch makes the wakeup nop on the current fiber making it safe to use anywhere. Closes #5292 Closes #6043 @TarantoolBot document Title: fiber.wakeup() in Lua and fiber_wakeup() in C are nop on self In Lua `fiber.wakeup()` being called on the current fiber does not do anything, safe to use. The same for `fiber_wakeup()` in C.
Gerold103
added a commit
that referenced
this issue
Apr 23, 2021
The previous commit made fiber_wakeup() safe to use on the current fiber. Leverage the new behaviour everywhere in the source code to remove all checks f != fiber() before fiber_wakeup(f) calls. Follow-up #5292
kyukhin
pushed a commit
that referenced
this issue
Apr 27, 2021
fiber.wakeup() in Lua and fiber_wakeup() in C could lead to a crash or undefined behaviour when called on the currently running fiber. In particular, if after wakeup was started a new fiber in a blocking way (fiber.create() and fiber_start()) it would crash in debug build, and lead to unknown results in release. If after wakeup was made a sleep with non-zero timeout or an infinite yield (fiber_yield()), the fiber woke up in the same event loop iteration regardless of any timeout or other wakeups. It was a spurious wakeup, which is not expected in most of the places internally. The patch makes the wakeup nop on the current fiber making it safe to use anywhere. Closes #5292 Closes #6043 @TarantoolBot document Title: fiber.wakeup() in Lua and fiber_wakeup() in C are nop on self In Lua `fiber.wakeup()` being called on the current fiber does not do anything, safe to use. The same for `fiber_wakeup()` in C. (cherry picked from commit db0ded5)
kyukhin
pushed a commit
that referenced
this issue
Apr 27, 2021
The previous commit made fiber_wakeup() safe to use on the current fiber. Leverage the new behaviour everywhere in the source code to remove all checks f != fiber() before fiber_wakeup(f) calls. Follow-up #5292
kyukhin
pushed a commit
that referenced
this issue
Apr 27, 2021
fiber.wakeup() in Lua and fiber_wakeup() in C could lead to a crash or undefined behaviour when called on the currently running fiber. In particular, if after wakeup was started a new fiber in a blocking way (fiber.create() and fiber_start()) it would crash in debug build, and lead to unknown results in release. If after wakeup was made a sleep with non-zero timeout or an infinite yield (fiber_yield()), the fiber woke up in the same event loop iteration regardless of any timeout or other wakeups. It was a spurious wakeup, which is not expected in most of the places internally. The patch makes the wakeup nop on the current fiber making it safe to use anywhere. Closes #5292 Closes #6043 @TarantoolBot document Title: fiber.wakeup() in Lua and fiber_wakeup() in C are nop on self In Lua `fiber.wakeup()` being called on the current fiber does not do anything, safe to use. The same for `fiber_wakeup()` in C. (cherry picked from commit db0ded5)
kyukhin
pushed a commit
that referenced
this issue
Apr 27, 2021
fiber.wakeup() in Lua and fiber_wakeup() in C could lead to a crash or undefined behaviour when called on the currently running fiber. In particular, if after wakeup was started a new fiber in a blocking way (fiber.create() and fiber_start()) it would crash in debug build, and lead to unknown results in release. If after wakeup was made a sleep with non-zero timeout or an infinite yield (fiber_yield()), the fiber woke up in the same event loop iteration regardless of any timeout or other wakeups. It was a spurious wakeup, which is not expected in most of the places internally. The patch makes the wakeup nop on the current fiber making it safe to use anywhere. Closes #5292 Closes #6043 @TarantoolBot document Title: fiber.wakeup() in Lua and fiber_wakeup() in C are nop on self In Lua `fiber.wakeup()` being called on the current fiber does not do anything, safe to use. The same for `fiber_wakeup()` in C. (cherry picked from commit db0ded5)
avtikhon
pushed a commit
to avtikhon/tarantool
that referenced
this issue
Jun 1, 2021
fiber.wakeup() in Lua and fiber_wakeup() in C could lead to a crash or undefined behaviour when called on the currently running fiber. In particular, if after wakeup was started a new fiber in a blocking way (fiber.create() and fiber_start()) it would crash in debug build, and lead to unknown results in release. If after wakeup was made a sleep with non-zero timeout or an infinite yield (fiber_yield()), the fiber woke up in the same event loop iteration regardless of any timeout or other wakeups. It was a spurious wakeup, which is not expected in most of the places internally. The patch makes the wakeup nop on the current fiber making it safe to use anywhere. Closes tarantool#5292 Closes tarantool#6043 @TarantoolBot document Title: fiber.wakeup() in Lua and fiber_wakeup() in C are nop on self In Lua `fiber.wakeup()` being called on the current fiber does not do anything, safe to use. The same for `fiber_wakeup()` in C.
avtikhon
pushed a commit
to avtikhon/tarantool
that referenced
this issue
Jun 1, 2021
The previous commit made fiber_wakeup() safe to use on the current fiber. Leverage the new behaviour everywhere in the source code to remove all checks f != fiber() before fiber_wakeup(f) calls. Follow-up tarantool#5292
avtikhon
pushed a commit
to avtikhon/tarantool
that referenced
this issue
Jun 1, 2021
fiber.wakeup() in Lua and fiber_wakeup() in C could lead to a crash or undefined behaviour when called on the currently running fiber. In particular, if after wakeup was started a new fiber in a blocking way (fiber.create() and fiber_start()) it would crash in debug build, and lead to unknown results in release. If after wakeup was made a sleep with non-zero timeout or an infinite yield (fiber_yield()), the fiber woke up in the same event loop iteration regardless of any timeout or other wakeups. It was a spurious wakeup, which is not expected in most of the places internally. The patch makes the wakeup nop on the current fiber making it safe to use anywhere. Closes tarantool#5292 Closes tarantool#6043 @TarantoolBot document Title: fiber.wakeup() in Lua and fiber_wakeup() in C are nop on self In Lua `fiber.wakeup()` being called on the current fiber does not do anything, safe to use. The same for `fiber_wakeup()` in C.
avtikhon
pushed a commit
to avtikhon/tarantool
that referenced
this issue
Jun 1, 2021
The previous commit made fiber_wakeup() safe to use on the current fiber. Leverage the new behaviour everywhere in the source code to remove all checks f != fiber() before fiber_wakeup(f) calls. Follow-up tarantool#5292
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Fiber_wakeup() being called on self (currently active fiber) leads to undefined behaviour. Most of the time it ends the process life some time after in an assertion when it tries to start another fiber for example. But in prod the assertion won't fire, and anything can happen.
I would end this UB right away in a simple patch, but it appears fiber_wakeup() in fiber.c file has a comment saying that it is used together with fiber_yield() right after for some special rescheduling case.
Need to see if it is true, if it is used anywhere, and find a way to make this function less error-prone.
The text was updated successfully, but these errors were encountered: