BUG/MINOR: hlua: SET_SAFE_LJMP misuse in hlua_event_runner()

When hlua_event_runner() pauses the subscription (ie: if the consumer
can't keep up the pace), hlua_traceback() is used to get the current
lua trace (running context) to provide some info to the user.

However, as hlua_traceback() may raise an error (__LJMP) is set, it is
used within a SET_SAFE_LJMP() / RESET_SAFE_LJMP() combination to ensure
lua errors are properly handled and don't result in unexpected behavior.

But the current usage of SET_SAFE_LJMP() within the function is wrong
since hlua_traceback() will run a second time (unprotected) if the
first (protected) attempt fails. This is undefined behavior and could
even lead to crashes.

Hopefully it is very hard to trigger this code path, thus we can consider
this as a minor bug.

Also using this as an opportunity to enhance the message report to make
it more meaningful to the user.

This should fix GH #2159.

It is a 2.8 specific bug, no backport needed unless c84899c636
("MEDIUM: hlua/event_hdl: initial support for event handlers") gets
backported.

diff --git a/src/hlua.c b/src/hlua.c
index 98ee44170a0aaea33d1a0697b1e173178862be96..4ad23f2e20647cd0ad4c962842378da411df626b 100644 (file)
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -9329,7 +9329,7 @@ static struct task *hlua_event_runner(struct task *task, void *context, unsigned
        const char *error = NULL;
 
        if (!hlua_sub->paused && event_hdl_async_equeue_size(&hlua_sub->equeue) > 100) {
-               const char *trace;
+               const char *trace = NULL;
 
                /* We reached the limit of pending events in the queue: we should
                 * warn the user, and temporarily pause the subscription to give a chance
@@ -9345,14 +9345,19 @@ static struct task *hlua_event_runner(struct task *task, void *context, unsigned
                event_hdl_pause(hlua_sub->sub);
                hlua_sub->paused = 1;
 
-               /* The following Lua calls can fail. */
-               if (!SET_SAFE_LJMP(hlua_sub->hlua))
-                       trace = NULL;
-               trace = hlua_traceback(hlua_sub->hlua->T, ", ");
-               /* At this point the execution is safe. */
-               RESET_SAFE_LJMP(hlua_sub->hlua);
-
-               ha_warning("Lua event_hdl: pausing the subscription because the function fails to keep up the pace (%u unconsumed events): %s\n", event_hdl_async_equeue_size(&hlua_sub->equeue), ((trace) ? trace : ""));
+               if (SET_SAFE_LJMP(hlua_sub->hlua)) {
+                       /* The following Lua call may fail. */
+                       trace = hlua_traceback(hlua_sub->hlua->T, ", ");
+                       /* At this point the execution is safe. */
+                       RESET_SAFE_LJMP(hlua_sub->hlua);
+               } else {
+                       /* Lua error was raised while fetching lua trace from current ctx */
+                       SEND_ERR(NULL, "Lua event_hdl: unexpected error (memory failure?).\n");
+               }
+               ha_warning("Lua event_hdl: pausing the subscription because the handler fails "
+                          "to keep up the pace (%u unconsumed events) from %s.\n",
+                          event_hdl_async_equeue_size(&hlua_sub->equeue),
+                          (trace) ? trace : "[unknown]");
        }
 
        if (HLUA_IS_RUNNING(hlua_sub->hlua)) {