From 7345891f6bfd1049054b101bbdd957bf8adad164 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petri=20H=C3=A4kkinen?=
 <petrihakkinen@users.noreply.github.com>
Date: Tue, 11 Apr 2023 22:46:55 +0300
Subject: [PATCH] Add lua_getuserdatadtor (#870)

Some userdata objects may need to support manual destruction in addition
to automatic GC. For example, files, threads, GPU resources and objects
with large external allocations.

With Lua, a finalizer can be _generically_ called by invoking the __gc
metamethod manually, but this is currently not possible with tagged
userdata in Luau because it's not possible to query the destructor
associated with an userdata. While it is possible to workaround this by
duplicating the destructor table locally on client side (*), it's more
convenient to deduplicate the data and get the destructor using the API
instead.

(*) Note: a separate destructor table for each VM may be required if the
VMs use different set of tags.

Implementation notes:

1. I first considered adding a typedef for lua_Destructor but
unfortunately there are two kinds of destructors, one with and one
without the lua_State* argument, so I decided against it at this point.
Maybe it should be added later if the destructor API is unified (by
dropping the Lua state pointer argument?).

2. For some reason the conformance test produced warning "qualifier
applied to function type has no meaning; ignored" on VS2017 (possibly
because the test framework does not like function pointers for some
reason?). I silenced this by pulling out the test expressions from those
CHECKs.
---
 VM/include/lua.h           | 6 +++++-
 VM/src/lapi.cpp            | 8 +++++++-
 VM/src/ludata.cpp          | 3 +--
 tests/Conformance.test.cpp | 9 +++++++--
 4 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/VM/include/lua.h b/VM/include/lua.h
index 3f5e99f..e49e6ad 100644
--- a/VM/include/lua.h
+++ b/VM/include/lua.h
@@ -313,7 +313,11 @@ LUA_API uintptr_t lua_encodepointer(lua_State* L, uintptr_t p);
 LUA_API double lua_clock();
 
 LUA_API void lua_setuserdatatag(lua_State* L, int idx, int tag);
-LUA_API void lua_setuserdatadtor(lua_State* L, int tag, void (*dtor)(lua_State*, void*));
+
+typedef void (*lua_Destructor)(lua_State* L, void* userdata);
+
+LUA_API void lua_setuserdatadtor(lua_State* L, int tag, lua_Destructor dtor);
+LUA_API lua_Destructor lua_getuserdatadtor(lua_State* L, int tag);
 
 LUA_API void lua_clonefunction(lua_State* L, int idx);
 
diff --git a/VM/src/lapi.cpp b/VM/src/lapi.cpp
index 1528aa3..08d64d5 100644
--- a/VM/src/lapi.cpp
+++ b/VM/src/lapi.cpp
@@ -1378,12 +1378,18 @@ void lua_setuserdatatag(lua_State* L, int idx, int tag)
     uvalue(o)->tag = uint8_t(tag);
 }
 
-void lua_setuserdatadtor(lua_State* L, int tag, void (*dtor)(lua_State*, void*))
+void lua_setuserdatadtor(lua_State* L, int tag, lua_Destructor dtor)
 {
     api_check(L, unsigned(tag) < LUA_UTAG_LIMIT);
     L->global->udatagc[tag] = dtor;
 }
 
+lua_Destructor lua_getuserdatadtor(lua_State* L, int tag)
+{
+    api_check(L, unsigned(tag) < LUA_UTAG_LIMIT);
+    return L->global->udatagc[tag];
+}
+
 void lua_clonefunction(lua_State* L, int idx)
 {
     luaC_checkGC(L);
diff --git a/VM/src/ludata.cpp b/VM/src/ludata.cpp
index c2110cb..13e8702 100644
--- a/VM/src/ludata.cpp
+++ b/VM/src/ludata.cpp
@@ -24,8 +24,7 @@ void luaU_freeudata(lua_State* L, Udata* u, lua_Page* page)
 {
     if (u->tag < LUA_UTAG_LIMIT)
     {
-        void (*dtor)(lua_State*, void*) = nullptr;
-        dtor = L->global->udatagc[u->tag];
+        lua_Destructor dtor = L->global->udatagc[u->tag];
         // TODO: access to L here is highly unsafe since this is called during internal GC traversal
         // certain operations such as lua_getthreaddata are okay, but by and large this risks crashes on improper use
         if (dtor)
diff --git a/tests/Conformance.test.cpp b/tests/Conformance.test.cpp
index 2a32bce..0a9d1f7 100644
--- a/tests/Conformance.test.cpp
+++ b/tests/Conformance.test.cpp
@@ -1352,9 +1352,14 @@ TEST_CASE("UserdataApi")
     lua_State* L = globalState.get();
 
     // setup dtor for tag 42 (created later)
-    lua_setuserdatadtor(L, 42, [](lua_State* l, void* data) {
+    auto dtor = [](lua_State* l, void* data) {
         dtorhits += *(int*)data;
-    });
+    };
+    bool dtorIsNull = lua_getuserdatadtor(L, 42) == nullptr;
+    CHECK(dtorIsNull);
+    lua_setuserdatadtor(L, 42, dtor);
+    bool dtorIsSet = lua_getuserdatadtor(L, 42) == dtor;
+    CHECK(dtorIsSet);
 
     // light user data
     int lud;