Re: [C++-sig] Bug and patch for boost.python with enable_shared_from_this
No news that I'm aware of. I'd be happy to check in patches, but someone else has to take the lead in communicating with Peter, and updating the unit tests. Ralf ----- Original Message ---- From: Edson Tadeu <e.tadeu@gmail.com> To: Development of Python/C++ integration <c++-sig@python.org> Sent: Thursday, February 28, 2008 11:45:32 AM Subject: Re: [C++-sig] Bug and patch for boost.python with enable_shared_from_this Any news on this issue? Will this patch get into Boost 1.35? Thanks, Edson On Wed, Feb 13, 2008 at 5:27 PM, Ralf W. Grosse-Kunstleve <rwgk@yahoo.com> wrote: Hi Chad, Could you at least ask Peter Dimov if he thinks the dont_enable_shared_from_this patch could be generally useful? Maybe he's OK adding it, then it wouldn't be a big deal fixing Boost.Python, which would probably prevent a lot of confusion and lost time in the future. http://www.boost.org/people/peter_dimov.htm p-d-i-m-o-v-at-m-m-l-t-d-dot-n-e-t Ralf ----- Original Message ---- From: Chad Austin <chad@imvu.com> To: Development of Python/C++ integration <c++-sig@python.org> Sent: Monday, February 11, 2008 10:10:31 PM Subject: Re: [C++-sig] Bug and patch for boost.python with enable_shared_from_this There probably is a way to prevent boost.python from creating shared_ptrs with a null deleter like it does, but it would require far more knowledge than I have. :) I agree that it would be a better fix, though. On 2/11/08, Ralf W. Grosse-Kunstleve <rwgk@yahoo.com> wrote:
Thanks for the patch! I'd love to check this in for you, but for the change in shared_ptr.hpp you'll need Peter Dimov's approval, and the shared_ptr documentation would have to be updated. Also, your reproducer should be turned into a unit test (in boost/libs/python/test).
Is there any chance that the shared_ptr patch could somehow be avoided?
Ralf
----- Original Message ---- From: Chad Austin <chad@imvu.com> To: c++-sig@python.org Sent: Sunday, February 10, 2008 5:14:45 PM Subject: [C++-sig] Bug and patch for boost.python with enable_shared_from_this
Hi all,
A month ago or so we discovered a bug in Boost.Python that manifests when using enable_shared_from_this. Here is a test case that demonstrates the bug:
...
-----Inline Attachment Follows----- _______________________________________________ C++-sig mailing list C++-sig@python.org http://mail.python.org/mailman/listinfo/c++-sig
Hi,
Could you at least ask Peter Dimov if he thinks the dont_enable_shared_from_this patch could be generally useful? Maybe he's OK adding it, then it wouldn't be a big deal fixing Boost.Python, which would probably prevent a lot of confusion and lost time in the future.
I stumbled once again on this problem, so I took some time to bother Peter Dimov with the patch suggestion. Here, is a copy of our exchange : Hello Peter, I wanted to contact you about a problem that was raised, on february, on c++-sig mailing list about a problem happening when wrapping classes derived from 'enable_shared_from_this' using boost.python library. The problem is briefly described in this post - including a patch to boost/shared_ptr.hpp that would allow to fix the problem. http://mail.python.org/pipermail/cplusplus-sig/2008-February/012973.html The original poster was encouraged by Ralf W. Grosse-Kunstleve to contact you about this, but we never heard from him on the list (http://mail.python.org/pipermail/cplusplus-sig/2008-February/013003.html). To make it short, boost.python creates shared_ptr objects, holding the wrapped c++ objects, with a custom deleter managing the python object reference count. This leads to something like that : #include <boost/enable_shared_from_this.hpp> namespace { class A : public boost::enable_shared_from_this<A> { public: ~A() {}; }; void my_deleter(void*) { } }; BOOST_AUTO_TEST_CASE( test_enable_shared_from_this ) { boost::shared_ptr<A> a( new A ); { boost::shared_ptr<A> ater = boost::shared_ptr<A>( a.get(), my_deleter ); // OP patch proposal :: boost::shared_ptr<A> ater = boost::shared_ptr<A>( a.get(), my_deleter // , boost::dont_enable_shared_from_this() ); BOOST_CHECK( a == ater ); } boost::shared_ptr<A> abug = a->shared_from_this(); // this throws bad_weak_ptr } Did you hear about this problem ? Do you think this patch is viable and could be applied in boost trunk ? Do you happen to have another idea of workaround that could be applied in client code and/or in shared_ptr ? Thanks for your time, best regards, Nicolas Lelong mgdesign Followed by Peter's answer that lead to Trac ticket #2584 : Yes, I am aware of this problem. It is possible today to emulate the proposed functionality by using the aliasing constructor, but I think that a more proper fix for the issue is to make enable_shared_from_this more intelligent so that it doesn't reinitialize itself in such scenarios. I'll appreciate it if you file a Trac ticket for this and assign it to me. I've a few pending enable_shared_from_this issues anyway, and I'll try to find the time to address them before the next release ships. -- Peter Dimov http://www.pdplayer.com
FWIW, it seems I can't reproduce the problem with current boost svn trunk code, the problem still existed in boost_1_37_0 though. I could not spot the changes made that fixed the problem. I took a look at the shared_ptr aliasing constructor, and made a patch to my boost_1_37_0 that solves the problem without altering shared_ptr code. All boost.python tests still pass after that. It may be useful to someone (the test case from Chad is attached) --- python_1_37\converter\shared_ptr_from_python.hpp 2008-12-12 15:49:04.156250000 +0100 +++ python\converter\shared_ptr_from_python.hpp 2008-12-12 14:24:24.781250000 +0100 @@ -45,10 +45,14 @@ if (data->convertible == source) new (storage) shared_ptr<T>(); else + { + boost::shared_ptr<void> hold_convertible_ref_count( (void*)0, shared_ptr_deleter(handle<>(borrowed(source))) ); + // use aliasing constructor new (storage) shared_ptr<T>( - static_cast<T*>(data->convertible), - shared_ptr_deleter(handle<>(borrowed(source))) + hold_convertible_ref_count, + static_cast<T*>(data->convertible) ); + } data->convertible = storage; } --- to include the test in python test pass : Index: libs/python/test/Jamfile.v2 =================================================================== --- libs/python/test/Jamfile.v2 (revision 72) +++ libs/python/test/Jamfile.v2 (working copy) @@ -75,6 +75,7 @@ [ bpl-test return_arg ] [ bpl-test staticmethod ] [ bpl-test shared_ptr ] +[ bpl-test enable_shared_from_this ] [ bpl-test andreas_beyer ] [ bpl-test polymorphism ] [ bpl-test polymorphism2 ]
Thanks for the patches! They are now in the boost trunk: ------------------------------------------------------------------------ r50368 | rwgk | 2008-12-22 23:55:33 -0800 (Mon, 22 Dec 2008) | 4 lines Boost.Python enable_shared_from_this patches by Nicolas Lelong and Chad Austin: http://mail.python.org/pipermail/cplusplus-sig/2008-December/014103.html http://mail.python.org/pipermail/cplusplus-sig/2008-February/013003.html ------------------------------------------------------------------------
on Mon Dec 22 2008, "Ralf W. Grosse-Kunstleve" <rwgk-AT-yahoo.com> wrote:
Thanks for the patches! They are now in the boost trunk:
------------------------------------------------------------------------ r50368 | rwgk | 2008-12-22 23:55:33 -0800 (Mon, 22 Dec 2008) | 4 lines
Boost.Python enable_shared_from_this patches by Nicolas Lelong and Chad Austin: http://mail.python.org/pipermail/cplusplus-sig/2008-December/014103.html http://mail.python.org/pipermail/cplusplus-sig/2008-February/013003.html
FYI, Ralf, trunk changes don't get merged automatically to release, so we now need a mechanism to make sure changes we want released are actually released. -- Dave Abrahams BoostPro Computing http://www.boostpro.com
Oh, that's new to me. Thanks! Is there a web page describing the release procedure? (A quick search didn't get me to the right place.) ----- Original Message ---- From: David Abrahams <dave@boostpro.com> To: Development of Python/C++ integration <cplusplus-sig@python.org> Sent: Tuesday, December 23, 2008 1:10:33 AM Subject: Re: [C++-sig] Bug and patch for boost.pythonwithenable_shared_from_this on Mon Dec 22 2008, "Ralf W. Grosse-Kunstleve" <rwgk-AT-yahoo.com> wrote:
Thanks for the patches! They are now in the boost trunk:
------------------------------------------------------------------------ r50368 | rwgk | 2008-12-22 23:55:33 -0800 (Mon, 22 Dec 2008) | 4 lines
Boost.Python enable_shared_from_this patches by Nicolas Lelong and Chad Austin: http://mail.python.org/pipermail/cplusplus-sig/2008-December/014103.html http://mail.python.org/pipermail/cplusplus-sig/2008-February/013003.html
FYI, Ralf, trunk changes don't get merged automatically to release, so we now need a mechanism to make sure changes we want released are actually released. -- Dave Abrahams BoostPro Computing http://www.boostpro.com _______________________________________________ Cplusplus-sig mailing list Cplusplus-sig@python.org http://mail.python.org/mailman/listinfo/cplusplus-sig
participants (3)
-
David Abrahams -
Nicolas Lelong -
Ralf W. Grosse-Kunstleve