Is Poco RefCountedObject thread safe?

Question

Poco RefCountedObject offer 2 interfaces:

    inline void RefCountedObject::duplicate() const
{
    ++_counter;
}


inline void RefCountedObject::release() const throw()
{
    try
    {
        if (--_counter == 0) delete this;
    }
    catch (...)
    {
        poco_unexpected();
    }
}

With:

class Foundation_API RefCountedObject
    /// A base class for objects that employ
    /// reference counting based garbage collection.
    ///
    /// Reference-counted objects inhibit construction
    /// by copying and assignment.
{
public:
    RefCountedObject();
        /// Creates the RefCountedObject.
        /// The initial reference count is one.

    void duplicate() const;
        /// Increments the object's reference count.

    void release() const throw();
        /// Decrements the object's reference count
        /// and deletes the object if the count
        /// reaches zero.

    int referenceCount() const;
        /// Returns the reference count.

protected:
    virtual ~RefCountedObject();
        /// Destroys the RefCountedObject.

private:
    RefCountedObject(const RefCountedObject&);
    RefCountedObject& operator = (const RefCountedObject&);

    mutable AtomicCounter _counter;
};

Note that: mutable AtomicCounter _counter;

my question is if I use a RefCountedObject in multithreading is it safe?

In my opinion it's not because only --_counter is atomic but if(--_count) is not atomic and could lead to having a reference to a deleted object. For example let say I have 2 threads A and B one executing duplicate and the other release, the execution sequence is as follow:

• B start executing release and reach --_counter
• A start executing duplicate and reach ++_counter
• At this point _counter=1
• B execute --_counter and return the result to be evaluated by the if (which is 0 && counter is 0 now)
• B is preempted and stop before the branching statement (the if)
• A execute _counter++ and return a reference to the object
• B continue and evaluate the branching statement with value 0 and delete the object

We end up with A having a reference to a deleted object. even if the mutable keyword force the compiler to not optimize the _counter it won't help in multithreading

Am I missing something?

Answer 1

Strictly from looking at duplicate and release in isolation, the above is correct.

However, if you have two threads, both having a pointer to the same RefCountedObject, you either should have separate AutoPtr instances in each thread (which implies the _counter is > 1 to begin with), or, if you share the same AutoPtr, it must be protected by a mutex, if at least one thread can change the AutoPtr (which would result in release() being called). Needless to say sharing an mutable AutoPtr between multiple threads is a recipe for disaster.

If you manage RefCountedObject by manually calling duplicate() and release() (which I would not recommend), you should follow POCO's reference counting rules and be very careful.

So, for well-behaved code, RefCountedObject together with AutoPtr is safe for multithreaded use.