Thread: Interlocked refcounting and possible crash

Hi there,

my problem is related to thread-safe reference counting. I know this has been asked thousands of times as I did some google search. But there is a problem which I couldn't find on the internet and which I can't solve without critical sections (?). I noticed this might happen not only in my code as I looked at others' implementations.

I have my own library with smart pointers (I know about boost, but I want to use my own), shared_ptr and instrusive_ptr which work pretty well.
Here is the important part of my shared_ptr:

Code:

template <class base>
class shared_ptr {
private:
    typedef base* P_BASE;
    typedef base& R_BASE;
    /*
        Block with ref count and a pointer, self-explanatory
    */
    class block {
    private:
        P_BASE m_ptr;
        REF_COUNT m_refcount;
    // constructor & destructor
    public:
        block(P_BASE ptr)
            : m_ptr(ptr), m_refcount(1)
        {
        }
        ~block()
        {
            if (m_ptr != NULL)
            {
                delete m_ptr;
            }
        }
    // inline methods
    public:
        inline P_BASE get_ptr() const
        {
            return m_ptr;
        }
        inline bool add_ref()
        {
            return InterlockedIncrement(&m_refcount) > 1; //return win32::interlocked_add_ref(m_refcount);
        }
        inline bool release()
        {
            return InterlockedDecrement(&m_refcount) == 0; //return win32::interlocked_release(m_refcount);
        }
    };
    typedef block* P_BLOCK;
// fields
private:
    P_BLOCK m_block;
// methods
public:
    void reset(P_BASE ptr)
    {
        if (m_block != NULL)            // if block doesnt exists
        {
            if (m_block->release())     // decrement ref count, returns true if reached 0
            {
                delete m_block;         // deletes block
            }
            m_block = NULL;
        }
        if (ptr != NULL)                // creates new block, not important
        {
            m_block = new block(ptr);
        }
    }
// operators
public:
    shared_ptr& operator = (const shared_ptr& a)
    {
        reset(NULL);
        if (a.m_block != NULL)          // (IMPORTANT #1)
        {                               // ... HERE ANOTHER THREAD MIGHT HAVE CALLED delete m_block
                                        // (for example, by calling: reset(NULL) on 'a' object
            if (a.m_block->add_ref())   // (IMPORTANT #2) CRASH ??
            {
                // ... if successfully added reference (adding 1 to 0 means it was being destructed)
                m_block = a.m_block;
            }
        }
        return *this;
    }
};

It works in similar way to boost::shared_ptr, except it has a pointer to PTR - REFCOUNT block instread of two pointers (because each shared_ptr instance occupies less memory, it's important to me).

And now:

STARTUP:

shared_ptr<int> a;
a = new int(100); // ref count is 1 now (a.m_block->m_refcount == 1)

THREAD 1:

shared_ptr<int> b = a; // attemping to increment ref count first

THREAD 2 (executing at the same time)

a = NULL;


and now:
* THREAD 1 has been suspended after executing line (IMPORTANT #1)
* THREAD 2 has successfully called release() and deleted the block
* THREAD 1 is trying to execute line (IMPORTANT #2) but a.m_block points to an already deleted block

I've been considering the following:

* make delete only block->ptr instead of whole block, but it requires additional ref count (problem wouldn't disappear?)

* make static critical section (in the template, so for each type it will be a seperate one), problem appears when there is a lot of instances or when one shared_ptr destructor calls other (for example, in deleting trees)

* EDIT: yet another idea to make threaded_ptr with critical section and use it as a main node, I think it's the best solution


Note:

There is a win32 wrapper class (win32:, because this code is going to be cross-platform and used across various compilers, so I can't use inline assembly and I would really like to make it LOCK-FREE.

Any ideas?

Thanks in advance