Is this intrusive pointer implementation correct? I tried to use the CRTP Pattern to avoid a virtual destructor in my class ReferenceCounting which provides the reference counter.

Is this efficient?

The following things I could not solve yet:

How can I make a conversion between IntrusionPtr<A> to IntrusionPtr<B> as long as A* is implicitly convertable to B*? (That's how boost does it somehow)

What are the most important things we should prohibit and things we can allow to do with an IntrusionPtr? I am quite unsure about this.

#include <iostream>
#include <type_traits>
using namespace std;


    template<typename TDerived>
    class ReferenceCounting {
    public:
        typedef TDerived Derived ;

        ReferenceCounting() : m_ref(0) { }
        ~ReferenceCounting() {std::cout <<" RC:DTOR" <<std::endl; }

        unsigned long int addRef() const{
            ++m_ref;
            std::cout << "RC::addRef: " << m_ref <<  std::endl;
            return m_ref;
        }

        // NoDelete is for IntrusivePtr<T>().release()!
        template<bool Delete = true>
        void release() const{
            --m_ref;
            std::cout << "RC::release: " <<  m_ref << std::endl;
            if(!m_ref && Delete){
                std::cout << "RC::delete" << std::endl;
                delete static_cast<Derived const *>(this);
            } 
        }
        unsigned long int getRefCount() const{return m_ref;}

    private:
        mutable unsigned long int m_ref; // Mutable to be changeable also for const objects!
    };


    template<typename T>
    class IntrusivePtr {
    public:

        using NonConstType = typename std::remove_const<T>::type;

        IntrusivePtr() : m_p(nullptr) { }

        // Explicit constructor from T* , because we want to avoid that this constructor can be used to convert implicitly to IntrusivePtr
        // somewhere in the code which then deletes the resource unexpectetly!
        explicit IntrusivePtr(T* p) : m_p(p) {
            if(p) static_cast<const ReferenceCounting<NonConstType> *>(m_p)->addRef();
        }

        IntrusivePtr(const IntrusivePtr & rhs) : m_p(rhs.m_p) {
            if(m_p) static_cast<const ReferenceCounting<NonConstType> *>(m_p)->addRef();
        }


        ~IntrusivePtr() {
            if(m_p) static_cast<const ReferenceCounting<NonConstType> *>(m_p)->release();
        }


        // We want to assign the intrusive ptr to this class
        // m_p points to A, rhs->m_p  points to B
        // This means, decrease ref count of current object A, an set m_p=rhs->m_p 
        // and increase ref count of rhs resource. This can by:
        // Copy and swap idiom, call by value to copy the IntrusivePtr (ref count of B increments)
        // swap this resource pointer into the local temporary rhs (only pointer swap) 
        // deleting the local rhs at end of function decrements ref count of initial resource A 
        IntrusivePtr& operator=(IntrusivePtr rhs) {
            rhs.swap(*this); // swaps the resource pointers
            return *this; // delete rhs-> which decrements correctly our initial resource A!
        }

        // Reset the IntrusivePtr to some other resource B, 
        // meaning decrementing our resource A and setting the new pointer to B
        // and incrementing B
        // Can also take a nullptr!, making it not default argument because avoiding syntax mistakes with release()
        // which does a complete different thing (takes off the IntrusivePtr)
        void reset(T* p) {
            // Make temporary intrusive pointer for *p (incrementing ref count B)
            // swapping pointers with our resource A, and deleting temporary, decrement A
            IntrusivePtr(p).swap(*this);
        }

        // Release a IntrusivePtr from managing the shared resource
        // Decrements ref count of this resource A but without deleting it!
        T* release() {
            static_cast<const ReferenceCounting<NonConstType> *>(m_p)->template release<false>();
            T* p = m_p;
            m_p = nullptr;
            return p;
        }

        // Get the underlying pointer   
        T* get() const { return m_p; }

        // Implicit cast to T*
        operator T*() const { return m_p; } 
        // Implicit cast to T&
        operator T&() const { return *m_p; }

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

        void swap(IntrusivePtr& rhs) {
            std::swap(m_p, rhs.m_p);
        }
    private:
        T* m_p;
    };


    class A : public ReferenceCounting<A>{
        public:

        typedef ReferenceCounting<A> RCBaseType;
        A(){}
        A(A&c){std::cout << "A::copy construct" << std::endl;}
        A& operator=(A&c){std::cout << "A::copy assign" << std::endl;}
        ~A(){
            std::cout << "A::DTOR: " <<this <<  std::endl;  
        }

        int foo(){return i[10000];}

        int i[10001];
    };

    int main(){
        {
        A *a = new A();
        IntrusivePtr<A> p (a);
        std::cout << "Create Local " <<p<< std::endl;
        {
            std::cout << "Create IntrPtr" << std::endl;         
                IntrusivePtr<A> p2(p);
                // Copy object
                A c(p2);
                std::cout << "Ref count of copied obj:" << c.getRefCount() << std::endl;

                A b = *a;
                std::cout << "Ref count of copied obj:" << b.getRefCount() << std::endl;
            {
                std::cout << "Create IntrPtr2 from;" << p.get()<< std::endl;            
                IntrusivePtr<A const> p3(p);
                std::cout << "p3 points to " << p3.get() << " with ref count: " << p3->getRefCount()<< std::endl;
                //p1->i[0]=10;
                //p1.release(); p2.release();  // does not delete object
                const A* r =p3.release();
                std::cout << "Ref count of copied obj:" << r->getRefCount() << std::endl;
            }
        }
        }
    }

The code can be found here.

The original old code can still be found here.

  • added some new code which is simpler and with some usefull comment to understand the assumptions, and what the functions should actually do – Gabriel Jun 14 '14 at 16:24
up vote 4 down vote accepted

The copy and swap idiom is easier to write when pass by value.

IntrusivePtr& operator=(IntrusivePtr rhs)   // Use pass by value to generate copy.
{
    rhs.swap(*this);
    return *this;
}

Doing it this way also gives the compiler better opportunities for optimizations.

I am not sure I would do release like that.

T* release() {
    T* p = m_p;
    m_p = 0;
    return p;
}

You now have a pointer to an object with the ref-count incremented to +1. If you now put the pointer back into an intrusive pointer the count will increase to +2 and when that goes out of scope it is back to +1 and will not be deleted.

Personally I think you should still decrement that reference count, but not delete the object if it reaches zero. You still have to remember to manually delete the object when you are done. This also mimics the behavior of the other types of smart pointer.

In addition to the cast to the pointer.

operator T*() const { return m_p; }

How about a cast to the T as well:

operator T&() const { return *m_p; }
  • Very good comment about the release()! And the Copy Swap!! – Gabriel Jun 14 '14 at 14:17
  • @Gabriel: Note there is a good analysis and recomendations for "Copy and Swap Idium" on SO under the c++-faq tag What is the copy-and-swap idiom? – Martin York Jun 14 '14 at 15:28
  • Is it possible to use the CRTP Pattern with Multiple INheritance: Say class A: public Bwhere B inherits from ReferenceCounting – Gabriel Jun 15 '14 at 9:50
  • @Gabriel: You will have to ask a specific question on SO. There is not enough room to answer in the comments. But suffice to say probably its a pattern and thus can be implemented in many ways. – Martin York Jun 15 '14 at 17:49
  • ok I ask a question: – Gabriel Jun 15 '14 at 20:44

Your Answer

 
discard

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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