Merge pull request #1984 from secondlife/lua-no-reuse

Remove ability to reuse a `LuaState` between `LLLUAmanager` functions.

7 files changed, 55 insertions, 83 deletions

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<LLCheckBoxCtrl>("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<std::pair<int, LLSD>>
+LLCoros::Future<LLLUAmanager::script_result>
 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<LLCoros::Promise<std::pair<int, LLSD>>>() };
+    auto promise{ std::make_shared<LLCoros::Promise<script_result>>() };
     runScriptFile(filename,
                   [promise](int count, LLSD result)
                   { promise->set_value({ count, result }); });
     return LLCoros::getFuture(*promise);
 }
 
-std::pair<int, LLSD> 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<std::pair<int, LLSD>>
-LLLUAmanager::startScriptLine(LuaState& L, const std::string& chunk)
+LLCoros::Future<LLLUAmanager::script_result>
+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<LLCoros::Promise<std::pair<int, LLSD>>>() };
-    runScriptLine(L, chunk,
+    auto promise{ std::make_shared<LLCoros::Promise<script_result>>() };
+    runScriptLine(chunk,
                   [promise](int count, LLSD result)
                   { promise->set_value({ count, result }); });
     return LLCoros::getFuture(*promise);
 }
 
-std::pair<int, LLSD> 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<void(int count, const LLSD& result)> script_result_fn;
+    // same semantics as script_result_fn parameters
+    typedef std::pair<int, LLSD> 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<std::pair<int, LLSD>>
-        startScriptFile(const std::string& filename);
+    static LLCoros::Future<script_result> 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<int, LLSD> 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<std::pair<int, LLSD>>
-        startScriptLine(LuaState& L, const std::string& chunk);
+    static LLCoros::Future<script_result> 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<int, LLSD> waitScriptLine(LuaState& L, const std::string& chunk);
+    static script_result waitScriptLine(const std::string& chunk);
 
     static const std::map<std::string, std::string> getScriptNames() { return sScriptNames; }
 
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/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:
     </text>
-    <check_box
-     follows="right|top"
-     height="15"
-     label="Use clean lua_State"
-     layout="topleft"
-     top="10"
-     right ="-70"
-     name="clean_lua_state"
-     width="70"/>
     <line_editor
      border_style="line"
      border_thickness="1"
diff --git a/indra/newview/tests/llluamanager_test.cpp b/indra/newview/tests/llluamanager_test.cpp
index 824ddd445b..26a4ac95e3 100644
--- a/indra/newview/tests/llluamanager_test.cpp
+++ b/indra/newview/tests/llluamanager_test.cpp
@@ -123,11 +123,10 @@ namespace tut
     void object::test<1>()
     {
         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"
@@ -418,6 +413,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;
@@ -429,10 +430,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 +442,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 +468,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 +490,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);