1 files changed, 74 insertions, 27 deletions

diff --git a/indra/llcommon/lua_function.h b/indra/llcommon/lua_function.h
index 8b93053a46..7a3d9e7dd7 100644
--- a/indra/llcommon/lua_function.h
+++ b/indra/llcommon/lua_function.h
@@ -213,19 +213,21 @@ std::string lua_emplace_metaname(const std::string& Tname = LLError::Log::classn
  * On the stack belonging to the passed lua_State, push a Lua userdata object
  * with a newly-constructed C++ object std::optional<T>(args...). The new
  * userdata has a metadata table with a __gc() function to ensure that when
- * the userdata instance is garbage-collected, ~T() is called.
+ * the userdata instance is garbage-collected, ~T() is called. Also call
+ * LL.atexit(lua_emplace_call_gc<T>(object)) to make ~LuaState() call ~T().
  *
  * We wrap the userdata object as std::optional<T> so we can explicitly
  * destroy the contained T, and detect that we've done so.
  *
  * Usage:
  * lua_emplace<T>(L, T constructor args...);
+ * // L's Lua stack top is now a userdata containing T
  */
 template <class T, typename... ARGS>
 void lua_emplace(lua_State* L, ARGS&&... args)
 {
     using optT = std::optional<T>;
-    luaL_checkstack(L, 3, nullptr);
+    luaL_checkstack(L, 5, nullptr);
     auto ptr = lua_newuserdata(L, sizeof(optT));
     // stack is uninitialized userdata
     // For now, assume (but verify) that lua_newuserdata() returns a
@@ -254,38 +256,94 @@ void lua_emplace(lua_State* L, ARGS&&... args)
     lua_setmetatable(L, -2);
     // Stack is now userdata, initialized with T(args),
     // with metatable.__gc pointing to lua_emplace_gc<T>.
+
     // But wait, there's more! Use our atexit() function to ensure that this
-    // C++ object is eventually cleaned up even if the garbage collector never
+    // C++ object is eventually destroyed even if the garbage collector never
     // gets around to it.
     lua_getglobal(L, "LL");
     // stack contains userdata, LL
     lua_getfield(L, -1, "atexit");
     // stack contains userdata, LL, LL.atexit
+    // ditch LL
+    lua_replace(L, -2);
+    // stack contains userdata, LL.atexit
+
+    // We have a bit of a problem here. We want to allow the garbage collector
+    // to collect the userdata if it must; but we also want to register a
+    // cleanup function to destroy the value if (usual case) it has NOT been
+    // garbage-collected. The problem is that if we bind into atexit()'s queue
+    // a strong reference to the userdata, we ensure that the garbage
+    // collector cannot collect it, making our metatable with __gc function
+    // completely moot. And we must assume that lua_pushcclosure() binds a
+    // strong reference to each value passed as a closure.
+
+    // The solution is to use one more indirection: create a weak table whose
+    // sole entry is the userdata. If all other references to the new userdata
+    // are forgotten, so the only remaining reference is the weak table, the
+    // userdata can be collected. Then we can bind that weak table as the
+    // closure value for our cleanup function.
+    // The new weak table will have at most 1 array value, 0 other keys.
+    lua_createtable(L, 1, 0);
+    // stack contains userdata, LL.atexit, weak_table
+    if (luaL_newmetatable(L, "weak_values"))
+    {
+        // stack contains userdata, LL.atexit, weak_table, weak_values
+        // just created "weak_values" metatable: populate it
+        // Registry.weak_values = {__mode="v"}
+        lua_pushliteral(L, "v");
+        // stack contains userdata, LL.atexit, weak_table, weak_values, "v"
+        lua_setfield(L, -2, "__mode");
+    }
+    // stack contains userdata, LL.atexit, weak_table, weak_values
+    // setmetatable(weak_table, weak_values)
+    lua_setmetatable(L, -2);
+    // stack contains userdata, LL.atexit, weak_table
+    lua_pushinteger(L, 1);
+    // stack contains userdata, LL.atexit, weak_table, 1
     // duplicate userdata
-    lua_pushvalue(L, -3);
-    // stack contains userdata, LL, LL.atexit, userdata
-    // push a closure binding (lua_emplace_call_gc<T>, userdata)
+    lua_pushvalue(L, -4);
+    // stack contains userdata, LL.atexit, weak_table, 1, userdata
+    // weak_table[1] = userdata
+    lua_settable(L, -3);
+    // stack contains userdata, LL.atexit, weak_table
+
+    // push a closure binding (lua_emplace_call_gc<T>, weak_table)
     auto callgcname{ stringize("lua_emplace_call_gc<", Tname, ">") };
     lua_pushcclosure(L, lua_emplace_call_gc<T>, callgcname.c_str(), 1);
-    // stack contains userdata, LL, LL.atexit, closure
+    // stack contains userdata, LL.atexit, closure
     // Call LL.atexit(closure)
     lua_call(L, 1, 0);
-    // stack contains userdata, LL
-    lua_pop(L, 1);
     // stack contains userdata -- return that
 }
 
 namespace {
 
-// passed to LL.atexit(closure(lua_emplace_call_gc<T>, userdata));
+// passed to LL.atexit(closure(lua_emplace_call_gc<T>, weak_table{userdata}));
 // retrieves bound userdata to pass to lua_emplace_gc<T>()
 template <class T>
 int lua_emplace_call_gc(lua_State* L)
 {
-    luaL_checkstack(L, 1, nullptr);
-    // retrieve the first (only) bound upvalue and push to stack top as the
-    // argument for lua_emplace_gc<T>()
+    luaL_checkstack(L, 2, nullptr);
+    // retrieve the first (only) bound upvalue and push to stack top
     lua_pushvalue(L, lua_upvalueindex(1));
+    // This is the weak_table bound by lua_emplace<T>(). Its one and only
+    // entry should be the lua_emplace<T>() userdata -- unless userdata has
+    // been garbage collected. Retrieve weak_table[1].
+    lua_pushinteger(L, 1);
+    // stack contains weak_table, 1
+    lua_gettable(L, -2);
+    // stack contains weak_table, weak_table[1]
+    // If our userdata was garbage-collected, there is no weak_table[1],
+    // and we just retrieved nil.
+    if (lua_isnil(L, -1))
+    {
+        lua_pop(L, 2);
+        return 0;
+    }
+    // stack contains weak_table, userdata
+    // ditch weak_table
+    lua_replace(L, -2);
+    // pass userdata to lua_emplace_gc<T>()
     return lua_emplace_gc<T>(L);
 }
 
@@ -325,23 +383,12 @@ template <class T>
 T* lua_toclass(lua_State* L, int index)
 {
     using optT = std::optional<T>;
-    luaL_checkstack(L, 2, nullptr);
+    // recreate the name lua_emplace<T>() uses for its metatable
+    auto metaname{ lua_emplace_metaname<T>() };
     // get void* pointer to userdata (if that's what it is)
-    auto ptr{ lua_touserdata(L, index) };
+    void* ptr{ luaL_checkudata(L, index, metaname.c_str()) };
     if (! ptr)
         return nullptr;
-    // push the metatable for this userdata, if any
-    if (! lua_getmetatable(L, index))
-        return nullptr;
-    // now push the metatable created by lua_emplace<T>()
-    auto metaname{ lua_emplace_metaname<T>() };
-    luaL_getmetatable(L, metaname.c_str());
-    auto equal{ lua_equal(L, -1, -2) };
-    // Having compared the userdata's metatable with the one set by
-    // lua_emplace<T>(), we no longer need either metatable on the stack.
-    lua_pop(L, 2);
-    if (! equal)
-        return nullptr;
     // Derive the optT* from ptr. If in future lua_emplace() must manually
     // align our optT* within the Lua-provided void*, adjust accordingly.
     optT* tptr(static_cast<optT*>(ptr));