Fix memory leak when indexing user types

[Assumeru]

sol::u_detail::usertype_storage_base::self_index_call contains auto it = self.auxiliary_keys.find(k). k is a stack_reference and auxiliary_keys an unordered map with stateless_reference keys.

When compiled with Visual Studio 2022, k is implicitly converted to a stateless_reference via stateless_reference(const stack_reference&) and stateless_reference(lua_State*, int). The latter adds a reference to the registry through ref = luaL_ref(L_, LUA_REGISTRYINDEX), but stateless_reference's destructor doesn't call deref so the registry table retains the value even after the call to find completes.

I don't know if stateless_reference should have a destructor that takes care of this. If not, I'd say its constructors should probably be marked explicit to avoid doing this by accident. Regardless, this PR skirts around that by avoiding the conversion in the first place. Which seems desirable.

I don't know if this affects other compilers/standard libraries/code paths. @magicaldave's comment on https://gitlab.com/OpenMW/openmw/-/issues/8614 suggests that some platforms might be unaffected.

[Nerixyz]

This doesn't work before C++ 20, because these lack support for transparent lookup in std::unordered_map. The following should do it in both cases:

diff --git a/include/sol/usertype_storage.hpp b/include/sol/usertype_storage.hpp
index bdca6146..691a025b 100644
--- a/include/sol/usertype_storage.hpp
+++ b/include/sol/usertype_storage.hpp
@@ -502,7 +502,11 @@ namespace sol { namespace u_detail {
 					stateless_reference* target = nullptr;
 					{
 						stack_reference k = stack::get<stack_reference>(L, 2);
+#if __cpp_lib_generic_unordered_lookup >= 201811L
 						auto it = self.auxiliary_keys.find(k);
+#else
+						auto it = self.auxiliary_keys.find(basic_reference(k));
+#endif
 						if (it != self.auxiliary_keys.cend()) {
 							target = &it->second;
 						}

A test (based on one example) to ensure this doesn't happen again:

Test

#include <catch2/catch_all.hpp>

#include <sol/sol.hpp>

TEST_CASE("#1716 - index and newindex should not leak values into the registry", "[sol2][regression][issue1716]") {
	class vector {
	public:
		vector() = default;

		static int my_index(vector&, int) {
			return 0;
		}

		static void my_new_index(vector&, int, int) {
			return;
		}
	};

	sol::state lua;
	lua.open_libraries(sol::lib::base);
	lua.new_usertype<vector>("vector",
	     sol::constructors<sol::types<>>(), //
	     sol::meta_function::index,
	     &vector::my_index, //
	     sol::meta_function::new_index,
	     &vector::my_new_index);
	auto begin = lua.registry().size();
	lua.script(R"(
			local v = vector.new()
			local unused = 0
			for i=1,100 do
				unused = v[1]
				v[1] = 1
			end
		)");

	REQUIRE(lua.registry().size() <= begin + 1);
}

[Assumeru]

Oh right, C++17.

Thanks. I've added your changes.

[cengshanglou]

这在 C++ 20 之前不起作用，因为这些不支持 .在这两种情况下，以下作都应执行此作：std::unordered_map

diff --git a/include/sol/usertype_storage.hpp b/include/sol/usertype_storage.hpp
index bdca6146..691a025b 100644
--- a/include/sol/usertype_storage.hpp
+++ b/include/sol/usertype_storage.hpp
@@ -502,7 +502,11 @@ namespace sol { namespace u_detail {
 					stateless_reference* target = nullptr;
 					{
 						stack_reference k = stack::get<stack_reference>(L, 2);
+#if __cpp_lib_generic_unordered_lookup >= 201811L
 						auto it = self.auxiliary_keys.find(k);
+#else
+						auto it = self.auxiliary_keys.find(basic_reference(k));
+#endif
 						if (it != self.auxiliary_keys.cend()) {
 							target = &it->second;
 						}

一个测试（基于一个示例）以确保这种情况不会再次发生：

测试

这在 C++ 20 之前不起作用，因为这些不支持 .在这两种情况下，以下作都应执行此作：std::unordered_map

diff --git a/include/sol/usertype_storage.hpp b/include/sol/usertype_storage.hpp
index bdca6146..691a025b 100644
--- a/include/sol/usertype_storage.hpp
+++ b/include/sol/usertype_storage.hpp
@@ -502,7 +502,11 @@ namespace sol { namespace u_detail {
 					stateless_reference* target = nullptr;
 					{
 						stack_reference k = stack::get<stack_reference>(L, 2);
+#if __cpp_lib_generic_unordered_lookup >= 201811L
 						auto it = self.auxiliary_keys.find(k);
+#else
+						auto it = self.auxiliary_keys.find(basic_reference(k));
+#endif
 						if (it != self.auxiliary_keys.cend()) {
 							target = &it->second;
 						}

一个测试（基于一个示例）以确保这种情况不会再次发生：

测试

100/5000
Is the conditional judgment of your macro written in reverse? I used c++20 and auto it = self.auxiliary_keys.find(basic_reference(k)); There will be no memory leak, but using the above one will cause a memory leak

[Nerixyz]

100/5000 Is the conditional judgment of your macro written in reverse? I used c++20 and auto it = self.auxiliary_keys.find(basic_reference(k)); There will be no memory leak, but using the above one will cause a memory leak

Did you apply the full PR (https://github.com/ThePhD/sol2/pull/1716.patch)?

With MSVC and C++ 20, I don't get any test failures and the added test runs:

> cmake -B build-20 -DCMAKE_CXX_STANDARD=20 -DSOL2_LUA_VERSION="5.4.8" -DSOL2_BUILD_LUA=On -DSOL2_TESTS=On -DSOL2_CI=On -DBUILD_LUA_AS_DLL=Off -G Ninja
> ninja -C build-20
> ctest --test-dir build-20 -j 20
...
20/25 Test #22: sol2.tests.regression_1716 .................................   Passed    0.10 sec
21/25 Test #23: sol2.tests.regression_1716.SOL_ALL_SAFETIES_ON .............   Passed    0.08 sec
...