sol2 icon indicating copy to clipboard operation
sol2 copied to clipboard

Set an object to nil after destroying it

Open crwn1337 opened this issue 3 years ago • 1 comments

I've been wondering is there anyway to make the variable nil after calling destroy? I don't want to do i:destroy(); i = nil; every time i want to destroy the 'item' object.

Short reproducible test case
#include <iostream>
#include "sol/sol.hpp"

struct item {
	std::string name;
};

std::vector<std::unique_ptr<item>> items;

int main() {
	sol::state lua;
	lua.open_libraries(sol::lib::base);

	lua.new_usertype<item>(
		"item", sol::call_constructor,
			sol::factories([](const std::string& name) {
				items.emplace_back(std::make_unique<item>(name));
				return std::cref(items.back());
			}),
		"destroy", [](item* self) {
			const auto i = std::ranges::find_if(items, [self](const std::unique_ptr<item>& a) { return self == a.get(); });
			if (i == items.end())
				return;

			items.erase(i);
			// set object to nil somehow
		},
		"print", [](const item *self) {
			printf("name: %s\n", self->name.c_str());
		}
	);

	lua.script(R"(
		local i = item("test")
		i:print()
		i:destroy()
		i:print() -- causes a segmentation fault (instead of a lua error) because 'i' doesn't exist in memory anymore
	)");

	return 0;
}

crwn1337 avatar Jun 05 '22 12:06 crwn1337

I dont think you can edit the value of a variable like that. Sounds really hacky. You could maybe look into the debug libraries and how they function, but other than that Im not sure. With setlocal (https://www.lua.org/manual/5.2/manual.html#lua_setlocal) you can probably achieve something like that, but I think you will find it difficult to set all variables that reference the "item" to nil, as you can maybe set only a specific variable at a time to nil.

One option is to have a wrapper that holds a boolean for existence of the object. Then you would need to check that boolean on each access (which could be automated through extension points, see https://github.com/ThePhD/sol2/issues/1074). Though this could slow things down as you might imagine. Makes me think of std::shared_ptr and std::weak_ptr.

Another option is to try remove the metatable of the object, but I dont know if this interferes with sol in some way. It definitely interferes with any implementation that requires the use of __gc metamethod. From https://sol2.readthedocs.io/en/latest/api/usertype_memory.html we can see that For T* type there is no deleter needed, so it likely has no __gc metamethod and we can probably delete the metatable. The resulting error might be a bit weird for the user, but its an error nevertheless and quite fast, safe and actually might use this myself also. Hmm. Here is an example https://godbolt.org/z/1Er3cqqfW and now we get a proper error instead of undefined behavior.

[sol3] An error occurred and panic has been invoked: [string "..."]:5: attempt to index a userdata value (local 'i')
terminate called after throwing an instance of 'sol::error'
  what():  lua: error: [string "..."]:5: attempt to index a userdata value (local 'i')

The error isnt very informative to the user though. The error could be improved probably if instead of deleting the metatable, a new metatable is made that then holds data in a specific way for sol to show in an error message when the object is used. It could indicate that you are using an already deleted object. I was going to use a plain table as a metatable, but in this example below I decided to use sol usertype instead as with that I can directly assume that sol knows how to access the usertype name on its own for any error messages that I want to have the usertype name instead of a generic "userdata" printed as the type. Example: https://godbolt.org/z/5xE95hc47

#include <iostream>
#include <ranges>

#include "sol/sol.hpp"

struct item {
	std::string name;
};

struct already_destroyed_object {};

std::vector<std::unique_ptr<item>> items;

int main() {
	sol::state lua;
	lua.open_libraries(sol::lib::base);

	lua.new_usertype<already_destroyed_object>(
		"already destroyed object", sol::no_constructor,
        "__index", [](sol::stack_object o){ luaL_argerror(o.lua_state(), o.stack_index(), "Trying to use an already destroyed object"); },
        "__newindex", [](sol::stack_object o){ luaL_argerror(o.lua_state(), o.stack_index(), "Trying to use an already destroyed object"); }
	);

	lua.new_usertype<item>(
		"item", sol::call_constructor,
			sol::factories([](const std::string& name) {
				items.emplace_back(std::make_unique<item>(name));
				return std::cref(items.back());
			}),
		"destroy", [](sol::object o) {
            auto self = o.as<item*>();
			const auto i = std::ranges::find_if(items, [self](const std::unique_ptr<item>& a) { return self == a.get(); });
			if (i == items.end())
				return;

			items.erase(i);
			// set object to nil somehow
            o.push();
            // Assertion in case you try to set the metatable of a value that has __gc
            // This is the case for example for types passed by value. All values are expected to be passed by pointer.
            if (luaL_getmetafield (o.lua_state(), -1, "__gc")) {
                throw sol::error("Trying to destroy a value with __gc metamethod without calling it!");
            }
            luaL_setmetatable(o.lua_state(), sol::usertype_traits<already_destroyed_object>::metatable().c_str());
		},
		"print", [](const item *self) {
			printf("name: %s\n", self->name.c_str());
		}
	);

	lua.script(R"(
		local i = item("test")
		i:print()
		i:destroy()
        print(i)
		i:print() -- causes a segmentation fault (instead of a lua error) because 'i' doesn't exist in memory anymore
	)");

	return 0;
}
somefunc(i)

[sol3] An error occurred and panic has been invoked: stack index 1, expected userdata, received sol.already_destroyed_object: value at this index does not properly reflect the desired type (bad argument into 'void(const item*)')
terminate called after throwing an instance of 'sol::error'
  what():  lua: error: stack index 1, expected userdata, received sol.already_destroyed_object: value at this index does not properly reflect the desired type (bad argument into 'void(const item*)')
i:somefunc()

[sol3] An error occurred and panic has been invoked: [string "..."]:6: bad argument #1 to '__index' (Trying to use an already destroyed object)
terminate called after throwing an instance of 'sol::error'
  what():  lua: error: [string "..."]:6: bad argument #1 to '__index' (Trying to use an already destroyed object)
print(i)

sol.already_destroyed_object: 0xb33b68

Rochet2 avatar Jun 05 '22 19:06 Rochet2