BUG/MINOR: hlua: hook yield does not behave as expected

In function hlua_hook, a yieldk is performed when function is yieldable.

But the following code in that function seems to assume that the yield
never returns, which is not the case!

Moreover, Lua documentation says that in this situation the yieldk call
must immediately be followed by a return.

This patch adds a return statement after the yieldk call.
It also adds some comments and removes a needless lua_sethook call.

It could be backported to all stable versions, but it is not mandatory,
because even if it is undefined behavior this bug doesn't seem to
negatively affect lua 5.3/5.4 stacks.

diff --git a/src/hlua.c b/src/hlua.c
index f2e6f24a23119e31737e745c3d0906fd31300a81..263d254ed08065f83d3e27fb0ef868f85ef3c3f5 100644 (file)
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -1408,14 +1408,30 @@ void hlua_hook(lua_State *L, lua_Debug *ar)
                return;
        }
 
-       /* restore the interrupt condition. */
-       lua_sethook(hlua->T, hlua_hook, LUA_MASKCOUNT, hlua_nb_instruction);
-
        /* If we interrupt the Lua processing in yieldable state, we yield.
         * If the state is not yieldable, trying yield causes an error.
         */
-       if (lua_isyieldable(L))
+       if (lua_isyieldable(L)) {
+               /* note: for converters/fetches.. where yielding is not allowed
+                * hlua_ctx_resume() will simply perform a goto resume_execution
+                * instead of rescheduling hlua->task.
+                * also: hlua_ctx_resume() will take care of checking execution
+                * timeout and re-applying the hook as needed.
+                */
                MAY_LJMP(hlua_yieldk(L, 0, 0, NULL, TICK_ETERNITY, HLUA_CTRLYIELD));
+               /* lua docs says that the hook should return immediately after lua_yieldk
+                *
+                * From: https://www.lua.org/manual/5.3/manual.html#lua_yieldk
+                *
+                * Moreover, it seems that we don't want to continue after the yield
+                * because the end of the function is about handling unyieldable function,
+                * which is not the case here.
+                *
+                *  ->if we don't return lua_sethook gets incorrectly set with MASKRET later
+                *  in the function.
+                */
+               return;
+       }
 
        /* If we cannot yield, update the clock and check the timeout. */
        clock_update_date(0, 1);