From e26727e03bb02d26b3f0d83f748dbd8eb88e4940 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 18 Jun 2024 13:13:39 -0400 Subject: Remove special-case ~LuaState() code to call fiber.run(). Instead, make fiber.lua call LL.atexit(fiber.run) to schedule that final run() call at ~LuaState() time using the generic mechanism. Append an explicit fiber.run() call to a specific test in llluamanager_test.cpp because the test code wants to interact with multiple Lua fibers *before* we destroy the LuaState. --- indra/newview/scripts/lua/require/fiber.lua | 6 ++++++ indra/newview/tests/llluamanager_test.cpp | 6 ++++++ 2 files changed, 12 insertions(+) (limited to 'indra/newview') diff --git a/indra/newview/scripts/lua/require/fiber.lua b/indra/newview/scripts/lua/require/fiber.lua index cae27b936b..b3c684dd67 100644 --- a/indra/newview/scripts/lua/require/fiber.lua +++ b/indra/newview/scripts/lua/require/fiber.lua @@ -337,4 +337,10 @@ function fiber.run() return idle_done end +-- Make sure we finish up with a call to run(). That allows a consuming script +-- to kick off some number of fibers, do some work on the main thread and then +-- fall off the end of the script without explicitly calling fiber.run(). +-- run() ensures the rest of the fibers run to completion (or error). +LL.atexit(fiber.run) + return fiber diff --git a/indra/newview/tests/llluamanager_test.cpp b/indra/newview/tests/llluamanager_test.cpp index 2d525f7913..206dbd9e71 100644 --- a/indra/newview/tests/llluamanager_test.cpp +++ b/indra/newview/tests/llluamanager_test.cpp @@ -385,6 +385,12 @@ namespace tut "fiber.launch('catch_special', drain, catch_special)\n" "fiber.launch('requester(a)', requester, 'a')\n" "fiber.launch('requester(b)', requester, 'b')\n" + // A script can normally count on an implicit fiber.run() call + // because fiber.lua calls LL.atexit(fiber.run). But atexit() + // functions are called by ~LuaState(), which (in the code below) + // won't be called until *after* we expect to interact with the + // various fibers. So make an explicit call for test purposes. + "fiber.run()\n" ); LLSD requests; -- cgit v1.2.3 From 8c94ff566a4f9076607d1b988f3eb7ad7e200bd9 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 10 Jul 2024 15:14:13 -0400 Subject: Remove ability to reuse a LuaState between LLLUAmanager functions. Remove LLLUAmanager::mumbleScriptLine() LuaState& parameters. Make startScriptLine(), waitScriptLine() and runScriptLine() exactly parallel to startScriptFile(), waitScriptFile() and runScriptFile(). That means that runScriptLine()'s C++ coroutine instantiates and destroys its own LuaState, which means that LL.atexit() functions will run on the Lua-specific C++ coroutine rather than (say) the viewer's main coroutine. Introduce LLLUAmanager::script_result typedef for std::pair and use in method returns. Remove LuaState::initLuaState(); move its logic back into the constructor. Remove initLuaState() calls in the expr() error cases: they're moot now that we won't get subsequent expr() calls on the same LuaState instance. Remove LLFloaterLUADebug "Use clean lua_State" checkbox and the cleanLuaState() method. Remove mState member. Remove explicit LuaState declarations from LLLUAmanager tests. Adapt one test for implicit LuaState: it was directly calling LuaState::obtainListener() to discover the LuaListener's reply-pump name. But since that test also captures two leap.request() calls from the Lua script, it can just look at the "reply" key in either of those requests. --- indra/newview/llfloaterluadebug.cpp | 13 +----- indra/newview/llfloaterluadebug.h | 2 - indra/newview/llluamanager.cpp | 48 +++++++++------------- indra/newview/llluamanager.h | 20 ++++----- .../skins/default/xui/en/floater_lua_debug.xml | 9 ---- indra/newview/tests/llluamanager_test.cpp | 34 ++++++--------- 6 files changed, 43 insertions(+), 83 deletions(-) (limited to 'indra/newview') diff --git a/indra/newview/llfloaterluadebug.cpp b/indra/newview/llfloaterluadebug.cpp index f715327ec8..9981126e4f 100644 --- a/indra/newview/llfloaterluadebug.cpp +++ b/indra/newview/llfloaterluadebug.cpp @@ -27,7 +27,6 @@ #include "llfloaterluadebug.h" -#include "llcheckboxctrl.h" #include "lllineeditor.h" #include "lltexteditor.h" #include "llviewermenufile.h" // LLFilePickerReplyThread @@ -92,8 +91,7 @@ void LLFloaterLUADebug::onExecuteClicked() mResultOutput->setValue(""); std::string cmd = mLineInput->getText(); - cleanLuaState(); - LLLUAmanager::runScriptLine(mState, cmd, [this](int count, const LLSD& result) + LLLUAmanager::runScriptLine(cmd, [this](int count, const LLSD& result) { completion(count, result); }); @@ -174,12 +172,3 @@ void LLFloaterLUADebug::completion(int count, const LLSD& result) sep = ", "; } } - -void LLFloaterLUADebug::cleanLuaState() -{ - if(getChild("clean_lua_state")->get()) - { - //Reinit to clean lua_State - mState.initLuaState(); - } -} diff --git a/indra/newview/llfloaterluadebug.h b/indra/newview/llfloaterluadebug.h index ae30b7cf25..e513d9a095 100644 --- a/indra/newview/llfloaterluadebug.h +++ b/indra/newview/llfloaterluadebug.h @@ -58,14 +58,12 @@ class LLFloaterLUADebug : private: void completion(int count, const LLSD& result); - void cleanLuaState(); LLTempBoundListener mOutConnection; LLTextEditor* mResultOutput; LLLineEditor* mLineInput; LLLineEditor* mScriptPath; - LuaState mState; U32 mAck{ 0 }; bool mExecuting{ false }; }; diff --git a/indra/newview/llluamanager.cpp b/indra/newview/llluamanager.cpp index ccfa08078e..7014c59e4e 100644 --- a/indra/newview/llluamanager.cpp +++ b/indra/newview/llluamanager.cpp @@ -162,24 +162,25 @@ lua_function(get_event_next, return 2; } -LLCoros::Future> +LLCoros::Future LLLUAmanager::startScriptFile(const std::string& filename) { // Despite returning from startScriptFile(), we need this Promise to // remain alive until the callback has fired. - auto promise{ std::make_shared>>() }; + auto promise{ std::make_shared>() }; runScriptFile(filename, [promise](int count, LLSD result) { promise->set_value({ count, result }); }); return LLCoros::getFuture(*promise); } -std::pair LLLUAmanager::waitScriptFile(const std::string& filename) +LLLUAmanager::script_result LLLUAmanager::waitScriptFile(const std::string& filename) { return startScriptFile(filename).get(); } -void LLLUAmanager::runScriptFile(const std::string &filename, script_result_fn result_cb, script_finished_fn finished_cb) +void LLLUAmanager::runScriptFile(const std::string &filename, script_result_fn result_cb, + script_finished_fn finished_cb) { // A script_result_fn will be called when LuaState::expr() completes. LLCoros::instance().launch(filename, [filename, result_cb, finished_cb]() @@ -212,39 +213,25 @@ void LLLUAmanager::runScriptFile(const std::string &filename, script_result_fn r }); } -void LLLUAmanager::runScriptLine(const std::string& chunk, script_finished_fn cb) -{ - // A script_finished_fn is used to initialize the LuaState. - // It will be called when the LuaState is destroyed. - LuaState L(cb); - runScriptLine(L, chunk); -} - -void LLLUAmanager::runScriptLine(const std::string& chunk, script_result_fn cb) -{ - LuaState L; - // A script_result_fn will be called when LuaState::expr() completes. - runScriptLine(L, chunk, cb); -} - -LLCoros::Future> -LLLUAmanager::startScriptLine(LuaState& L, const std::string& chunk) +LLCoros::Future +LLLUAmanager::startScriptLine(const std::string& chunk) { // Despite returning from startScriptLine(), we need this Promise to // remain alive until the callback has fired. - auto promise{ std::make_shared>>() }; - runScriptLine(L, chunk, + auto promise{ std::make_shared>() }; + runScriptLine(chunk, [promise](int count, LLSD result) { promise->set_value({ count, result }); }); return LLCoros::getFuture(*promise); } -std::pair LLLUAmanager::waitScriptLine(LuaState& L, const std::string& chunk) +LLLUAmanager::script_result LLLUAmanager::waitScriptLine(const std::string& chunk) { - return startScriptLine(L, chunk).get(); + return startScriptLine(chunk).get(); } -void LLLUAmanager::runScriptLine(LuaState& L, const std::string& chunk, script_result_fn cb) +void LLLUAmanager::runScriptLine(const std::string& chunk, script_result_fn result_cb, + script_finished_fn finished_cb) { // find a suitable abbreviation for the chunk string std::string shortchunk{ chunk }; @@ -256,12 +243,15 @@ void LLLUAmanager::runScriptLine(LuaState& L, const std::string& chunk, script_r shortchunk = stringize(shortchunk.substr(0, shortlen), "..."); std::string desc{ "lua: " + shortchunk }; - LLCoros::instance().launch(desc, [&L, desc, chunk, cb]() + LLCoros::instance().launch(desc, [desc, chunk, result_cb, finished_cb]() { + // A script_finished_fn is used to initialize the LuaState. + // It will be called when the LuaState is destroyed. + LuaState L(finished_cb); auto [count, result] = L.expr(desc, chunk); - if (cb) + if (result_cb) { - cb(count, result); + result_cb(count, result); } }); } diff --git a/indra/newview/llluamanager.h b/indra/newview/llluamanager.h index dcbb91f799..50f922a80f 100644 --- a/indra/newview/llluamanager.h +++ b/indra/newview/llluamanager.h @@ -55,32 +55,32 @@ public: // count > 1 with result.isArray() means the script returned multiple // results, represented as the entries of the result array. typedef std::function script_result_fn; + // same semantics as script_result_fn parameters + typedef std::pair script_result; - static void runScriptFile(const std::string &filename, script_result_fn result_cb = {}, script_finished_fn finished_cb = {}); + static void runScriptFile(const std::string &filename, script_result_fn result_cb = {}, + script_finished_fn finished_cb = {}); // Start running a Lua script file, returning an LLCoros::Future whose // get() method will pause the calling coroutine until it can deliver the // (count, result) pair described above. Between startScriptFile() and // Future::get(), the caller and the Lua script coroutine will run // concurrently. - static LLCoros::Future> - startScriptFile(const std::string& filename); + static LLCoros::Future startScriptFile(const std::string& filename); // Run a Lua script file, and pause the calling coroutine until it completes. // The return value is the (count, result) pair described above. - static std::pair waitScriptFile(const std::string& filename); + static script_result waitScriptFile(const std::string& filename); - static void runScriptLine(const std::string &chunk, script_finished_fn cb = {}); - static void runScriptLine(const std::string &chunk, script_result_fn cb); - static void runScriptLine(LuaState& L, const std::string &chunk, script_result_fn cb = {}); + static void runScriptLine(const std::string &chunk, script_result_fn result_cb = {}, + script_finished_fn finished_cb = {}); // Start running a Lua chunk, returning an LLCoros::Future whose // get() method will pause the calling coroutine until it can deliver the // (count, result) pair described above. Between startScriptLine() and // Future::get(), the caller and the Lua script coroutine will run // concurrently. - static LLCoros::Future> - startScriptLine(LuaState& L, const std::string& chunk); + static LLCoros::Future startScriptLine(const std::string& chunk); // Run a Lua chunk, and pause the calling coroutine until it completes. // The return value is the (count, result) pair described above. - static std::pair waitScriptLine(LuaState& L, const std::string& chunk); + static script_result waitScriptLine(const std::string& chunk); static const std::map getScriptNames() { return sScriptNames; } diff --git a/indra/newview/skins/default/xui/en/floater_lua_debug.xml b/indra/newview/skins/default/xui/en/floater_lua_debug.xml index 012ea6f254..15027f1647 100644 --- a/indra/newview/skins/default/xui/en/floater_lua_debug.xml +++ b/indra/newview/skins/default/xui/en/floater_lua_debug.xml @@ -25,15 +25,6 @@ width="100"> LUA string: - () { set_test_name("test Lua results"); - LuaState L; for (auto& luax : lua_expressions) { auto [count, result] = - LLLUAmanager::waitScriptLine(L, "return " + luax.expr); + LLLUAmanager::waitScriptLine("return " + luax.expr); auto desc{ stringize("waitScriptLine(", luax.desc, "): ") }; // if count < 0, report Lua error message ensure_equals(desc + result.asString(), count, 1); @@ -144,8 +143,7 @@ namespace tut "data = ", construct, "\n" "LL.post_on('testpump', data)\n" )); - LuaState L; - auto [count, result] = LLLUAmanager::waitScriptLine(L, lua); + auto [count, result] = LLLUAmanager::waitScriptLine(lua); // We woke up again ourselves because the coroutine running Lua has // finished. But our Lua chunk didn't actually return anything, so we // expect count to be 0 and result to be undefined. @@ -182,11 +180,10 @@ namespace tut "LL.post_on('testpump', data)\n" "LL.post_on('testpump', 'exit')\n" ); - LuaState L; // It's important to let the startScriptLine() coroutine run // concurrently with ours until we've had a chance to post() our // reply. - auto future = LLLUAmanager::startScriptLine(L, lua); + auto future = LLLUAmanager::startScriptLine(lua); StringVec expected{ "entry", "get_event_pumps()", @@ -217,8 +214,7 @@ namespace tut "pump, data = LL.get_event_next()\n" "return data\n" ); - LuaState L; - auto future = LLLUAmanager::startScriptLine(L, lua); + auto future = LLLUAmanager::startScriptLine(lua); // We woke up again ourselves because the coroutine running Lua has // reached the get_event_next() call, which suspends the calling C++ // coroutine (including the Lua code running on it) until we post @@ -356,8 +352,7 @@ namespace tut sendReply(data, data); })); - LuaState L; - auto [count, result] = LLLUAmanager::waitScriptLine(L, lua); + auto [count, result] = LLLUAmanager::waitScriptLine(lua); ensure_equals("Lua script didn't return item", count, 1); ensure_equals("echo failed", result, llsd::map("a", "a", "b", "b")); } @@ -371,18 +366,18 @@ namespace tut "\n" "fiber = require('fiber')\n" "leap = require('leap')\n" - "-- debug = require('printf')\n" "local function debug(...) end\n" + "-- debug = require('printf')\n" "\n" "-- negative priority ensures catchall is always last\n" - "catchall = leap.WaitFor:new(-1, 'catchall')\n" + "catchall = leap.WaitFor(-1, 'catchall')\n" "function catchall:filter(pump, data)\n" " debug('catchall:filter(%s, %s)', pump, data)\n" " return data\n" "end\n" "\n" "-- but first, catch events with 'special' key\n" - "catch_special = leap.WaitFor:new(2, 'catch_special')\n" + "catch_special = leap.WaitFor(2, 'catch_special')\n" "function catch_special:filter(pump, data)\n" " debug('catch_special:filter(%s, %s)', pump, data)\n" " return if data['special'] ~= nil then data else nil\n" @@ -429,10 +424,7 @@ namespace tut requests.append(data); })); - LuaState L; - auto future = LLLUAmanager::startScriptLine(L, lua); - auto replyname{ L.obtainListener().getReplyName() }; - auto& replypump{ LLEventPumps::instance().obtain(replyname) }; + auto future = LLLUAmanager::startScriptLine(lua); // LuaState::expr() periodically interrupts a running chunk to ensure // the rest of our coroutines get cycles. Nonetheless, for this test // we have to wait until both requester() coroutines have posted and @@ -444,6 +436,8 @@ namespace tut llcoro::suspend(); } ensure_equals("didn't get both requests", requests.size(), 2); + auto replyname{ requests[0]["reply"].asString() }; + auto& replypump{ LLEventPumps::instance().obtain(replyname) }; // moreover, we expect they arrived in the order they were created ensure_equals("a wasn't first", requests[0]["name"].asString(), "a"); ensure_equals("b wasn't second", requests[1]["name"].asString(), "b"); @@ -468,8 +462,7 @@ namespace tut "\n" "LL.get_event_next()\n" ); - LuaState L; - auto future = LLLUAmanager::startScriptLine(L, lua); + auto future = LLLUAmanager::startScriptLine(lua); // Poke LLTestApp to send its preliminary shutdown message. mApp.setQuitting(); // but now we have to give the startScriptLine() coroutine a chance to run @@ -491,8 +484,7 @@ namespace tut " x = 1\n" "end\n" ); - LuaState L; - auto [count, result] = LLLUAmanager::waitScriptLine(L, lua); + auto [count, result] = LLLUAmanager::waitScriptLine(lua); // We expect the above erroneous script has been forcibly terminated // because it ran too long without doing any actual work. ensure_equals(desc + " count: " + result.asString(), count, -1); -- cgit v1.2.3