decimad / luabind-deboostified

Create Lua bindings for your C++ code easily - my improvements
http://www.vrac.iastate.edu/vancegroup/docs/luabind/
Other
70 stars 27 forks source link

assertion failure in object_rep::add_dependency #22

Open nitrocaster opened 8 years ago

nitrocaster commented 8 years ago

Although this is not specifically related to deboostifying, I considered it would be reasonable to open an issue here since it is the most active fork at the moment.

The problem was described here:

The minimal working example is provided by the author of the above topic:

I basically copy-pasted it here:

extern "C" {
#include <lualib.h>
};
#include <luabind/luabind.hpp>
#include <stdio.h>

using namespace luabind;

class Widget
{
public:
    Widget(const char* msg = "no msg", Widget* parent = nullptr) :
        msg(msg), parent(parent)
    {}
    virtual void foo() { puts("Widget::foo()"); }

    std::string msg;
    Widget* parent;
};

class WidgetWrap : public Widget, public luabind::wrap_base
{
public:
    WidgetWrap(const char* msg, Widget* parent) :
        Widget(msg, parent)
    {}
    void foo() override { call<void>("foo"); }
    static void default_foo(Widget* base) { base->Widget::foo(); }
};

int main()
{
    lua_State* L = luaL_newstate();
    luaL_openlibs(L);
    luabind::open(L);
    module(L)
    [
        class_<Widget, no_bases, default_holder, WidgetWrap>("Widget")
            .def(constructor<const char*, Widget*>())
            .def_readonly("parent", &Widget::parent)
            .def("foo", &Widget::foo, &WidgetWrap::default_foo)
    ];
    const char* code =
        "class 'MyWidget' (Widget)\n"
        "function MyWidget:__init(msg, parent)\n"
        "  Widget.__init(self, msg, parent)\n"
        "  if parent ~= nil then\n"
        "    self.parent.num = 0\n"
        "  end\n"
        "end\n"
        "function MyWidget:foo()\n"
        "  self.parent.num = self.parent.num + 1\n"
        "end\n"
        "parent = MyWidget('parent')\n"
        "child = MyWidget('child', parent)\n";
    luaL_dostring(L, code);
    auto child = object_cast<Widget*>(globals(L)["child"]);
    for (int i = 0; i < 48; i++)
        child->foo();
    return 0;
}
nitrocaster commented 8 years ago

It turns out previously there was a saparate table for reference storage: f1fe251c

Here

self.parent.num = self.parent.num + 1

auto-generated getter for parent returns a reference, which is equivalent (for non-primitive types) to calling a function with return_reference_to policy. In this particular case, each time foo() is called, object_rep gets 2 additional references. These references are not collected by GC. It seems for me that the new referencing scheme (not sure about old variant) is not supposed to delete references ever before corresponding c++ object gets destroyed.

@decimad, any thoughts?

decimad commented 8 years ago

Well I had a quick look and it seems to me, that the automatic reference counting is wrong. The member is looked up (pointer), then luabind finds there's already an object_rep for it (global "parent" variable), so a dependency is added but no holder for it is created. Because no holder gets created, there's no holder which could be destroyed, thus the reference counting scheme doesn't work anymore. The dependency policy should really not add a ref, if we're accessing a property which is a pointer to a different object which can be found by get_back_reference... But I don't know how to achieve this currently...

decimad commented 8 years ago

Addind a custom overload for a pointer-to-pointer-member solves the problem at hand, however this pointer might point to a true member, so...

template <class T, class D>
object make_get(lua_State* L, D* T::* mem_ptr, std::true_type /*member_ptr*/) const
{
    using result_type = typename reference_result<D*>::type;
    using get_signature = meta::type_list<result_type, Class const&>;

    return make_function(L, access_member_ptr<T, D*, result_type>(mem_ptr), get_signature(), GetPolicies());
}
decimad commented 8 years ago

One might be able to replace the pointer converter with a custom one in this case. If it converts to a pointer that is outside of (&object, &object + sizeof(object)) then it would not add a dependency...

decimad commented 8 years ago

Oh, i think the right way to go would be to have the pointer-converter of a member-access create an object_rep (through make_pointer_instance) even if there is already a reference to the pointed-to-object.

decimad commented 7 years ago

I developed a patch on branch "experimental" which trades off performance for ultimate correctness / memory saving. It would be great if brave people could try that one out, I currently don't have much code triggering these cases at hand.