5
\$\begingroup\$

So I am pretty sure the CppCoreGuidelines say that you should disregard standard libraries and write your own. Not sure I remember where, probably P.13 or somewhere around there...

Anyway here is my implementation:

#ifndef SMART_POINTER_HEADER
#define SMART_POINTER_HEADER

#include <cstddef>
#include "mutex.hpp"

namespace my_experimental
{

struct ref_count_data
{
    unsigned int weak_count;
    unsigned int use_count;
};

class reference_count
{
    public:
        reference_count()
        {
            ref_counts.weak_count = 1;
            ref_counts.use_count = 0;
        }

        void add_shared_ref()
        {
            lock_guard< mutex > lock( ref_count_mutex );
            ref_counts.use_count++;
        }

        ref_count_data release_shared_ref()
        {
            lock_guard< mutex > lock( ref_count_mutex );
            ref_counts.use_count--;

            if( !ref_counts.use_count )
            {
                ref_counts.weak_count--;
            }

            return ref_counts;
        }

        void add_weak_ref()
        {
            lock_guard< mutex > lock( ref_count_mutex );
            ref_counts.weak_count++;
        }

        ref_count_data release_weak_ref()
        {
            lock_guard< mutex > lock( ref_count_mutex );
            ref_counts.weak_count--;
            return ref_counts;
        }

        unsigned int get_use_count() const
        {
            lock_guard< mutex > lock( ref_count_mutex );
            return ref_counts.use_count;
        }

    private:
        mutable mutex ref_count_mutex;
        ref_count_data ref_counts;
};

template < typename T > class shared_pointer
{
    public:
        shared_pointer() : pdata( NULL ), rc( NULL ) {} //default constructor

        shared_pointer( T* pvalue ) : pdata( pvalue ), rc( NULL )
        {
            rc = new reference_count();
            rc->add_shared_ref();
        }

        //Copy constructor for dynamic_pointer_cast and weak_pointer lock
        shared_pointer( T* pvalue_arg, reference_count * rc_arg ) : pdata( pvalue_arg ), rc( rc_arg )
        {
            if ( NULL == pdata )
            {
                rc = NULL;
            }
            else
            {
                rc->add_shared_ref();
            }
        }

        //Copy constructor
        shared_pointer( const shared_pointer<T>& sp ) : pdata( sp.pdata ), rc( sp.rc )
        {
            if ( NULL != rc )
            {
                rc->add_shared_ref();
            }
        }

        ~shared_pointer()
        {
            if ( NULL != rc )
            {
                ref_count_data updated_counts = rc->release_shared_ref();

                if ( !updated_counts.use_count )
                {
                    delete pdata;
                }

                if( !updated_counts.weak_count )
                {
                    delete rc;
                }
            }
        }

        T& operator* () const
        {
            return *pdata;
        }

        T* operator-> () const
        {
            return pdata;
        }

        shared_pointer<T>& operator = ( const shared_pointer<T>& sp )
        {
            if ( this != &sp ) // Avoid self assignment
            {
                if ( NULL != rc )
                {
                    ref_count_data updated_counts = rc->release_shared_ref();

                    if ( !updated_counts.use_count )
                    {
                        delete pdata;
                    }

                    if( !updated_counts.weak_count )
                    {
                        delete rc;
                    }
                }

                pdata = sp.pdata;
                rc = sp.rc;

                if ( NULL != rc )
                {
                    rc->add_shared_ref();
                }
            }

            return *this;
        }

        bool operator !() const
        {
            return !pdata;
        }

        bool operator == ( const shared_pointer<T>& other ) const
        {
            return ( pdata == other.pdata );
        }

        bool operator == ( void * other ) const
        {
            return ( pdata == other );
        }

        bool operator != ( const shared_pointer<T>& other ) const
        {
            return ( pdata != other.pdata );
        }

        bool operator != ( void * other ) const
        {
            return ( pdata != other );
        }

        template <class Y, class Z> friend shared_pointer<Y> dynamic_pointer_cast( const shared_pointer<Z>& sp );
        template < class Y > friend class weak_pointer;

    private:
        T* pdata;            // pointer
        reference_count* rc; // Reference count
};

template < typename T > class weak_pointer
{
    public:
        weak_pointer() : pdata( NULL ), rc( NULL ) {} //default constructor

        weak_pointer( const weak_pointer<T>& wp ) : pdata( wp.pdata ), rc( wp.rc )
        {
            if ( NULL != rc )
            {
                rc->add_weak_ref();
            }
        }

        weak_pointer( const shared_pointer<T>& sp ) : pdata( sp.pdata ), rc( sp.rc )
        {
            if ( NULL != rc )
            {
                rc->add_weak_ref();
            }
        }

        ~weak_pointer()
        {
            if ( NULL != rc )
            {
                ref_count_data updated_counts = rc->release_weak_ref();

                if( !updated_counts.weak_count )
                {
                    delete rc;
                }
            }
        }

        weak_pointer<T>& operator = ( const weak_pointer<T>& wp )
        {
            if ( this != &wp ) // Avoid self assignment
            {
                if ( NULL != rc )
                {
                    ref_count_data updated_counts = rc->release_weak_ref();

                    if( !updated_counts.weak_count )
                    {
                        delete rc;
                    }
                }

                pdata = wp.pdata;
                rc = wp.rc;

                if ( NULL != rc )
                {
                    rc->add_weak_ref();
                }
            }

            return *this;
        }

        bool expired() const
        {
            if( NULL == rc )
            {
                return true;
            }
            else if( 0 == rc->get_use_count() )
            {
                return true;
            }

            return false;
        }

        int use_count() const
        {
            if( NULL == rc )
            {
                return 0;
            }
            else
            {
                return rc->get_use_count();
            }
        }

        shared_pointer<T> lock() const
        {
            if( expired() )
            {
                return shared_pointer<T>( NULL );
            }
            else
            {
                return shared_pointer<T>( pdata, rc );
            }
        }

    private:
        T* pdata;       // pointer
        reference_count* rc; // Reference count
};


template <class T, class U> shared_pointer<T> dynamic_pointer_cast( const shared_pointer<U>& sp )
{
    return shared_pointer<T>( dynamic_cast<T*>( sp.pdata ), sp.rc );
}
}//namespace my_experimental

#endif

And here is the contents of mutex.hpp in case you want to compile this code (I use gcc in Linux but should be easy to adapt):

#ifndef MUTEX_HEADER
#define MUTEX_HEADER

#include <pthread.h>

namespace my_experimental
{

class mutex
{
    public:
        mutex() { pthread_mutex_init( &pthread_mutex, NULL ); }
        ~mutex() { pthread_mutex_destroy( &pthread_mutex ); }
        int lock() { return pthread_mutex_lock( &pthread_mutex ); }
        int try_lock() { return pthread_mutex_trylock( &pthread_mutex ); }
        int unlock() { return pthread_mutex_unlock( &pthread_mutex ); }

    private:
        pthread_mutex_t pthread_mutex;

        //disallow
        mutex( const mutex& );
        mutex& operator=( mutex const& );
};

template< class Mutex > class lock_guard
{
    public:
        lock_guard( Mutex& mutex_to_lock ) : locked_mutex( mutex_to_lock )
        {
            locked_mutex.lock();
        }

        ~lock_guard() { locked_mutex.unlock(); }

    private:
        Mutex& locked_mutex;

        //disallow
        lock_guard();
        lock_guard( const lock_guard& );
        lock_guard& operator=( lock_guard const& );
};

} //namespace my_experimental

#endif

Seems to work fairly well. I do wonder if there is any way to better handle the synchronization of the reference_count object.

Before I added support for weak pointer I used __sync_add_and_fetch and __sync_sub_and_fetch to make the ref count atomic, however once I introduced multiple ref counts that was no longer sufficient to ensure synchronization.

\$\endgroup\$
6
\$\begingroup\$

Nothing much to say about mutex. Simple wrapper, straight and to the point. Very neatly written.

Now to what I consider glaring issues

  1. reference_count::release_shared_ref() is touching weak_count. Calling add_shared_ref doesn't increment weak_count, so neither should release_shared_ref decrement it. It seems like a manufactured problem for a mutex.

  2. You have a data race in weak_ptr::lock(). It's all in the branch here.

    if( expired() )
    {
        return shared_pointer<T>( NULL );
    }
    else
    {
        return shared_pointer<T>( pdata, rc );
    }
    

    Even if expired() is done atomically, there can be a context switch right as the else branch is entered. During that context switch, pdata and rc may be destroyed as the last shared pointer dies. So you return a shared_ptr which you initialized from two invalid addresses.

\$\endgroup\$
  • \$\begingroup\$ 1. I am deriving this methodology from boost. In boost the weak count is always the number of weak pointer + ( use_count != 0 ). Maybe I should adjust the function names (I am open to suggestions). \$\endgroup\$ – Nathan Owen Jan 31 '18 at 17:46
  • \$\begingroup\$ 2. I agree this is a race condition and I have been pondering how best to fix it. I could lock the reference mutex directly from the weak pointer lock function , a bit dirty but should work. Have any better ideas? \$\endgroup\$ – Nathan Owen Jan 31 '18 at 17:48
  • \$\begingroup\$ @NathanOwen - That maybe for boost, but I don't see the major need for it in your own implementation. I could have missed something, so IDK. Regarding the data race, why not just query rc about the proper pointers? Then initialize the return of lock from those queried values. \$\endgroup\$ – StoryTeller - Unslander Monica Feb 1 '18 at 5:58
  • \$\begingroup\$ I think it may originally have been to allow for atomic logic, not sure. I suppose I don't really need to do that, but the logic is slightly simpler, at least in the case of week pointer (arguably less clear though). I would perhaps also argue that it is nice to have the lifetime of pdata controlled solely by use_count and the lifetime of rc being controlled solely by weak_count. Come to think about it, I might be able to get away with using synchronous increment/decrement builtin atomic operators using this logic. I'll ponder it some more in the morning. \$\endgroup\$ – Nathan Owen Feb 1 '18 at 6:39
  • \$\begingroup\$ ...As for storing the pdata pointer in rc, I belive that might introduce a fair amount of overhead. Since the '->' operator is going to be, by far, the most frequently called function (for typical programs) that operation should have as few instructions as possible. I could store a copy of the pdata pointer in rc though... Will need to give it some more thought. \$\endgroup\$ – Nathan Owen Feb 1 '18 at 6:40
0
\$\begingroup\$

I have edited my solution based on StoryTeller's feedback. I don't love this implementation, since it relies on public data members, but I believe it is thread safe now, and that is more important.

#ifndef SMART_POINTER_HEADER
#define SMART_POINTER_HEADER

#include <cstddef>
#include "mutex.hpp"

namespace my_experimental
{

class reference_count
{
    public:
        unsigned int weak_count;
        unsigned int use_count;
        mutable mutex ref_count_mutex;

        reference_count() : weak_count( 1 ), use_count( 0 ) {}
};

template < typename T > class shared_pointer
{
    public:
        shared_pointer() : pdata( NULL ), rc( NULL ) {} //default constructor

        shared_pointer( T* pvalue ) : pdata( pvalue ), rc( NULL )
        {
            if( NULL != pvalue )
            {
                rc = new reference_count();
                increment_reference_count();
            }
        }

        //Copy constructor
        shared_pointer( const shared_pointer<T>& sp ) : pdata( sp.pdata ), rc( sp.rc )
        {
            increment_reference_count();
        }

        ~shared_pointer()
        {
            decrement_reference_count_and_delete_if_needed();
        }

        T& operator* () const
        {
            return *pdata;
        }

        T* operator-> () const
        {
            return pdata;
        }

        shared_pointer<T>& operator = ( const shared_pointer<T>& sp )
        {
            if ( this != &sp ) // Avoid self assignment
            {
                decrement_reference_count_and_delete_if_needed();

                pdata = sp.pdata;
                rc = sp.rc;

                increment_reference_count();
            }

            return *this;
        }

        bool operator !() const
        {
            return !pdata;
        }

        bool operator == ( const shared_pointer<T>& other ) const
        {
            return ( pdata == other.pdata );
        }

        bool operator == ( void * other ) const
        {
            return ( pdata == other );
        }

        bool operator != ( const shared_pointer<T>& other ) const
        {
            return ( pdata != other.pdata );
        }

        bool operator != ( void * other ) const
        {
            return ( pdata != other );
        }

        template <class Y, class Z> friend shared_pointer<Y> dynamic_pointer_cast( const shared_pointer<Z>& sp );
        template < class Y > friend class weak_pointer;

    private:
        T* pdata;            // pointer
        reference_count* rc; // Reference count

        //Copy constructor for dynamic pointer casting
        shared_pointer( T* pvalue_arg, reference_count * rc_arg ) : pdata( pvalue_arg ), rc( rc_arg )
        {
            if ( NULL == pdata )
            {
                rc = NULL; //just set to NULL
            }
            else
            {
                increment_reference_count();
            }
        }

        void increment_reference_count()
        {
            if ( NULL != rc )
            {
                lock_guard< mutex > rc_lock( rc->ref_count_mutex );
                rc->use_count++;
            }
        }

        void decrement_reference_count_and_delete_if_needed()
        {
            if ( NULL != rc )
            {
                bool delete_pdata = false;
                bool delete_rc = false;

                { //lock
                    lock_guard< mutex > rc_lock( rc->ref_count_mutex );

                    rc->use_count--;

                    if ( 0 == rc->use_count )
                    {
                        rc->weak_count--;
                        delete_pdata = true;
                    }

                    if( 0 == rc->weak_count )
                    {
                        delete_rc = true;
                    }
                } //unlock

                if( delete_pdata )
                {
                    //best not to call unknown code from locked context so delete here
                    //for example the destructor could end up doing something with a
                    //weak_pointer to pdata (that would be poorly written code but we know nothing
                    //about the destructor we are calling so best not to call from locked context)
                    delete pdata;
                }

                if( delete_rc )
                {
                    //rc contains the mutex that we were locking so we must delete outside of locked context
                    //this is safe as we know that no one else holds a reference to rc once we get here
                    delete rc;
                }
            }
        }
};

template < typename T > class weak_pointer
{
    public:
        weak_pointer() : pdata( NULL ), rc( NULL ) {} //default constructor

        weak_pointer( const weak_pointer<T>& wp ) : pdata( wp.pdata ), rc( wp.rc )
        {
            increment_reference_count();
        }

        weak_pointer( const shared_pointer<T>& sp ) : pdata( sp.pdata ), rc( sp.rc )
        {
            increment_reference_count();
        }

        ~weak_pointer()
        {
            decrement_reference_count_and_delete_if_needed();
        }

        weak_pointer<T>& operator = ( const weak_pointer<T>& wp )
        {
            if ( this != &wp ) // Avoid self assignment
            {
                decrement_reference_count_and_delete_if_needed();

                pdata = wp.pdata;
                rc = wp.rc;

                increment_reference_count();
            }

            return *this;
        }

        bool expired() const
        {
            if( NULL == rc )
            {
                return true;
            }
            else
            {
                lock_guard< mutex > rc_lock( rc->ref_count_mutex );

                if( 0 == rc->use_count )
                {
                    return true;
                }
            }

            return false;
        }

        int use_count() const
        {
            if( NULL == rc )
            {
                return 0;
            }
            else
            {
                lock_guard< mutex > rc_lock( rc->ref_count_mutex );
                return rc->use_count;
            }
        }

        shared_pointer<T> lock() const
        {
            if( NULL == rc )
            {
                return shared_pointer<T>( NULL );
            }
            else
            {
                //create an empty shared pointer and manually assign values
                //once inside locked context.  We cannot return from within
                //locked context as the return will call the copy constructor,
                //which will in turn lock the ref_count_mutex resulting in a
                //deadlock if we are still in locked context
                shared_pointer<T> new_sp;

                {//lock
                    lock_guard< mutex > rc_lock( rc->ref_count_mutex );

                    if( 0 == rc->use_count )
                    {
                        return new_sp;
                    }

                    new_sp.rc = rc;
                    new_sp.pdata = pdata;

                    rc->use_count++;
                }//unlock

                return new_sp;
            }
        }

    private:
        T* pdata;            // pointer
        reference_count* rc; // Reference count

        void increment_reference_count()
        {
            if ( NULL != rc )
            {
                lock_guard< mutex > rc_lock( rc->ref_count_mutex );
                rc->weak_count++;
            }
        }

        void decrement_reference_count_and_delete_if_needed()
        {
            if ( NULL != rc )
            {
                bool delete_rc = false;

                { //lock
                    lock_guard< mutex > rc_lock( rc->ref_count_mutex );

                    rc->weak_count--;

                    if( 0 == rc->weak_count )
                    {
                        delete_rc = true;
                    }
                } //unlock

                if( delete_rc )
                {
                    //rc contains the mutex that we were locking so we must delete outside of locked context
                    //this is safe as we know that no one else holds a reference to rc once we get here
                    delete rc;
                }
            }
        }
};


template <class T, class U> shared_pointer<T> dynamic_pointer_cast( const shared_pointer<U>& sp )
{
    return shared_pointer<T>( dynamic_cast<T*>( sp.pdata ), sp.rc );
}

}//namespace my_experimental

#endif

I look forward to your feedback :-)

\$\endgroup\$
0
\$\begingroup\$

Here is yet another variant based on StoryTeller's feedback. This variant removes the public data members which better protects the locking mechanisms. It also makes the use_count and weak_count independent, unlike boost's implementation. Not sure how it compares performance-wise to my previous implementation, will need to run some benchmarks and look into the resulting assembly code.

#ifndef SMART_POINTER_HEADER
#define SMART_POINTER_HEADER

#include <cstddef>
#include "mutex.hpp"

namespace my_experimental
{

struct ref_count_data
{
    unsigned int weak_count;
    unsigned int use_count;
};

class reference_count
{
    public:
        reference_count()
        {
            ref_counts.weak_count = 0;
            ref_counts.use_count = 1;
        }

        void * get_shared_ref( void * pdata )
        {
            lock_guard< mutex > lock( ref_count_mutex );

            if ( ref_counts.use_count )
            {
                ref_counts.use_count++;
                return pdata;
            }
            else
            {
                return NULL;
            }
        }

        ref_count_data release_shared_ref()
        {
            lock_guard< mutex > lock( ref_count_mutex );
            ref_counts.use_count--;
            return ref_counts;
        }

        void add_weak_ref()
        {
            lock_guard< mutex > lock( ref_count_mutex );
            ref_counts.weak_count++;
        }

        ref_count_data release_weak_ref()
        {
            lock_guard< mutex > lock( ref_count_mutex );
            ref_counts.weak_count--;
            return ref_counts;
        }

        unsigned int get_use_count() const
        {
            lock_guard< mutex > lock( ref_count_mutex );
            return ref_counts.use_count;
        }

    private:
        mutable mutex ref_count_mutex;
        ref_count_data ref_counts;
};

template < typename T > class shared_pointer
{
    public:
        shared_pointer() : pdata( NULL ), rc( NULL ) {} //default constructor

        shared_pointer( T* pvalue ) : pdata( pvalue ), rc( NULL )
        {
            if ( NULL != pdata )
            {
                rc = new reference_count();
            }
        }

        //Copy constructor
        shared_pointer( const shared_pointer<T>& sp ) : pdata( NULL ), rc( sp.rc )
        {
            if ( NULL != rc )
            {
                pdata = static_cast<T*>( rc->get_shared_ref( sp.pdata ) );

                if ( NULL == pdata )
                {
                    rc = NULL;
                }
            }
        }

        ~shared_pointer()
        {
            if ( NULL != rc )
            {
                ref_count_data updated_counts = rc->release_shared_ref();

                if ( 0 == updated_counts.use_count )
                {
                    delete pdata;

                    if ( 0 == updated_counts.weak_count )
                    {
                        delete rc;
                    }
                }

            }
        }

        T& operator* () const
        {
            return *pdata;
        }

        T* operator-> () const
        {
            return pdata;
        }

        shared_pointer<T>& operator = ( const shared_pointer<T>& sp )
        {
            if ( this != &sp ) // Avoid self assignment
            {
                if ( NULL != rc )
                {
                    ref_count_data updated_counts = rc->release_shared_ref();

                    if ( 0 == updated_counts.use_count )
                    {
                        delete pdata;

                        if ( 0 == updated_counts.weak_count )
                        {
                            delete rc;
                        }
                    }
                }

                pdata = NULL;
                rc = sp.rc;

                if ( NULL != rc )
                {
                    pdata = static_cast<T*>( rc->get_shared_ref( sp.pdata ) );

                    if ( NULL == pdata )
                    {
                        rc = NULL;
                    }
                }
            }

            return *this;
        }

        bool operator !() const
        {
            return !pdata;
        }

        bool operator == ( const shared_pointer<T>& other ) const
        {
            return ( pdata == other.pdata );
        }

        bool operator == ( void * other ) const
        {
            return ( pdata == other );
        }

        bool operator != ( const shared_pointer<T>& other ) const
        {
            return ( pdata != other.pdata );
        }

        bool operator != ( void * other ) const
        {
            return ( pdata != other );
        }

        template <class Y, class Z> friend shared_pointer<Y> dynamic_pointer_cast( const shared_pointer<Z>& sp );
        template < class Y > friend class weak_pointer;

    private:
        T* pdata;            // pointer
        reference_count* rc; // Reference count

        //Copy constructor for dynamic_pointer_cast and weak_pointer lock
        shared_pointer( T* pvalue_arg, reference_count * rc_arg ) : pdata( NULL ), rc( rc_arg )
        {
            if ( NULL == pvalue_arg )
            {
                rc = NULL;
            }
            else
            {
                pdata = static_cast<T*>( rc->get_shared_ref( pvalue_arg ) );

                if ( NULL == pdata )
                {
                    rc = NULL;
                }
            }
        }
};

template < typename T > class weak_pointer
{
    public:
        weak_pointer() : pdata( NULL ), rc( NULL ) {} //default constructor

        weak_pointer( const weak_pointer<T>& wp ) : pdata( wp.pdata ), rc( wp.rc )
        {
            if ( NULL != rc )
            {
                rc->add_weak_ref();
            }
        }

        weak_pointer( const shared_pointer<T>& sp ) : pdata( sp.pdata ), rc( sp.rc )
        {
            if ( NULL != rc )
            {
                rc->add_weak_ref();
            }
        }

        ~weak_pointer()
        {
            if ( NULL != rc )
            {
                ref_count_data updated_counts = rc->release_weak_ref();

                if ( ( 0 == updated_counts.use_count ) && ( 0 == updated_counts.weak_count ) )
                {
                    delete rc;
                }
            }
        }

        weak_pointer<T>& operator = ( const weak_pointer<T>& wp )
        {
            if ( this != &wp ) // Avoid self assignment
            {
                if ( NULL != rc )
                {
                    ref_count_data updated_counts = rc->release_weak_ref();

                    if ( ( 0 == updated_counts.use_count ) && ( 0 == updated_counts.weak_count ) )
                    {
                        delete rc;
                    }
                }

                pdata = wp.pdata;
                rc = wp.rc;

                if ( NULL != rc )
                {
                    rc->add_weak_ref();
                }
            }

            return *this;
        }

        bool expired() const
        {
            if ( NULL == rc )
            {
                return true;
            }
            else if ( 0 == rc->get_use_count() )
            {
                return true;
            }

            return false;
        }

        int use_count() const
        {
            if ( NULL == rc )
            {
                return 0;
            }
            else
            {
                return rc->get_use_count();
            }
        }

        shared_pointer<T> lock() const
        {
            return shared_pointer<T>( pdata, rc ); //this will return a NULL shared_pointer if expired
        }

    private:
        T* pdata;            // pointer
        reference_count* rc; // Reference count
};

template <class T, class U> shared_pointer<T> dynamic_pointer_cast( const shared_pointer<U>& sp )
{
    return shared_pointer<T>( dynamic_cast<T*>( sp.pdata ), sp.rc );
}

}//namespace my_experimental

#endif
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.