sol2

sol2 copied to clipboard

Issue https://github.com/ThePhD/sol2/issues/1074 introduced a new customization point to handle checked access. This is currently done only for "real" function arguments, and not for "implicit" argument like the this pointer when calling a member function. The following code illustrates the problem:

#include <sol/sol.hpp>
#include <memory>
#include <iostream>

// Create a user class
struct foo {
    int value;

    foo(int v) : value(v) {}
    ~foo() { value = 0; }

    void bar() { std::cout << value << std::endl; }
};

// Register customization points to sol to handle weak pointer
namespace sol {
    template<typename T>
    struct unique_usertype_traits<std::weak_ptr<T>> {
        static T* get(lua_State*, const std::weak_ptr<T>& ptr) noexcept {
            return ptr.lock().get();
        }

        static bool is_null(lua_State*, const std::weak_ptr<T>& ptr) noexcept {
            return ptr.expired();
        }
    };
}

template<typename T>
void sol_lua_check_access(sol::types<T>, lua_State* L, int index, sol::stack::record& tracking) {
    sol::optional<std::weak_ptr<T>&> maybe_checked =
        sol::stack::check_get<std::weak_ptr<T>&>(L, index, sol::no_panic, tracking);

    if (!maybe_checked.has_value()) {
        return;
    }

    if (maybe_checked->expired()) {
        // We want to throw if the pointer has expired
        throw std::runtime_error("object has been deleted");
    }
}

int main() {
    sol::state lua;

    // Register our user type
    auto foo_type = lua.new_usertype<foo>("Foo");
    foo_type.set_function("bar", &foo::bar);

    // Create an object and give Lua an observer to it
    std::shared_ptr<foo> owner = std::make_shared<foo>(42);
    lua["observer"] = std::weak_ptr<foo>(owner);

    // The observer works as expected
    lua.do_string("observer:bar()");

    // Reset the owner, so the observed object becomes invalid
    owner.reset();

    // We expect this to throw now; it does not! The call goes through.
    lua.do_string("observer:bar()");
}

Compiled with g++ -std=c++17 test.cpp -o test -I"." -I"/usr/include/lua5.2/" -llua5.2

I traced it down to lua_call_wrapper::call using sol::check_get() to fetch the this pointer. Function arguments use stack_detail::check_get_arg(), which is the only place in which the new customization point is used. Therefore, this does not go through this check.

I fixed this by adding the following just before the call to sol::check_get() (call.hpp:486):

if constexpr (meta::meta_detail::is_adl_sol_lua_check_access_v<Ta>) {
    stack::record tracking {};
    sol_lua_check_access(types<meta::unqualified_t<Ta>>(), L, 1, tracking);
}

I assume there are quite a few other places where this should be done, to cover all kinds of wrappers. Perhaps this is best added inside sol::check_get() to avoid duplication? I'm not sure why this was added only to stack_detail::check_get_arg() in the first place.

FYI this https://github.com/cschreib/sol2/commit/f01daeb521ae1628798ef0a21f2def2ae696ec1e is how I fixed it on a fork, but I'm not sure this is 100% correct. It also required my custom sol_lua_check_access() to support T and T*:

template<typename T>
void sol_lua_check_access(sol::types<T>, lua_State* L, int index, sol::stack::record& tracking) {
    using ObjectType = std::remove_pointer_t<T>;

    sol::optional<std::weak_ptr<ObjectType>&> maybe_checked =
        sol::stack::check_get<std::weak_ptr<ObjectType>&>(L, index, sol::no_panic, tracking);

    if (!maybe_checked.has_value()) {
        return;
    }

    if (maybe_checked->expired()) {
        // We want to throw if the pointer has expired
        throw std::runtime_error("object has been deleted");
    }
}

I tried just adding the sol_lua_check_access customization to sol::check_get(), but that created a circular dependency (need to call sol::check_get() to get the pointer to check if it is valid). So I take it sol::check_get() is too low level for this.