From b3fb23ee0c6d33f5eba3502328ffb0011b5f25fb Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 15 Aug 2024 09:49:34 -0400 Subject: Introduce lluau_checkstack(L, n); use instead of luaL_checkstack(). luaL_checkstack() accepts a third parameter which is included in the stack overflow error message. We've been passing nullptr, leading to messages of the form "stack overflow ((null))". lluau_checkstack() implicitly passes __FUNCTION__, so we can distinguish which underlying luaL_checkstack() call encountered the stack overflow condition. Also, when calling each atexit() function, pass Luau's debug.traceback() function as the lua_pcall() error handler. This should help diagnose errors in atexit() functions. --- indra/newview/llluamanager.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'indra/newview') diff --git a/indra/newview/llluamanager.cpp b/indra/newview/llluamanager.cpp index 7014c59e4e..0b1a73f4e9 100644 --- a/indra/newview/llluamanager.cpp +++ b/indra/newview/llluamanager.cpp @@ -66,7 +66,7 @@ std::string lua_print_msg(lua_State* L, const std::string_view& level) { // On top of existing Lua arguments, we're going to push tostring() and // duplicate each existing stack entry so we can stringize each one. - luaL_checkstack(L, 2, nullptr); + lluau_checkstack(L, 2); luaL_where(L, 1); // start with the 'where' info at the top of the stack std::ostringstream out; @@ -139,7 +139,7 @@ lua_function(get_event_pumps, "Events posted to replypump are queued for get_event_next().\n" "post_on(commandpump, ...) to engage LLEventAPI operations (see helpleap()).") { - luaL_checkstack(L, 2, nullptr); + lluau_checkstack(L, 2); auto& listener{ LuaState::obtainListener(L) }; // return the reply pump name and the command pump name on caller's lua_State lua_pushstdstring(L, listener.getReplyName()); @@ -153,7 +153,7 @@ lua_function(get_event_next, "is returned by get_event_pumps(). Blocks the calling chunk until an\n" "event becomes available.") { - luaL_checkstack(L, 2, nullptr); + lluau_checkstack(L, 2); auto& listener{ LuaState::obtainListener(L) }; const auto& [pump, data]{ listener.getNext() }; lua_pushstdstring(L, pump); -- cgit v1.2.3 From 376c890c095fbc59b83402255cc1036c411150b9 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 20 Aug 2024 21:12:25 -0400 Subject: Fix for #2237: intermittent Lua data stack overflow. Use a static unordered_map to allow a function receiving (lua_State* L) to look up the LuaState instance managing that lua_State. We've thought about this from time to time already. LuaState's constructor creates the map entry; its destructor removes it; the new static getParent(lua_State* L) method performs the lookup. Migrate lluau::set_interrupts_counter() and check_interrupts_counter() into LuaState member functions. Add a new mInterrupts counter for them. Importantly, LuaState::check_interrupts_counter(), which is indirectly called by a lua_callbacks().interrupt function, no longer performs any Lua stack operations. Empirically, it seems the Lua engine is capable of interrupting itself at a moment when re-entry confuses it. Change previous lluau::set_interrupts_counter(L, 0) calls to LuaState::getParent(L).set_interrupts_counter(0). Also add LuaStackDelta class, and a lua_checkdelta() helper macro, to verify that the Lua data stack depth on exit from a block differs from the depth on entry by exactly the expected amount. Sprinkle lua_checkdelta() macros in likely places. --- indra/newview/llluamanager.cpp | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'indra/newview') diff --git a/indra/newview/llluamanager.cpp b/indra/newview/llluamanager.cpp index 0b1a73f4e9..6de8829308 100644 --- a/indra/newview/llluamanager.cpp +++ b/indra/newview/llluamanager.cpp @@ -53,6 +53,7 @@ std::map LLLUAmanager::sScriptNames; lua_function(sleep, "sleep(seconds): pause the running coroutine") { + lua_checkdelta(L, -1); F32 seconds = lua_tonumber(L, -1); lua_pop(L, 1); llcoro::suspendUntilTimeout(seconds); @@ -125,6 +126,7 @@ lua_function(print_warning, "print_warning(args...): WARNING level logging") lua_function(post_on, "post_on(pumpname, data): post specified data to specified LLEventPump") { + lua_checkdelta(L, -2); std::string pumpname{ lua_tostdstring(L, 1) }; LLSD data{ lua_tollsd(L, 2) }; lua_pop(L, 2); @@ -139,6 +141,7 @@ lua_function(get_event_pumps, "Events posted to replypump are queued for get_event_next().\n" "post_on(commandpump, ...) to engage LLEventAPI operations (see helpleap()).") { + lua_checkdelta(L, 2); lluau_checkstack(L, 2); auto& listener{ LuaState::obtainListener(L) }; // return the reply pump name and the command pump name on caller's lua_State @@ -153,6 +156,7 @@ lua_function(get_event_next, "is returned by get_event_pumps(). Blocks the calling chunk until an\n" "event becomes available.") { + lua_checkdelta(L, 2); lluau_checkstack(L, 2); auto& listener{ LuaState::obtainListener(L) }; const auto& [pump, data]{ listener.getNext() }; @@ -271,6 +275,7 @@ std::string read_file(const std::string &name) lua_function(require, "require(module_name) : load module_name.lua from known places") { + lua_checkdelta(L); std::string name = lua_tostdstring(L, 1); lua_pop(L, 1); -- cgit v1.2.3 From d37aa5c1823fcb202e87b1457842e49655c72b95 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 20 Aug 2024 21:22:46 -0400 Subject: Defend timers.Timer(iterate=True) against long callbacks. Specifically, defend against a callback that runs so long it suspends at a point after the next timer tick. --- indra/newview/scripts/lua/require/timers.lua | 34 +++++++++++++++++++++------- 1 file changed, 26 insertions(+), 8 deletions(-) (limited to 'indra/newview') diff --git a/indra/newview/scripts/lua/require/timers.lua b/indra/newview/scripts/lua/require/timers.lua index e4938078dc..ab1615ffbf 100644 --- a/indra/newview/scripts/lua/require/timers.lua +++ b/indra/newview/scripts/lua/require/timers.lua @@ -34,35 +34,53 @@ function timers.Timer:new(delay, callback, iterate) callback = callback or function() obj:tick() end - local first = true + local calls = 0 if iterate then + -- With iterative timers, beware of running a timer callback which + -- performs async actions lasting longer than the timer interval. The + -- lengthy callback suspends, allowing leap to retrieve the next + -- event, which is a timer tick. leap calls a new instance of the + -- callback, even though the previous callback call is still + -- suspended... etc. 'in_callback' defends against that recursive + -- case. Rather than re-enter the suspended callback, drop the + -- too-soon timer event. (We could count the too-soon timer events and + -- iterate calling the callback, but it's a bathtub problem: the + -- callback could end up getting farther and farther behind.) + local in_callback = false obj.id = leap.eventstream( 'Timers', {op='scheduleEvery', every=delay}, function (event) local reqid = event.reqid - if first then - first = false + calls += 1 + if calls == 1 then dbg('timer(%s) first callback', reqid) -- discard the first (immediate) response: don't call callback return nil else - dbg('timer(%s) nth callback', reqid) - return callback(event) + if in_callback then + dbg('dropping timer(%s) callback %d', reqid, calls) + else + dbg('timer(%s) callback %d', reqid, calls) + in_callback = true + local ret = callback(event) + in_callback = false + return ret + end end end ).reqid - else + else -- (not iterate) obj.id = leap.eventstream( 'Timers', {op='scheduleAfter', after=delay}, function (event) + calls += 1 -- Arrange to return nil the first time, true the second. This -- callback is called immediately with the response to -- 'scheduleAfter', and if we immediately returned true, we'd -- be done, and the subsequent timer event would be discarded. - if first then - first = false + if calls == 1 then -- Caller doesn't expect an immediate callback. return nil else -- cgit v1.2.3 From 409745eb370d7609df70da81584142dffbfe9d3f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 20 Aug 2024 21:41:26 -0400 Subject: Fix a couple more set_interrupts_counter() calls. --- indra/newview/llluamanager.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/newview') diff --git a/indra/newview/llluamanager.cpp b/indra/newview/llluamanager.cpp index 6de8829308..22b51d7b72 100644 --- a/indra/newview/llluamanager.cpp +++ b/indra/newview/llluamanager.cpp @@ -57,7 +57,7 @@ lua_function(sleep, "sleep(seconds): pause the running coroutine") F32 seconds = lua_tonumber(L, -1); lua_pop(L, 1); llcoro::suspendUntilTimeout(seconds); - lluau::set_interrupts_counter(L, 0); + LuaState::getParent(L).set_interrupts_counter(0); return 0; }; @@ -162,7 +162,7 @@ lua_function(get_event_next, const auto& [pump, data]{ listener.getNext() }; lua_pushstdstring(L, pump); lua_pushllsd(L, data); - lluau::set_interrupts_counter(L, 0); + LuaState::getParent(L).set_interrupts_counter(0); return 2; } -- cgit v1.2.3