[openbeos-midi] Re: BMidiEndPoint

Michael Pfeiffer wrote:

> 10.10.2002 10:44:06, "Marcus Overhagen" <ml@xxxxxxxxxxxx> wrote:
> >Michael Pfeiffer wrote:
> >
> >> For an implementation with atomic=3D5Fadd see "class Object" in
> >> BeUtils.h in directory current/src/servers/print:
> >> http://cvs.sourceforge.net/cgi-bin/viewcvs.cgi/open-beos/current/src/servers/print/BeUtils.h=3D3Frev=3D3D1.2&content-type=3D3Dtext/vnd.viewcvs-markup
> >
> 
> >That implementation is NOT correct. You do a non-atomic read of the 
> >fRefCount variable.
> 
> It is correct with regard to the comment about MT-safeness included in the 
> header file.

I still think it is not thread save. can you please prove why do you think it 
should be save=3F
The comment says:

// thread-safe as long as thread that calls acquire has already a reference to 
the object
        void Acquire() { 
                atomic=5Fadd(&fRefCount, 1); 
        } 

        bool Release() { 
                atomic=5Fadd(&fRefCount, -1); 
                if (fRefCount =3D=3D 0) { 
                        delete this; return true; 
                } else { 
                        return false; 
                } 
        }

If you have only one single thread that is calling aquire or release, you don't 
need atomic=5Fadd() at all.

Assuming one thread calls Acquire(), and passes the pointer to another thread, 
which calles release.

If you have two or more threads, the following can happen:

fRefCount is 1

Thread 1 calls Acquire, fRefCount is now 2
Thread 2 calls Release, and gets interrupted after atomic=5Fadd, but before 
if(), fRefCount is now 1
Thread 1 calls Release, (does not get interrupted) fRefCount is now 0, and 
executes "delete this";
Thread 2 continues, checks fRefCount, it is still 0, and executes "delete this";

You see, it is NOT multi thread save. You delete this twice.

You should really use the return value of atomic=5Fadd() to avoid that.

Marcus

Other related posts: