sol2 icon indicating copy to clipboard operation
sol2 copied to clipboard

Critical: std::shared_ptr wrong type dispatch on MSVC Release

Open deadlocklogic opened this issue 2 years ago • 2 comments

  1. Repro
#define SOL_ALL_SAFETIES_ON 1
#include <sol/sol.hpp>

struct ClassA
{
    virtual ~ClassA() = default;

    static std::shared_ptr<ClassA> makeShared()
    {
        return std::make_shared<ClassA>();
    }
};

class ClassB
{
};

int main()
{
    sol::state lua;

    lua.open_libraries(sol::lib::base);

    lua["call"] = [](sol::object ptr)
    {
        if ((ptr.is<std::shared_ptr<ClassB>>()))
        {
            return "ClassB"; // Actual behavior: should not get in here in our test!!!
        }
        if ((ptr.is<std::shared_ptr<ClassA>>()))
        {
            return "ClassA"; // Expected behavior
        }
        return "nullptr";
    };
    lua.new_usertype<ClassA>("ClassA", "makeShared", &ClassA::makeShared);
    lua.new_usertype<ClassB>("ClassB");

    lua.script(R"(
do
  local instance = ClassA.makeShared()
  assert(call(instance) == "ClassA")
end
)",
               [](lua_State*, sol::protected_function_result pfr)
               {
                   sol::error err = pfr;
                   std::cout << err.what() << std::endl;
                   return pfr;
               });

    return 0;
}
  1. Tests fails unexpectedly (Expected behavior: ClassA, Actual behavior: ClassB). Notice 1: this behavior is on MSVC Release (Debug/RelWithDebInfo work fine) Notice 2: std::unique_ptr behaves correctly.

  2. Compiler/IDE: Visual Studio Language: C++ sol2 version: latest @ThePhD 👀.

Thanks.

deadlocklogic avatar Nov 19 '23 23:11 deadlocklogic

I can repro that. In Release (where your assertion fails), note that it passes if you change the order of your checks (so it also passes the case for shared_ptr<ClassA>. Hmm, the reason is that in fact, ptr.is returns true for any shared_ptr type, such as shared_ptr<int>!

    // This seems to return true in Release, for sol::object of any shared_ptr<> type
    bool result = ptr.is<std::shared_ptr<int>>();

totalgee avatar Mar 09 '24 11:03 totalgee

@totalgee Exactly, weirdly enough. It is sad that these bugs exist in a nice library.

deadlocklogic avatar May 19 '24 16:05 deadlocklogic