Re: Garbage collector crashes if luabind objects are created in a lua coroutine

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: Garbage collector crashes if luabind objects are created in a lua coroutine

Thomas Schöps
Hi,
sorry to bug you again, but has some progress been made regarding this problem?
https://sourceforge.net/mailarchive/forum.php?thread_name=167c0fce0902250453qa8232d5ja025f24543ae58d1%40mail.gmail.com&forum_name=luabind-user

If not, I'll have to look for another binding library ...

Thomas

------------------------------------------------------------------------------
_______________________________________________
luabind-user mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/luabind-user
Reply | Threaded
Open this post in threaded view
|

Re: Garbage collector crashes if luabind objects are created in a lua coroutine

James Porter-2
Thomas Schöps wrote:
> Hi,
> sorry to bug you again, but has some progress been made regarding this problem?
> https://sourceforge.net/mailarchive/forum.php?thread_name=167c0fce0902250453qa8232d5ja025f24543ae58d1%40mail.gmail.com&forum_name=luabind-user

It seems to me that the obvious/naive solution is to check if the
current lua_State* is a coroutine in luabind::detail::ref and if so,
luaL_ref the coroutine with the main thread.

The attached version of detail/ref.hpp "seems" to work, but keep in mind
that 1) I don't know much about Lua coroutines, and 2) I'm writing this
through the haze of a pretty bad cold.

- Jim

// Copyright (c) 2003 Daniel Wallin and Arvid Norberg

// Permission is hereby granted, free of charge, to any person obtaining a
// copy of this software and associated documentation files (the "Software"),
// to deal in the Software without restriction, including without limitation
// the rights to use, copy, modify, merge, publish, distribute, sublicense,
// and/or sell copies of the Software, and to permit persons to whom the
// Software is furnished to do so, subject to the following conditions:

// The above copyright notice and this permission notice shall be included
// in all copies or substantial portions of the Software.

// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF
// ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED
// TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A
// PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
// SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR
// ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
// ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE
// OR OTHER DEALINGS IN THE SOFTWARE.


#ifndef LUABIND_REF_HPP_INCLUDED
#define LUABIND_REF_HPP_INCLUDED

#include <cassert>
#include <algorithm>

#include <luabind/config.hpp>
#include <luabind/lua_include.hpp>

struct lua_State;

namespace luabind
{

#if LUA_VERSION_NUM >= 501
#define LUA_REFNIL (-1)
#undef luaL_setn
#undef luaL_getn
#endif

namespace detail
{

        int LUABIND_API ref(lua_State *L);
        void LUABIND_API unref(lua_State *L, int ref);

        inline void getref(lua_State* L, int r)
        {
                lua_rawgeti(L, LUA_REGISTRYINDEX, r);
        }

        struct lua_reference
        {
                lua_reference(lua_State* L_ = 0)
                        : L(L_)
                        , m_ref(LUA_NOREF)
                        , m_coref(LUA_NOREF)
                {}
                lua_reference(lua_reference const& r)
                        : L(r.L)
                        , m_ref(LUA_NOREF)
                        , m_coref(LUA_NOREF)
                {
                        if (!r.is_valid()) return;
                        r.get(L);
                        set(L);
                }
                ~lua_reference() { reset(); }

                lua_State* state() const { return L; }

                void operator=(lua_reference const& r)
                {
                        // TODO: self assignment problems
                        reset();
                        if (!r.is_valid()) return;
                        r.get(r.state());
                        set(r.state());
                }

                bool is_valid() const
                { return m_ref != LUA_NOREF; }

                void set(lua_State* L_)
                {
                        reset();
                        L = L_;
                        m_ref = ref(L);
                        if(lua_pushthread(L) == 1)
                                lua_pop(L,1);
                        else
                                luaL_ref(L, LUA_REGISTRYINDEX);
                }

                void replace(lua_State* L_)
                {
                        lua_rawseti(L_, LUA_REGISTRYINDEX, m_ref);
                }

                // L may not be the same pointer as
                // was used when creating this reference
                // since it may be a thread that shares
                // the same globals table.
                void get(lua_State* L_) const
                {
                        assert(m_ref != LUA_NOREF);
                        assert(L_);
                        getref(L_, m_ref);
                }

                void reset()
                {
                        if (L && m_ref != LUA_NOREF)
                        {
                                unref(L, m_ref);
                                if(m_coref != LUA_NOREF)
                                {
                                        if(lua_pushthread(L) == 1)
                                                lua_pop(L,1);
                                        else
                                                luaL_unref(L, LUA_REGISTRYINDEX, m_coref);
                                }
                        }
                        m_ref = LUA_NOREF;
                }

                void swap(lua_reference& r)
                {
                        assert(r.L == L);
                        std::swap(r.m_ref, m_ref);
                }

        private:
                lua_State* L;
                int m_ref;
                int m_coref;
        };

}}

#endif // LUABIND_REF_HPP_INCLUDED


------------------------------------------------------------------------------

_______________________________________________
luabind-user mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/luabind-user
Reply | Threaded
Open this post in threaded view
|

Re: Garbage collector crashes if luabind objects are created in a lua coroutine

Thomas Schöps
In reply to this post by Thomas Schöps
Hi Jim,

thank you very much for your reply.

Your code keeps it from crashing (testing with the example program
from http://www.gamedev.net/community/forums/topic.asp?topic_id=525030
), but it's using more and more memory as it runs.
So I made two changes to your ref.hpp:
- add std::swap(r.m_coref, m_coref); to lua_reference::swap()   (don't
know what it's used for, but looks like it should be there)
- change luaL_ref(L, LUA_REGISTRYINDEX); in lua_reference::set() to
m_coref = luaL_ref(L, LUA_REGISTRYINDEX)   (otherwise, m_coref is not
set)

Unfortunately, the second change resulted in the program crashing
again unless calls to the garbage collector are added into the while
(true) loop of the example program, which leads to the desired
behaviour (no crashes, no memory leaks). But this can't be the
solution ...

Any ideas on what I'm doing wrong?

------------------------------------------------------------------------------
_______________________________________________
luabind-user mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/luabind-user
Reply | Threaded
Open this post in threaded view
|

Re: Garbage collector crashes if luabind objects are created in a lua coroutine

James Porter-2
Thomas Schöps wrote:
> Any ideas on what I'm doing wrong?

Having run it through a debugger, it seems to crash sometimes even with
my original code, so it's probably nothing you did. Looking back into
the mailing list archives, it appears there's a bug with ref/unref in
Luabind:

http://sourceforge.net/mailarchive/forum.php?thread_name=497847BC.30108%40front.ru&forum_name=luabind-user

By removing the calls to that and replacing them with
luaL_ref/luaL_unref, everything seems to work on my end. Attached is an
updated version of the changes. It's not release-quality, since I should
really remove ref/unref from the source entirely (as well as basically
everything else in ref.cpp), but if the patch works then we should be
one step closer to resolving the issue.

- Jim

// Copyright (c) 2003 Daniel Wallin and Arvid Norberg

// Permission is hereby granted, free of charge, to any person obtaining a
// copy of this software and associated documentation files (the "Software"),
// to deal in the Software without restriction, including without limitation
// the rights to use, copy, modify, merge, publish, distribute, sublicense,
// and/or sell copies of the Software, and to permit persons to whom the
// Software is furnished to do so, subject to the following conditions:

// The above copyright notice and this permission notice shall be included
// in all copies or substantial portions of the Software.

// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF
// ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED
// TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A
// PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
// SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR
// ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
// ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE
// OR OTHER DEALINGS IN THE SOFTWARE.


#ifndef LUABIND_REF_HPP_INCLUDED
#define LUABIND_REF_HPP_INCLUDED

#include <cassert>
#include <algorithm>

#include <luabind/config.hpp>
#include <luabind/lua_include.hpp>

struct lua_State;

namespace luabind
{

#if LUA_VERSION_NUM >= 501
#define LUA_REFNIL (-1)
#undef luaL_setn
#undef luaL_getn
#endif

namespace detail
{

        int LUABIND_API ref(lua_State *L);
        void LUABIND_API unref(lua_State *L, int ref);

        inline void getref(lua_State* L, int r)
        {
                lua_rawgeti(L, LUA_REGISTRYINDEX, r);
        }

        struct lua_reference
        {
                lua_reference(lua_State* L_ = 0)
                        : L(L_)
                        , m_ref(LUA_NOREF)
                        , m_coref(LUA_NOREF)
                {}
                lua_reference(lua_reference const& r)
                        : L(r.L)
                        , m_ref(LUA_NOREF)
                        , m_coref(LUA_NOREF)
                {
                        if (!r.is_valid()) return;
                        r.get(L);
                        set(L);
                }
                ~lua_reference() { reset(); }

                lua_State* state() const { return L; }

                void operator=(lua_reference const& r)
                {
                        // TODO: self assignment problems
                        reset();
                        if (!r.is_valid()) return;
                        r.get(r.state());
                        set(r.state());
                }

                bool is_valid() const
                { return m_ref != LUA_NOREF; }

                void set(lua_State* L_)
                {
                        reset();
                        L = L_;
                        if (lua_pushthread(L) == 1)
                                lua_pop(L, 1);
                        else
                                m_coref = luaL_ref(L, LUA_REGISTRYINDEX);
                        m_ref = luaL_ref(L, LUA_REGISTRYINDEX);
                }

                void replace(lua_State* L_)
                {
                        lua_rawseti(L_, LUA_REGISTRYINDEX, m_ref);
                }

                // L may not be the same pointer as
                // was used when creating this reference
                // since it may be a thread that shares
                // the same globals table.
                void get(lua_State* L_) const
                {
                        assert(m_ref != LUA_NOREF);
                        assert(L_);
                        getref(L_, m_ref);
                }

                void reset()
                {
                        if (L && m_ref != LUA_NOREF)
                                luaL_unref(L, LUA_REGISTRYINDEX, m_ref);

                        if (L && m_coref != LUA_NOREF)
                        {
                                if (lua_pushthread(L) == 1)
                                        lua_pop(L,1);
                                else
                                        luaL_unref(L, LUA_REGISTRYINDEX, m_coref);
                        }

                        m_ref = LUA_NOREF;
                        m_coref = LUA_NOREF;
                }

                void swap(lua_reference& r)
                {
                        assert(r.L == L);
                        std::swap(r.m_ref, m_ref);
                        std::swap(r.m_coref, m_coref);
                }

        private:
                lua_State* L;
                int m_ref;
                int m_coref;
        };

}}

#endif // LUABIND_REF_HPP_INCLUDED


------------------------------------------------------------------------------

_______________________________________________
luabind-user mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/luabind-user
Reply | Threaded
Open this post in threaded view
|

Re: Garbage collector crashes if luabind objects are created in a lua coroutine

James Porter-2
In reply to this post by Thomas Schöps
Oops! There was a bug in the version I just uploaded (it unnecessarily
pushes the current thread onto the stack when unref'ing). Attached is
the (hopefully) correct version.

- Jim

// Copyright (c) 2003 Daniel Wallin and Arvid Norberg

// Permission is hereby granted, free of charge, to any person obtaining a
// copy of this software and associated documentation files (the "Software"),
// to deal in the Software without restriction, including without limitation
// the rights to use, copy, modify, merge, publish, distribute, sublicense,
// and/or sell copies of the Software, and to permit persons to whom the
// Software is furnished to do so, subject to the following conditions:

// The above copyright notice and this permission notice shall be included
// in all copies or substantial portions of the Software.

// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF
// ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED
// TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A
// PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
// SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR
// ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
// ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE
// OR OTHER DEALINGS IN THE SOFTWARE.


#ifndef LUABIND_REF_HPP_INCLUDED
#define LUABIND_REF_HPP_INCLUDED

#include <cassert>
#include <algorithm>

#include <luabind/config.hpp>
#include <luabind/lua_include.hpp>

struct lua_State;

namespace luabind
{

#if LUA_VERSION_NUM >= 501
#define LUA_REFNIL (-1)
#undef luaL_setn
#undef luaL_getn
#endif

namespace detail
{

        int LUABIND_API ref(lua_State *L);
        void LUABIND_API unref(lua_State *L, int ref);

        inline void getref(lua_State* L, int r)
        {
                lua_rawgeti(L, LUA_REGISTRYINDEX, r);
        }

        struct lua_reference
        {
                lua_reference(lua_State* L_ = 0)
                        : L(L_)
                        , m_ref(LUA_NOREF)
                        , m_coref(LUA_NOREF)
                {}
                lua_reference(lua_reference const& r)
                        : L(r.L)
                        , m_ref(LUA_NOREF)
                        , m_coref(LUA_NOREF)
                {
                        if (!r.is_valid()) return;
                        r.get(L);
                        set(L);
                }
                ~lua_reference() { reset(); }

                lua_State* state() const { return L; }

                void operator=(lua_reference const& r)
                {
                        // TODO: self assignment problems
                        reset();
                        if (!r.is_valid()) return;
                        r.get(r.state());
                        set(r.state());
                }

                bool is_valid() const
                { return m_ref != LUA_NOREF; }

                void set(lua_State* L_)
                {
                        reset();
                        L = L_;
                        if (lua_pushthread(L) == 1)
                                lua_pop(L, 1);
                        else
                                m_coref = luaL_ref(L, LUA_REGISTRYINDEX);
                        m_ref = luaL_ref(L, LUA_REGISTRYINDEX);
                }

                void replace(lua_State* L_)
                {
                        lua_rawseti(L_, LUA_REGISTRYINDEX, m_ref);
                }

                // L may not be the same pointer as
                // was used when creating this reference
                // since it may be a thread that shares
                // the same globals table.
                void get(lua_State* L_) const
                {
                        assert(m_ref != LUA_NOREF);
                        assert(L_);
                        getref(L_, m_ref);
                }

                void reset()
                {
                        if (L && m_ref != LUA_NOREF)
                                luaL_unref(L, LUA_REGISTRYINDEX, m_ref);

                        if (L && m_coref != LUA_NOREF)
                                luaL_unref(L, LUA_REGISTRYINDEX, m_coref);

                        m_ref = LUA_NOREF;
                        m_coref = LUA_NOREF;
                }

                void swap(lua_reference& r)
                {
                        assert(r.L == L);
                        std::swap(r.m_ref, m_ref);
                        std::swap(r.m_coref, m_coref);
                }

        private:
                lua_State* L;
                int m_ref;
                int m_coref;
        };

}}

#endif // LUABIND_REF_HPP_INCLUDED


------------------------------------------------------------------------------

_______________________________________________
luabind-user mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/luabind-user
Reply | Threaded
Open this post in threaded view
|

Re: Garbage collector crashes if luabind objects are created in a lua coroutine

Thomas Schöps
In reply to this post by Thomas Schöps
The latest version seems to work fine. Thank you very much!

I'd suggest a new luabind bugfix release when this fix is ready
because it's really important for everyone wanting to use coroutines
with luabind.

------------------------------------------------------------------------------
_______________________________________________
luabind-user mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/luabind-user