shared_ptr use_count corruption / no downcasts with shared_ptr

[tripleslash]

Luabind causes a shared_ptr corruption when you do this in lua:

local gSharedObject = nil;
function Testfunction()
    gSharedObject = SharedObject.Create();
    return gSharedObject;
end

where the C++ export looks like that:

class SharedObject {
public:
    SharedObject() {}

    static boost::shared_ptr<SharedObject> Create() {
        return boost::make_shared<SharedObject>();
    }
}

luabind::module(L) [
    luabind::class_<SharedObject, boost::shared_ptr<SharedObject>>("SharedObject")
        .scope [ luabind::def("Create", &SharedObject::Create) ]
];

Now when you do the following you will get a use_count == 1 where it should actually be 2 because lua should hold a shared_ptr and your code should:

boost::shared_ptr<SharedObject> obj = luabind::call_function<boost::shared_ptr<SharedObject>>(L, "Testfunction");
std::cout << obj.use_count() << std::endl;
// prints 1 but should actually be 2, one from the current scope and one in lua globals, because the lua state is still alive... ?!?

This is a huge problem when your GC runs because it always thinks the shared_ptr is the only one in use and it will always release objects created like that, not matter if a ptr is used by the c++ code atm.

Example:

boost::shared_ptr<SharedObject> obj = luabind::call_function<boost::shared_ptr<SharedObject>>(L, "Testfunction");
std::cout << obj.use_count() << std::endl;

lua_close(L); // invokes ~SharedObject -_-

std::cout << obj.use_count() << std::endl; // still 1

The problem exists in lua 5.1 and 5.2, everywhere I checked

[tripleslash]

It works if you don't include shared_ptr_converter.hpp, please try it again using shared_ptr_converter.hpp.

[Oberon00]

I have added additional checks to test_automatic_smart_pointer in f9499ec now. I think this should quite exactly mirror what you reported as causing an error. However, I could not reproduce the problem at least with MSVC 11 (both x32 and x64), Lua 5.2 and Boost 1.53. I've also tried boost::make_shared() instead of "ordinary" construction but it makes no difference.
Please do not hesitate to provide any clarifications if I misunderstood you.

Your new comment came just in:
Ok, this explains it then. Just do not use shared_ptr_converter.hpp, I suspect it has been unmaintained for quite some time and is not necessary anyhow since Luabind can handle smart pointers transparently and automatically (it's even documented, contrary to shared_ptr_converter.hpp ).

I think I will just remove this header to save others from running into this trap. Thanks for reporting.

[tripleslash]

If you don't include shared_ptr_converter luabind should not be able to downcast smart pointers (static_pointer_cast) as far as I tested, that was a few months ago though and with the original luabind branch. Did that change?

Edit: I just tried it without shared_ptr_converter, it does exactly what I suspected: It isn't able to downcast the smart_pointer. lua> runtime error: No matching overload found. The function expects a shared_ptr<base and I'm passing a shared_ptr<derived to it.

[Oberon00]

If by original branch, you mean the last official release, then I don't think so. If you mean the luabind/luabind master branch, then I don't know.
So shared_ptr_converter has still a use then? Where do you expect downcasting? Do you mean automatic or manual downcasting? Does it work with plain pointers? Sorry for the many questions, I think providing a minimal (preferably compileable) example would be the best way of answering them.

[tripleslash]

class Base {
public:
    Base() {}
};

class Deriv : public Base {
public:
    Deriv() {}

    static boost::shared_ptr<Deriv> Create() {
        return boost::make_shared<Deriv>();
    }
};

// Expects Base object
void DoSomething(boost::shared_ptr<Base>) {
    // "No matching overload found" if shared_ptr_converter.hpp is not included.
}


int _tmain(int argc, _TCHAR* argv[])
{
    // create the wrapped lua state...
    std::shared_ptr<lua_State> myLuaState(
        luaL_newstate(), [] (lua_State *L) {
        lua_close(L);
    });

    luabind::open(myLuaState.get());

    luabind::module(myLuaState.get()) [
        luabind::class_<Base, boost::shared_ptr<Base>>("Base"),
        luabind::class_<Deriv, Base, boost::shared_ptr<Base>>("Deriv")
            .scope [ luabind::def("Create", &Deriv::Create) ],

        luabind::def("DoSomething", &DoSomething)
    ];

    luaL_dostring(myLuaState.get(),
        "function Testfunction()\n"
        "   DoSomething(Deriv.Create());\n"
        "end\n");

    try {
        luabind::call_function<void>(myLuaState.get(), "Testfunction");
    }
    catch (luabind::error &e) {
        luabind::object ex(luabind::from_stack(e.state(), -1));
        std::cout << e.what() << ": " << ex << std::endl;
    }
}

Regarding your question: Yes it does work with plain pointers. shared_ptr_converter basically extend that support for smart pointers. It tells luabind to treat the holder type as pointer type so it can apply static_pointer_cast.

Btw: Deutsch? :P

[Oberon00]

Thank you. I will investigate this.

[tripleslash]

I just found a post regarding this bug from Daniel Wallin: http://lua.2524044.n2.nabble.com/double-deletion-problem-with-shared-pointers-td7582150.html

After looking at it I saw that shared_ptr_converter adds a custom deleter which does some unref thing in the lua registry once your pointer runs out of scope, you must not call lua_close before any such objects are gone :(
Maybe you can find a fix for it though :P

Here is my modified shared_ptr_converter which enables downcasting for std::shared_ptr types. You might want to add that functionality to yours.

// Copyright Daniel Wallin 2009. Use, modification and distribution is
// subject to the Boost Software License, Version 1.0. (See accompanying
// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)

#ifndef LUABIND_SHARED_PTR_CONVERTER_090211_HPP
# define LUABIND_SHARED_PTR_CONVERTER_090211_HPP

# include <luabind/detail/decorate_type.hpp>  // for LUABIND_DECORATE_TYPE
# include <luabind/detail/policy.hpp>    // for default_converter, etc
# include <luabind/get_main_thread.hpp>  // for get_main_thread
# include <luabind/handle.hpp>           // for handle

# include <boost/mpl/bool.hpp>           // for bool_, false_
# include <boost/smart_ptr/shared_ptr.hpp>  // for shared_ptr, get_deleter

#include <boost/config.hpp>

#include <memory> // shared_ptr

#if BOOST_VERSION >= 105300
# include <luabind/detail/has_get_pointer.hpp>

# include <boost/get_pointer.hpp>

namespace luabind { namespace detail { namespace has_get_pointer_ {
  template<class T>
  struct impl<std::shared_ptr<T>> {
      BOOST_STATIC_CONSTANT(bool, value = true);
      typedef boost::mpl::bool_<value> type;
  };

  template<class T>
  struct impl<const std::shared_ptr<T>>: impl<std::shared_ptr<T>> { };

  template<class T>
  struct impl<volatile std::shared_ptr<T>>: impl<std::shared_ptr<T>> { };

  template<class T>
  struct impl<const volatile std::shared_ptr<T>>: impl<std::shared_ptr<T>> { };
}}
using boost::get_pointer;
}



#else // if BOOST_VERSION < 105300

// Not standard conforming: add function to ::std(::tr1)
namespace std {

#if defined(_MSC_VER) && _MSC_VER < 1700
namespace tr1 {
#endif

    template<class T>
    T * get_pointer(shared_ptr<T> const& p) { return p.get(); }

#if defined(_MSC_VER) && _MSC_VER < 1700
} // namespace tr1
#endif

} // namespace std

#endif // if BOOST_VERSION < 105300 / else

namespace luabind {

namespace detail
{

  struct shared_ptr_deleter
  {
      shared_ptr_deleter(lua_State* L, int index)
        : life_support(get_main_thread(L), L, index)
      {}

      void operator()(void const*)
      {
          handle().swap(life_support);
      }

      handle life_support;
  };

} // namespace detail

template <class T>
struct default_converter<boost::shared_ptr<T> >
  : default_converter<T*>
{
    typedef boost::mpl::false_ is_native;

    template <class U>
    int match(lua_State* L, U, int index)
    {
        return default_converter<T*>::match(
            L, LUABIND_DECORATE_TYPE(T*), index);
    }

    template <class U>
    boost::shared_ptr<T> apply(lua_State* L, U, int index)
    {
        T* raw_ptr = default_converter<T*>::apply(
            L, LUABIND_DECORATE_TYPE(T*), index);
        if (!raw_ptr)
            return boost::shared_ptr<T>();
        return boost::shared_ptr<T>(
            raw_ptr, detail::shared_ptr_deleter(L, index));
    }

    void apply(lua_State* L, boost::shared_ptr<T> const& p)
    {
        if (detail::shared_ptr_deleter* d =
                boost::get_deleter<detail::shared_ptr_deleter>(p))
        {
            d->life_support.push(L);
        }
        else
        {
            detail::value_converter().apply(L, p);
        }
    }

    template <class U>
    void converter_postcall(lua_State*, U const&, int)
    {}
};

template <class T>
struct default_converter<boost::shared_ptr<T> const&>
  : default_converter<boost::shared_ptr<T> >
{};

#ifdef BOOST_HAS_RVALUE_REFS
template <class T>
struct default_converter<boost::shared_ptr<T>&&>
  : default_converter<boost::shared_ptr<T> >
{};
#endif

// std::shared_ptr

template <class T>
struct default_converter<std::shared_ptr<T> >
  : default_converter<T*>
{
    typedef boost::mpl::false_ is_native;

    template <class U>
    int match(lua_State* L, U, int index)
    {
        return default_converter<T*>::match(
            L, LUABIND_DECORATE_TYPE(T*), index);
    }

    template <class U>
    std::shared_ptr<T> apply(lua_State* L, U, int index)
    {
        T* raw_ptr = default_converter<T*>::apply(
            L, LUABIND_DECORATE_TYPE(T*), index);
        if (!raw_ptr)
            return std::shared_ptr<T>();
        return std::shared_ptr<T>(
            raw_ptr, detail::shared_ptr_deleter(L, index));
    }

    void apply(lua_State* L, std::shared_ptr<T> const& p)
    {
        if (detail::shared_ptr_deleter* d =
                std::get_deleter<detail::shared_ptr_deleter>(p))
        {
            d->life_support.push(L);
        }
        else
        {
            detail::value_converter().apply(L, p);
        }
    }

    template <class U>
    void converter_postcall(lua_State*, U const&, int)
    {}
};

template <class T>
struct default_converter<std::shared_ptr<T> const&>
  : default_converter<std::shared_ptr<T> >
{};

#ifdef BOOST_HAS_RVALUE_REFS
template <class T>
struct default_converter<std::shared_ptr<T>&&>
  : default_converter<std::shared_ptr<T> >
{};
#endif

} // namespace luabind

#endif // LUABIND_SHARED_PTR_CONVERTER_090211_HPP

[Oberon00]

From what I read, what this shared_ptr_converters do is:
For C++ -> Lua

• They pass the given shared_ptr to Lua, using luabinds automatic smart pointer support, if it does not already have the custom deleter set.
• Otherwise, they know that this is already in Lua and just push the internal object representation already created.

For Lua -> C++, they only have access to the raw pointer and create a new shared_ptr from it, with the custom deleter set. This immediately explains the use_count problem but the premature deletion problem is a bit more complicated (see below).

The custom deleter contains a strong Lua reference to luabind's internal representation of the object, thus preventing it's collection. Upon deletion it just releases this reference. This means that when there exist no other references to the object (from other shared_ptrs to the object, adopted wrap_base classes or from Lua variables) Lua can collect the object representation. If this is done the representations internal shared_ptr is destroyed, but this should not lead to the object's destruction if you still hold a shared_pointer to it. But: The shared_ptrs obtained from shared_ptr_converter do not count here because there reference count is independent from the owning (the one contained in the internal representation) shared_ptr's one. (This assumes that the object was originally created with a shared_ptr).

I do not know how I could possibly fix this. Sorry. Maybe there will be more insights at luabind#18 or rpavlik#9.

Workarounds

• A analogous handwritten converter for boost::intrusive_pointer should work because the referred object must store the reference count in itself then.
• You could accept raw pointers in your function and derive the referred object from enable_shared_from_this to get a shared_pointer from the given raw pointer.
• Maybe using the behavior shown in a13a7e2 and explained above is enough for you to avoid problems.

EDIT: If you know any other Lua/C++ binding library that is able to cast smart pointers, please let me know: it could be a great help in fixing this.

EDIT2: The LuaBridge documentation also regards this as undoable, a pull request that adds support for std::shared_ptr relies on enable_shared_from_this.

[tripleslash]

You could accept raw pointers in your function and derive the referred object from enable_shared_from_this to get a shared_pointer from the given raw pointer.

Wouldn't this result in a bad_weak_ptr exception?

I really hate intrusive_ptr because you cannot hold weak references / weak references don't know if the object is destroyed.

I guess I will just prevent the lua_State from being closed while any references to lua objects remain in my code (shared_ptr deleters). I shouldn't run into problems then. The only real problem is lua_close...

EDIT:

Wouldn't it be possible to fix the shared_ptr ref count if luabind constructs the shared_ptr from lua->c++ with enable_shared_from_raw?

[Oberon00]

Wouldn't this result in a bad_weak_ptr exception?

If you pass the object by shared_ptr to Lua or register the class to be constructed with shared_ptr, then luabind will hold a shared_ptr to the object, so it should work (luabind::detail::pointer_holder is instantiaded with P=shared_ptr<T>: Only this holder and some additional metadata is allocated inside the lua_State, the C++-Object itself is independent of it, allocated with ordinary new. wrap_base, however, would cause problems, since it holds references to Lua inside the C++ object directly.) . But to be honest, I have never used `enable_shared_from_this myself.

Wouldn't it be possible to fix the shared_ptr ref count if luabind constructs the shared_ptr from lua->c++ with enable_shared_from_raw?

I hear from enable_shared_from_raw for the first time now. It seems undocumented, but from looking into the header the difference to enable_shared_from_this seems to be that it does not require an existing shared_ptr to the object(?). And I suppose one could use dynamic_cast<enable_shared_from_raw*> to find out if a object is derived from it, which does not work with enable_shared_from_this because it is a template.
But since I can find out a compile time if a class requested from a luabind shared_ptr_converter has a shared_from_this method, I don't think that this would be necessary.

[Oberon00]

I have now implemented a fix/workaround (needs more testing and possibly refinement before pushing -- here's a gist: https://gist.github.com/Oberon00/6151412) which is based on shared_ptr_converter:

1. If the shared_ptr requested (from C++) has the exact same type as the one which is present in Lua (if any), the behavior is now the same as with the automatic smart pointer support (i.e. use_count will be increased properly but object identity will not be preserved).
2. If the pointee type of the requested shared_ptr has a shared_from_this member function (checked automatically at compile time), this will be used to obtain a shared_ptr. Caveats: (1) If the object is not already hold in a shared_ptr, behavior is undefined (probably a bad_weak_ptr exception will be thrown); (2) If the enable_shared_from_this() member is not a function with the right prototype (ptr_t enable_shared_from_this() with ptr_t being convertible to the requested boost::shared_ptr<T> type) a compile time error will result).
3. Otherwise, behavior is the same as before this fix.

For caveat (2) of 2.: Avoiding this issue by checking if the type has boost::enable_shared_from_this<T> as a base class is impossible because there is no way to know which template parameter T to choose.

[tripleslash]

I added a workaround. I'm simply not destroying the lua state until every object which has a back reference to lua is destroyed. Thanks for looking into it.

Regarding 2.2: I guess that way it wont work if you got 1 base class which inherits enable_shared_from_this<BaseClass and this base class is inherited by the class hierarchy and upcast with dynamic_pointer_cast<T ?

If not you could eventually force the user to inherit a base class which inherits enable_shared_from_this<BaseClass and check if the type is derived from that base class. This behaviour could be added to wrap_base.

[tripleslash]

So the only real problem at the moment is raw equality right?

[Oberon00]

This raw equality problem (which occurs when passing a pointer of any kind(1) from Lua and then back again or (2) to Lua multiple times) is not specific to smart pointers: To the contrary, a shared_ptr was (or is in case 3 of the fix) the only way to preserve raw equality but even then only for one that originally came from Lua via shared_ptr_converter. Even with raw pointers, raw equality is lost. To fix it, luabind would have to maintain a state-global table with an address->object mapping to always use the same object for the same pointer. However, the only case where this really matters is when using the objects as table indices and when you want to use rawequal for other reasons. The considerable performance hit such a table would incur seems not worth it to me.

The problem still left with shared_ptris that in case 3, one has to keep the Lua state open. Maybe I could provide something like an user queryable use_count (possibly with a callback invoked when it goes to zero) for the whole state to mitigate this problem a bit.

But I really reopened this issue because I think that I should provide upcasting capabilities for more smart pointers, namely intrusive_pointer and other smart pointers where construction of multiple ones from the same raw pointer is not a problem.

[Oberon00]

This issue should now be fixed as far as possible with dd1cb88, 014c3a0, bd99d5a and 39d6e3c.

[tripleslash]

Thank you very much! I'm so glad this is resolved now. I believe shared_ptr support can be considered complete now.
I see what you mean, the problem is that luabind always pushes the pointer as new userdata, right?

[Oberon00]

Yes, it more or less has to, as I said above

To fix it, luabind would have to maintain a state-global table with an address->object mapping to always use the same object for the same pointer.

[rpavlik]

Hoping to get this in my repo if it's reasonably separable, though it might be as a more full merge, instead.