I implemented vroad's weak references in AngelScript 2.27.0 WIP already, as mentioned here. Until now I was pleased with being able to add another popular user request, but then it struck me that the implemention is far from thread safe.

Unless I can figure out how to make the solution thread safe I think I'll have to remove this feature again. As I know there are some very bright people reading this forum I'm hoping you'll be able to help me in thinking of a solution to this.

The first problem is how objects that will support the weak references can provide shared boolean in a thread safe manner and guarantee that the object isn't destroyed by release() while another thread using the weakref to add a strong reference.

The following is a simple class, that I think solves this, but I'm still evaluating it:

class MyClass
{
public:
    MyClass() { refCount = 1; weakRefFlag = 0; }
    ~MyClass()
    {
        if( weakRefFlag )
        {
            weakRefFlag->Set(true);
            weakRefFlag->Release();
        }
    }
    void AddRef() { refCount++; }
    void Release() 
    {
        // If the weak ref flag exists it is because someone held a weak ref
        // and that someone may add a reference to the object at any time
        if( weakRefFlag && refCount == 1 )
        {
            // Set the flag to tell others that the object is no longer alive
            // We must do this before decreasing the refCount to 0 so we don't
            // end up with a race condition between this thread attempting to 
            // destroy the object and the other that temporary added a strong
            // ref from the weak ref.
            weakRefFlag->Set(true);
        }
 
        if( --refCount == 0 ) 
            delete this; 
    }
    asISharedBool *GetWeakRefFlag()
    {
        if( !weakRefFlag )
        {
            // Lock globally so no other thread can attempt
            // to create a shared bool at the same time
            asAcquireExclusiveLock();
 
            // Make sure another thread didn't create the flag while we waited for the lock
            if( !weakRefFlag )
                weakRefFlag = asCreateSharedBool();
 
            asReleaseExclusiveLock();
        }
 
        return weakRefFlag;
    }
 
    static MyClass *Factory() { return new MyClass(); }
 
protected:
    int refCount;
    asISharedBool *weakRefFlag;
};

The second problem is with the weakref container itself. How can I make sure that the method Get() is thread safe, so that object isn't destroyed by the owning thread between the check to the 'is alive' flag and the addref() call.

// AngelScript: used as '@obj = ref.get();'
void *CScriptWeakRef::Get() const
{
    // If we hold a null handle, then just return null
    if( m_ref == 0 )
        return 0;
    
    // TODO: This is not threadsafe. Another thread may release the object
    //       between the verification of the sharedbool and the addref
    if( m_weakRefFlag && !m_weakRefFlag->Get() )
    {
        m_type->GetEngine()->AddRefScriptObject(m_ref, m_type->GetSubType());
        return m_ref;
    }
 
    return 0;
}

I still haven't found a proper solution to this second problem. If anyone knows a proper solution, or have any suggestions I'd be happy to hear them.

Maybe if I add a lockable critical section in the shared boolean itself? That way the weakref container could lock on that while checking the flag and adding the reference, and the owning object could also lock on it while release a reference.

Yes, with a lockable critical section in the shared boolean I now believe the solution is thread-safe. Here's what the test class above now looks like (note, I didn't bother making the refCount itself thread-safe, but that is easily done with atomic instructions):

class MyClass
{
public:
    MyClass() { refCount = 1; weakRefFlag = 0; }
    ~MyClass()
    {
        if( weakRefFlag )
            weakRefFlag->Release();
    }
    void AddRef() { refCount++; }
    void Release() 
    {
        // If the weak ref flag exists it is because someone held a weak ref
        // and that someone may add a reference to the object at any time. It
        // is ok to check the existance of the weakRefFlag without locking here
        // because if the refCount is 1 then no other thread is currently 
        // creating the weakRefFlag.
        if( refCount == 1 && weakRefFlag )
        {
            // Set the flag to tell others that the object is no longer alive
            // We must do this before decreasing the refCount to 0 so we don't
            // end up with a race condition between this thread attempting to 
            // destroy the object and the other that temporary added a strong
            // ref from the weak ref.
            weakRefFlag->Set(true);
        }
 
        if( --refCount == 0 ) 
            delete this; 
    }
    asILockableSharedBool *GetWeakRefFlag()
    {
        if( !weakRefFlag )
        {
            // Lock globally so no other thread can attempt
            // to create a shared bool at the same time
            asAcquireExclusiveLock();
 
            // Make sure another thread didn't create the 
            // flag while we waited for the lock
            if( !weakRefFlag )
                weakRefFlag = asCreateLockableSharedBool();
 
            asReleaseExclusiveLock();
        }
 
        return weakRefFlag;
    }
 
    static MyClass *Factory() { return new MyClass(); }
 
protected:
    int refCount;
    asILockableSharedBool *weakRefFlag;
};

The implementation of the weakref add-on can be seen in the SVN.

If anyone can think of why this wouldn't be thread-safe, please let me know so I can fix it before releasing version 2.27.0. If you can think of a better solution I would also very much like to see that.