enable_shared_from_this2

[berserker_r]

After experiencing some troubles with boost::python and
enable_shared_from_this (as already reported in the past by someone to
Peter Dimov), I started looking at the new (not documented) class
enable_shared_from_this2.
I made two changes in init_weak_once and now it seems to support
shared_from_this in constructors and shared pointers "reinitialization".
The original code (from boost 1.40):

void init_weak_once() const
{
    if( weak_this_._empty() )
    {
       shared_this_.reset(static_cast< T* >( 0 ),
detail::esft2_deleter_wrapper());
       weak_this_ = shared_this_;
    }
}

The "patched" code:

void init_weak_once() const
{
    // Reinitialization support
    if( weak_this_.expired() )
    {
       // shared_from_this in costructors support
shared_this_.reset(static_cast
<T*>(const_cast<enable_shared_from_this2>(this)),
detail::esft2_deleter_wrapper());
       weak_this_ = shared_this_;
    }
}

I know that the static_cast/const_cast is "problematic" on VC6 (and
maybe other compilers) but it could be solved using an offset from
enable_shared_from_this2 * to T *.
I've attached a testcase that shows the problems with the current
enable_shared_from_this2 implementation (with the proposed patch it works).
Any feedback about this?

struct null_deleter
{
        template <typename T>
        inline void operator()(T *p) const { }
};

class EnableThisPtrTester : public boost::enable_shared_from_this2<EnableThisPtrTester>
{
public:
        EnableThisPtrTester()
        {

        }

        EnableThisPtrTester(shared_ptr<EnableThisPtrTester> &ptr)
        {
                ptr = shared_from_this();
                BOOST_CHECK(ptr);
        }
};

BOOST_AUTO_TEST_CASE(test_enable_this_ptr)
{
        {
                shared_ptr<EnableThisPtrTester> ptr1;
                shared_ptr<EnableThisPtrTester> ptr2(new EnableThisPtrTester(ptr1));
                BOOST_CHECK(ptr1 == ptr2);
                BOOST_CHECK(ptr1.use_count() == ptr2.use_count());
                BOOST_CHECK(ptr1->shared_from_this() == ptr1);
                BOOST_CHECK(ptr1->shared_from_this().use_count() == ptr1.use_count());
                shared_ptr<EnableThisPtrTester> ptr3 = ptr2->shared_from_this();
                BOOST_CHECK(ptr2 == ptr3);
                BOOST_CHECK(ptr2.use_count() == ptr3.use_count());
                BOOST_CHECK(ptr1->shared_from_this().use_count() == ptr1.use_count());
        }

        {
                shared_ptr<EnableThisPtrTester> ptr1;
                EnableThisPtrTester tester(ptr1);
                shared_ptr<EnableThisPtrTester> ptr2 = tester.shared_from_this();
                BOOST_CHECK(ptr1 == ptr2);
                BOOST_CHECK(ptr1.use_count() == ptr2.use_count());

                ptr1.reset();
                ptr2.reset();
        }

        {
                EnableThisPtrTester tester;

                {
                        shared_ptr<EnableThisPtrTester> ptr1 = tester.shared_from_this();
                        shared_ptr<EnableThisPtrTester> ptr2 = tester.shared_from_this();
                        BOOST_CHECK(ptr1 == ptr2);
                        BOOST_CHECK(ptr1.use_count() == ptr2.use_count());

                        shared_ptr<EnableThisPtrTester> ptr3(&tester, null_deleter());

                        BOOST_CHECK(ptr1 == ptr3);
                        BOOST_CHECK(ptr1.use_count() == ptr3.use_count());

                        shared_ptr<EnableThisPtrTester> ptr4 = tester.shared_from_this();
                        BOOST_CHECK(ptr4 == ptr1);
                        BOOST_CHECK(ptr4 == ptr3);

                        BOOST_CHECK(ptr1.use_count() == ptr4.use_count());
                        BOOST_CHECK(ptr3.use_count() == ptr4.use_count());
                }

                BOOST_CHECK(tester.shared_from_this());
        }
}
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Frank Mori Hess]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Monday 26 October 2009, Berserker wrote:

> The "patched" code:
>
> void init_weak_once() const
> {
>     // Reinitialization support
>     if( weak_this_.expired() )
>     {
>        // shared_from_this in costructors support
> shared_this_.reset(static_cast
> <T*>(const_cast<enable_shared_from_this2>(this)),
> detail::esft2_deleter_wrapper());
>        weak_this_ = shared_this_;
>     }
> }
>
> I know that the static_cast/const_cast is "problematic" on VC6 (and
> maybe other compilers) but it could be solved using an offset from
> enable_shared_from_this2 * to T *.
> I've attached a testcase that shows the problems with the current
> enable_shared_from_this2 implementation (with the proposed patch it works).
> Any feedback about this?

IIRC, the original version of enable_shared_from_this that supported use in
constructors did a static_cast to the derived type.  But it failed some of
the tests, so it was changed to a dynamic_cast.  The version currently in svn
looks broken, like it was from a later version which relied on a
shared_from_this() free function like:

template<typename T>
shared_ptr<T> shared_from_this(T *);

which used shared_ptr aliasing to create and return a shared_ptr which pointed
at the same object as was passed in as an argument (so
enable_shared_from_this didn't have to do a cast or store a pointer to the
derived type).  But I don't see any such free function in
enable_shared_from_this2.hpp.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iEYEARECAAYFAkrnL7QACgkQ5vihyNWuA4XFAACfZR2MYD0G5yVKwykKTq+BGwU1
Zm8An0ChaK+AZX/Pp3CRVKzWJKRS2JjL
=Qxri
-----END PGP SIGNATURE-----
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Peter Dimov-5]

Frank Mori Hess wrote:

> The version currently in svn looks broken, ...

It passes esft_constructor_test, which you wrote. :-)
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Frank Mori Hess]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Tuesday 27 October 2009, Peter Dimov wrote:
> Frank Mori Hess wrote:
> > The version currently in svn looks broken, ...
>
> It passes esft_constructor_test, which you wrote. :-)

I can fix that :)  It looks like I only did tests of ownership, I never tested
the actual pointer value that was returned.  I think I can fix
enable_shared_from_this2.hpp tommorrow, would it be okay for me to commit
changes to it to trunk?  I'd like to go the path of a non-template
enable_shared_from_this2 with a free shared_from_this() function.  The other
option would be adding a vtable to enable_shared_from_this2 and using
dynamic_cast (which I think I'm now remembering was required to make the
virtual inheritance stuff in shared_from_this_test.cpp pass).

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iEYEARECAAYFAkrnTWoACgkQ5vihyNWuA4XwrwCeOJXvd5onLA1yUZICgkgvj/8z
QLYAoNzbrowruGFlcmnTgfOL/4AWSQZp
=r36I
-----END PGP SIGNATURE-----
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Peter Dimov-5]

Frank Mori Hess wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On Tuesday 27 October 2009, Peter Dimov wrote:
>> Frank Mori Hess wrote:
>>> The version currently in svn looks broken, ...
>>
>> It passes esft_constructor_test, which you wrote. :-)
>
> I can fix that :)  It looks like I only did tests of ownership, I
> never tested the actual pointer value that was returned.  I think I
> can fix enable_shared_from_this2.hpp tommorrow, would it be okay for
> me to commit changes to it to trunk?

Before fixing it, can you please show me a test case that fails?
shared_from_this_test.cpp also seems to work when switched to esft2 (a test
that I should have added, but didn't, for some reason). Do you see anything
wrong?

The missing #include <boost/weak_ptr.hpp> doesn't count. ;-)

FWIW, I do not object in principle if you decide to

> go the path of a non-template enable_shared_from_this2 with a free
> shared_from_this() function;

it's just that the current esft2 was supposed to be a faithful
reconstruction of your constructor-enabled esft (minus the get_deleter
special case to keep shared_ptr unaware of it).

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Peter Dimov-5]

Peter Dimov wrote:

> Before fixing it, can you please show me a test case that fails?

I get it. I haven't read Berserker's tests carefully (sorry), and our tests
do indeed not cover this case. Either ~enable_shared_from_this2 should be
made virtual and (T*)0 be made a dynamic_cast<T*>(this) as in the original,
or shared_from_this should be a free function, as you suggest. (How about
'shared_from_raw' as a name for the free function?)

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Frank Mori Hess]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Tuesday 27 October 2009, Peter Dimov wrote:
> (How about
> 'shared_from_raw' as a name for the free function?)

That's fine by me.  I also like shared_from_pop (pop == plain old pointer)
although that might be more obscure.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iEYEARECAAYFAkrnjngACgkQ5vihyNWuA4UJ1wCg6J7pWMLOCrwlkbzaR+P+pQWX
LRgAoNeX+dDtgUxzZylXakxqbtvyW9jq
=lW5c
-----END PGP SIGNATURE-----
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Peter Dimov-5]

Frank Mori Hess wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On Tuesday 27 October 2009, Peter Dimov wrote:
>> (How about
>> 'shared_from_raw' as a name for the free function?)
>
> That's fine by me.  I also like shared_from_pop (pop == plain old
> pointer) although that might be more obscure.

OK, to get back to your earlier question, I consider you the 'owner' of
enable_shared_from_this2, so feel free to fix it whichever way you want. :-)

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[berserker_r]

> I get it. I haven't read Berserker's tests carefully (sorry), and our
> tests do indeed not cover this case. Either ~enable_shared_from_this2
> should be made virtual and (T*)0 be made a dynamic_cast<T*>(this) as in
> the original, or shared_from_this should be a free function, as you
> suggest. (How about 'shared_from_raw' as a name for the free function?)

Thanks for yours replies, I hope that you'll find the time to have a
look at my test :)
Peter did you remember the problem related to boost::python I'm talking
about? Here is the link of your reply
http://lists.boost.org/boost-users/2008/08/38864.php
I'm experiencing that problem and the only way I can solve it is using
the patch in my first message.

I'll "suggest" two new methods in enable_shared_from_this2 too:

template <typename I>
inline shared_ptr<I> shared_from_this()
{
    return boost::dynamic_pointer_cast<I>(shared_from_this());
}

template <typename I>
inline shared_ptr<I const> shared_from_this() const
{
    return boost::dynamic_pointer_cast<I const>(shared_from_this());
}

This is for sure nothing special, but I think it's very useful in case
of overriding from base classes which inherit from
enable_shared_from_this2, for example:

class A : public enable_shared_from_this2<A>
{

};

class B : public A
{
    void foo1()
    {
       // short version :)
       shared_ptr<B> me = shared_from_this<B>();
    }

    void foo2()
    {
       // verbose version :(
       shared_ptr<B> me =
boost::dynamic_pointer_cast<B>(shared_from_this());
    }
};

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Peter Dimov-5]

Berserker wrote:

> Thanks for yours replies, I hope that you'll find the time to have a
> look at my test :)
> Peter did you remember the problem related to boost::python I'm
> talking about? Here is the link of your reply
> http://lists.boost.org/boost-users/2008/08/38864.php

I remember now. But I thought I had fixed it. In this message I say:

> We've been thinking of changing the behavior of enable_shared_from_this in
> such cases, making it ignore subsequent shared_ptr instances to the same
> object and having it stick to the first one. This, however, will not
> happen for the upcoming 1.36 release of Boost.
>
> You can in principle achieve the same result by patching your local copy
> of Boost to make weak_ptr::_internal_assign only initialize when
> this->expired() is true, but this has never been tested.

and, in fact, the code in the trunk and in the release branch already does
this:

    // Note: invoked automatically by shared_ptr; do not call
    template<class X, class Y> void _internal_accept_owner( shared_ptr<X>
const * ppx, Y * py ) const
    {
        if( weak_this_.expired() )
        {
            weak_this_ = shared_ptr<T>( *ppx, py );
        }
    }

There is even a test for that, esft_second_ptr_test.cpp:

    boost::shared_ptr<X> px( new X );

    {
        boost::shared_ptr<X> px2( px.get(), null_deleter );
        BOOST_TEST( px == px2 );
    }

    try
    {
        boost::shared_ptr< X > qx = px->shared_from_this();

        BOOST_TEST( px == qx );
        BOOST_TEST( !( px < qx ) && !( qx < px ) );
    }
    catch( boost::bad_weak_ptr const& )
    {
        BOOST_ERROR( "px->shared_from_this() failed" );
    }

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[berserker_r]

> I remember now. But I thought I had fixed it. In this message I say:
> and, in fact, the code in the trunk and in the release branch already
> does this:

That's right but please have a look at my test example. You can comment
out the constructor stuffs and try to switch from
enable_shared_from_this and enable_shared_from_this2: there are other
cases where it fails (shared_ptrs reinitialization in particular).

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Peter Dimov-5]

Berserker wrote:
>> I remember now. But I thought I had fixed it. In this message I say:
>> and, in fact, the code in the trunk and in the release branch already
>> does this:
>
> That's right but please have a look at my test example. You can comment
> out the constructor stuffs and try to switch from enable_shared_from_this
> and enable_shared_from_this2: there are other cases where it fails
> (shared_ptrs reinitialization in particular).

All of your tests rely on the constructor stuff... if by reinitialization
you mean something like

#include <boost/enable_shared_from_this.hpp>
#include <boost/shared_ptr.hpp>
#include <boost/detail/lightweight_test.hpp>

//

class X: public boost::enable_shared_from_this<X>
{
};

void null_deleter( void const* )
{
}

void test( X & x )
{
    boost::shared_ptr<X> px( &x, null_deleter );

    try
    {
        boost::shared_ptr< X > qx = px->shared_from_this();

        BOOST_TEST( px == qx );
        BOOST_TEST( !( px < qx ) && !( qx < px ) );
    }
    catch( boost::bad_weak_ptr const& )
    {
        BOOST_ERROR( "px->shared_from_this() failed" );
    }
}

int main()
{
    X x;

    test( x );
    test( x );

    return boost::report_errors();
}

then it passes.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Gottlob Frege]

On Tue, Oct 27, 2009 at 8:42 PM, Peter Dimov <pdimov@...> wrote:

> Frank Mori Hess wrote:
>>
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> On Tuesday 27 October 2009, Peter Dimov wrote:
>>>
>>> (How about
>>> 'shared_from_raw' as a name for the free function?)
>>
>> That's fine by me.  I also like shared_from_pop (pop == plain old
>> pointer) although that might be more obscure.
>
> OK, to get back to your earlier question, I consider you the 'owner' of
> enable_shared_from_this2, so feel free to fix it whichever way you want. :-)

Fix it "whatever way you want" :-), but call it _raw, not _pop -- as a
user, I'd only understand the first one.

Tony
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[berserker_r]

The expirity check seems working fine to me if merged in
enable_shared_ptr2 and solves some problems.
I can summarize other boost::python's problems in this example:

class X : public boost::enable_shared_from_this2<X>
{
public:
    void test()
    {
       shared_ptr<X> me = shared_from_this();
       BOOST_ASSERT(me);
    }
};

void expose_x()
{
    boost::python::class_<X>("X", boost::python::init<>()).
       def("test", &X::test);
}

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

C++ case 1 (works fine):

shared_ptr<X> x(new X);
x->test();

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

C++ case 2 (doesn't work because of the missing static/dynamic cast):

X x;
x.test();

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

Python case (doesn't work, same problem of C++ case 2)

x = X()
x.test()

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Frank Mori Hess-2]

On Tuesday 27 October 2009, Peter Dimov wrote:
> OK, to get back to your earlier question, I consider you the 'owner' of
> enable_shared_from_this2, so feel free to fix it whichever way you want.
> :-)

I've commited boost/smart_ptr/enabled_shared_from_raw.hpp to trunk (I svn
moved enable_shared_from_this2.hpp).  It provides a (non-template)
boost::enabled_shared_from_raw base class, and a free function template
boost::shared_from_raw().  It supports usage from constructors and
re-initialization after shared_ptr expiration.  I'll try to add a html
page for it to the documentation this weekend.



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Peter Dimov-5]

Frank Mori Hess wrote:
> I've commited boost/smart_ptr/enabled_shared_from_raw.hpp to trunk (I svn
> moved enable_shared_from_this2.hpp).

So long as we're innovating, consider adding weak_from_raw as well. It can
give you an expired weak_ptr to 'this' from within a destructor, which is
useful for deregistration.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Emil Dotchevski-3]

On Wed, Oct 28, 2009 at 6:45 PM, Peter Dimov <pdimov@...> wrote:
> Frank Mori Hess wrote:
>>
>> I've commited boost/smart_ptr/enabled_shared_from_raw.hpp to trunk (I svn
>> moved enable_shared_from_this2.hpp).
>
> So long as we're innovating, consider adding weak_from_raw as well. It can
> give you an expired weak_ptr to 'this' from within a destructor, which is
> useful for deregistration.

Or you can use a factory function for registration and a deleter for
deregistration. That way the object registry is not coupled with T.

Emil Dotchevski
Reverge Studios, Inc.
http://www.revergestudios.com/reblog/index.php?n=ReCode
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Frank Mori Hess-2]

On Wednesday 28 October 2009, Peter Dimov wrote:
> Frank Mori Hess wrote:
> > I've commited boost/smart_ptr/enabled_shared_from_raw.hpp to trunk (I
> > svn moved enable_shared_from_this2.hpp).
>
> So long as we're innovating, consider adding weak_from_raw as well. It
> can give you an expired weak_ptr to 'this' from within a destructor,
> which is useful for deregistration.
>

I think that would require an aliasing constructor for weak_ptr, or friend
access to weak_ptr.  Perhaps a modification of
weak_ptr::_internal_assign(), which no longer seems to be used?



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Peter Dimov-5]

Frank Mori Hess wrote:
> > So long as we're innovating, consider adding weak_from_raw as well. It
> > can give you an expired weak_ptr to 'this' from within a destructor,
> > which is useful for deregistration.
>
> I think that would require an aliasing constructor for weak_ptr, or friend
> access to weak_ptr.

Yes, you're right, I was thinking in terms of the old and busted
enable_shared_from_this.

> Perhaps a modification of weak_ptr::_internal_assign(), which no longer
> seems to be used?

Feel free to remove its second argument. :-)

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[berserker_r]

News about this problem?
I just checked the latest svn revision and it seems that the code below
doesn't work yet.



Berserker wrote:

> The expirity check seems working fine to me if merged in
> enable_shared_ptr2 and solves some problems.
> I can summarize other boost::python's problems in this example:
>
> class X : public boost::enable_shared_from_this2<X>
> {
> public:
>    void test()
>    {
>       shared_ptr<X> me = shared_from_this();
>       BOOST_ASSERT(me);
>    }
> };
>
> void expose_x()
> {
>    boost::python::class_<X>("X", boost::python::init<>()).
>       def("test", &X::test);
> }
>
> ------------------------------------
>
> C++ case 1 (works fine):
>
> shared_ptr<X> x(new X);
> x->test();
>
> ------------------------------------
>
> C++ case 2 (doesn't work because of the missing static/dynamic cast):
>
> X x;
> x.test();
>
> ------------------------------------
>
> Python case (doesn't work, same problem of C++ case 2)
>
> x = X()
> x.test()
>
> _______________________________________________
> Unsubscribe & other changes:
> http://lists.boost.org/mailman/listinfo.cgi/boost
>

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost