Guard against running a C function multiple times without an interrupt check in C functions accepting callbacks

Author: HaroldCindy

VM/src/lstrlib.cpp

@@ -808,6 +808,8 @@ static void add_value(MatchState* ms, luaL_Strbuf* b, const char* s, const char*
         int n;
         lua_pushvalue(L, 3);
         n = push_captures(ms, s, e);
+        // ServerLua: Check for interrupt to allow pre-emptive abort before calling replacement function
+        luau_callinterrupthandler(L, -4);
         lua_call(L, n, 1);
         break;
     }

VM/src/ltablib.cpp

@@ -24,6 +24,8 @@ static int foreachi(lua_State* L)
         lua_pushvalue(L, 2);   // function
         lua_pushinteger(L, i); // 1st argument
         lua_rawgeti(L, 1, i);  // 2nd argument
+        // ServerLua: Check for interrupt to allow pre-emptive abort before calling user iterator function
+        luau_callinterrupthandler(L, -4);
         lua_call(L, 2, 1);
         if (!lua_isnil(L, -1))
             return 1;
@@ -42,6 +44,8 @@ static int foreach (lua_State* L)
         lua_pushvalue(L, 2);  // function
         lua_pushvalue(L, -3); // key
         lua_pushvalue(L, -3); // value
+        // ServerLua: Check for interrupt to allow pre-emptive abort before calling user iterator function
+        luau_callinterrupthandler(L, -4);
         lua_call(L, 2, 1);
         if (!lua_isnil(L, -1))
             return 1;
@@ -329,6 +333,8 @@ static int sort_func(lua_State* L, const TValue* l, const TValue* r)
     setobj2s(L, L->top + 1, l);
     setobj2s(L, L->top + 2, r);
     L->top += 3; // safe because of LUA_MINSTACK guarantee
+    // ServerLua: Check for interrupt to allow pre-emptive abort before calling user comparison function
+    luau_callinterrupthandler(L, -4);
     luaD_call(L, L->top - 3, 1);
     L->top -= 1; // maintain stack depth
 

tests/SLConformance.test.cpp

@@ -52,7 +52,7 @@ typedef struct SLTestRuntimeState : lua_SLRuntimeState
 
     // Flags for interrupt check testing
     bool interrupt_should_clear = false;
-    bool metamethod_ran = false;
+    bool callback_ran = false;
 } RuntimeState;
 
 
@@ -729,37 +729,53 @@ static int reset_interrupt_test(lua_State* L)
 {
     RuntimeState* state = (RuntimeState*)L->userdata;
     state->interrupt_should_clear = true;
-    state->metamethod_ran = false;
+    state->callback_ran = false;
     return 0;
 }
 
-static int check_metamethod_ran(lua_State* L)
+static int check_callback_ran(lua_State* L)
 {
     RuntimeState* state = (RuntimeState*)L->userdata;
-    lua_pushboolean(L, state->metamethod_ran);
+    lua_pushboolean(L, state->callback_ran);
     return 1;
 }
 
-static int test_metamethod(lua_State* L)
+static int test_callback(lua_State* L)
 {
     RuntimeState* state = (RuntimeState*)L->userdata;
 
     if (state->interrupt_should_clear)
     {
-        luaL_error(L, "Interrupt handler did not run before metamethod!");
+        luaL_error(L, "Interrupt handler did not run before callback!");
     }
 
-    state->metamethod_ran = true;
+    state->callback_ran = true;
     lua_pushstring(L, "ok");
     return 1;
 }
 
-TEST_CASE("Metamethods receive interrupt checks")
+static int test_sort_callback(lua_State* L)
 {
-    runConformance("metamethod_interrupts.lua", nullptr, [](lua_State* L) {
-        // Set up interrupt handler that clears the flag only for metamethod interrupts (-3)
+    RuntimeState* state = (RuntimeState*)L->userdata;
+
+    if (state->interrupt_should_clear)
+    {
+        luaL_error(L, "Interrupt handler did not run before sort comparison!");
+    }
+
+    state->callback_ran = true;
+    // `table.sort()` gets very angry if we don't do something that looks
+    // like sorting _eventually_
+    lua_pushboolean(L, lua_lessthan(L, 1, 2));
+    return 1;
+}
+
+TEST_CASE("Metamethods and library callbacks receive interrupt checks")
+{
+    runConformance("metamethod_and_callback_interrupts.lua", nullptr, [](lua_State* L) {
+        // Set up interrupt handler that clears the flag for metamethod (-3) and library callback (-4) interrupts
         lua_callbacks(L)->interrupt = [](lua_State* L, int gc) {
-            if (gc != -3)
+            if (gc != -3 && gc != -4)
                 return;
 
             RuntimeState* state = (RuntimeState*)L->userdata;
@@ -770,11 +786,14 @@ TEST_CASE("Metamethods receive interrupt checks")
         lua_pushcfunction(L, reset_interrupt_test, "reset_interrupt_test");
         lua_setglobal(L, "reset_interrupt_test");
 
-        lua_pushcfunction(L, check_metamethod_ran, "check_metamethod_ran");
-        lua_setglobal(L, "check_metamethod_ran");
+        lua_pushcfunction(L, check_callback_ran, "check_callback_ran");
+        lua_setglobal(L, "check_callback_ran");
+
+        lua_pushcfunction(L, test_callback, "test_callback");
+        lua_setglobal(L, "test_callback");
 
-        lua_pushcfunction(L, test_metamethod, "test_metamethod");
-        lua_setglobal(L, "test_metamethod");
+        lua_pushcfunction(L, test_sort_callback, "test_sort_callback");
+        lua_setglobal(L, "test_sort_callback");
     });
 }
 

tests/conformance/metamethod_and_callback_interrupts.lua

@@ -0,0 +1,100 @@
+-- Test that interrupt handlers are called before metamethods and library callbacks
+-- The C side sets up an interrupt handler that clears a flag,
+-- and callbacks that throw if the flag wasn't cleared.
+
+-- Create a test object with test_callback as various metamethods
+local function create_test_object()
+    local obj = {}
+    local mt = {
+        __tostring = test_callback,
+        __tojson = test_callback,
+        __len = test_callback,
+        __add = test_callback,
+        __sub = test_callback,
+        __mul = test_callback,
+        __div = test_callback,
+        __unm = test_callback,
+        __index = test_callback,
+        __newindex = test_callback,
+    }
+    return setmetatable(obj, mt)
+end
+
+-- Create test object once to avoid intermediate function calls
+local obj = create_test_object()
+local obj_len = {}
+setmetatable(obj_len, {__len = test_callback})
+
+-- Test __tostring (via luaL_callmeta path)
+reset_interrupt_test()
+local result = tostring(obj)
+assert(result == "ok", "__tostring should return 'ok'")
+assert(check_callback_ran(), "__tostring metamethod should have run")
+
+-- Test __tojson (via JSON encoder)
+reset_interrupt_test()
+result = lljson.encode(obj)
+assert(result == '"ok"', "__tojson should return '\"ok\"'")
+assert(check_callback_ran(), "__tojson metamethod should have run")
+
+-- Test __len (via JSON encoder when no __tojson)
+reset_interrupt_test()
+result = lljson.encode(obj_len)
+assert(check_callback_ran(), "__len metamethod should have run")
+
+-- Test arithmetic metamethods (__add, __sub, __mul, __div, __unm)
+-- These go through callTMres or luaV_callTM paths
+reset_interrupt_test()
+local _ = obj + obj
+assert(check_callback_ran(), "__add metamethod should have run")
+
+reset_interrupt_test()
+_ = obj - obj
+assert(check_callback_ran(), "__sub metamethod should have run")
+
+reset_interrupt_test()
+_ = obj * obj
+assert(check_callback_ran(), "__mul metamethod should have run")
+
+reset_interrupt_test()
+_ = obj / obj
+assert(check_callback_ran(), "__div metamethod should have run")
+
+reset_interrupt_test()
+_ = -obj
+assert(check_callback_ran(), "__unm metamethod should have run")
+
+-- Test __index (via callTMres path)
+reset_interrupt_test()
+_ = obj.some_field
+assert(check_callback_ran(), "__index metamethod should have run")
+
+-- Test __newindex (via callTM path)
+reset_interrupt_test()
+obj.some_field = 123
+assert(check_callback_ran(), "__newindex metamethod should have run")
+
+-- Test library function callbacks
+
+-- Create test table and data
+local test_table = {1, 2, 3, 4, 5}
+local test_dict = {a = 1, b = 2, c = 3, d = 4, e = 5}
+local test_string = "aabbbcccc"
+
+reset_interrupt_test()
+table.sort(test_table, test_sort_callback)
+assert(check_callback_ran(), "table.sort comparison function should have run")
+
+reset_interrupt_test()
+table.foreachi(test_table, test_callback)
+assert(check_callback_ran(), "table.foreachi callback should have run")
+
+reset_interrupt_test()
+table.foreach(test_dict, test_callback)
+assert(check_callback_ran(), "table.foreach callback should have run")
+
+reset_interrupt_test()
+local result = string.gsub(test_string, "a", test_callback)
+assert(check_callback_ran(), "string.gsub replacement function should have run")
+
+return "OK"