Patch - Fix for cross thread instantation when using wrap base.

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

Patch - Fix for cross thread instantation when using wrap base.

Nigel Atkinson-2
I've been bug squishing.  This one was a bit curly to track down, given
the symptoms.  I've included a test program that breaks without the
patch, and I've tested the fix in a larger program that uses luabind
extensively.

The test program seg-faults when trying to call the method, even with
the thread that made the Lua class still around - which I initially
thought was the problem, but it wasn't.  The bug only seems to show
itself, when using wrap_base and Lua overload-able class methods.

The first patch is some minor cleaning that I noticed while stepping
through the code.

The second is the bug fix.

More explanation in the patch files themselves.

Nigel

PS What's the preferred patch format?  I used git format-patch this
time.  Does that make things easier?


------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
luabind-user mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/luabind-user

main.cpp (1K) Download Attachment
0001-Remove-redundant-code.patch (1K) Download Attachment
0002-Changes-to-inject-backref-to-be-lua-thread-safe.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Patch - Fix for cross thread instantation when using wrap base.

Daniel Wallin
On Wed, Apr 07, 2010 at 04:43:03AM +1200, Nigel Atkinson wrote:

> I've been bug squishing.  This one was a bit curly to track down, given
> the symptoms.  I've included a test program that breaks without the
> patch, and I've tested the fix in a larger program that uses luabind
> extensively.
>
> The test program seg-faults when trying to call the method, even with
> the thread that made the Lua class still around - which I initially
> thought was the problem, but it wasn't.  The bug only seems to show
> itself, when using wrap_base and Lua overload-able class methods.
>
> The first patch is some minor cleaning that I noticed while stepping
> through the code.

Thanks, I've pushed this to the 0.9 branch.

> The second is the bug fix.

Thanks for figuring this out.

I'm wondering if it wouldn't be simpler and more efficient to add a
constructor to weak_ref et al. that takes the lua_State* of the main
thread. That would avoid doing all this stack moving and get rid of the
two extra API calls. Thoughts?

> PS What's the preferred patch format?  I used git format-patch this
> time.  Does that make things easier?

Yeah, it makes things easier because I can just use "git am" and get the
commit message and author correct right away.

--
Daniel Wallin
BoostPro Computing
http://www.boostpro.com

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

Re: Patch - Fix for cross thread instantation when using wrap base.

Nigel Atkinson-2
On Mon, 2010-04-26 at 09:47 +0200, Daniel Wallin wrote:
<snip>
> Thanks for figuring this out.
My pleasure.
>
> I'm wondering if it wouldn't be simpler and more efficient to add a
> constructor to weak_ref et al. that takes the lua_State* of the main
> thread. That would avoid doing all this stack moving and get rid of the
> two extra API calls. Thoughts?

Do you mean that the code should create class instances in the main
thread from the get go, regardless of what thread started the process?  
I originally thought of doing this, but made a bit of a mess while
trying and switched tack (thank you git-branch!!).  I'm still figuring
out how the code works. ;-)

On another note, in the bits of code I wrote, I'm wondering if the
second lua_xmove could be replaced simply by lua_pop.  Is this value
needed on the thread's stack afterwards?

Nigel


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