Refcounting in dsound

[Paul Vriens]

Hi, 

while going through some leftover TRACE's of the Interlocked cleanup, I
found the following refcounting in dsound/buffer.c and dsound.c:

dsound/buffer.c:                        This->buffer->ref--;
dsound/buffer.c:                This->buffer->ref--;
dsound/dsound.c:        dsb->buffer->ref++;

Is there a need to clean (i.e. use Interlocked*) this up?

Cheers,

Paul.

[Dimitrie O. Paun]

On Mon, Jan 24, 2005 at 02:50:02PM +0100, Paul Vriens wrote:
> Is there a need to clean (i.e. use Interlocked*) this up?

I thought we've decided to have all mods go through
Interlocked*, for consistency...

-- 
Dimi.

[Paul Vriens]

On Mon, 2005-01-24 at 15:21, Dimitrie O. Paun wrote:
> On Mon, Jan 24, 2005 at 02:50:02PM +0100, Paul Vriens wrote:
> > Is there a need to clean (i.e. use Interlocked*) this up?
>
> I thought we've decided to have all mods go through
> Interlocked*, for consistency...
> I already put something on wine-devel a few days ago, but here is a new
> list:

wine/server/object.c:    obj->refcount++;
wine/server/object.c:    if (!--obj->refcount)

and in dll's:

advapi32/crypt.c:       pProv->refcount++;
advapi32/crypt.c:       pProv->refcount--;
advapi32/service.c:    if (--hdr->ref_count)

dsound/buffer.c:                        This->buffer->ref--;
dsound/buffer.c:                This->buffer->ref--;
dsound/dsound.c:        dsb->buffer->ref++;

msi/table.c:    table->ref_count --;
msi/table.c:        if ( --t->ref_count )
msi/table.c:        (*ptable)->ref_count++;
msi/table.c:    (*ptable)->ref_count++;

Mike about msi:
====
I don't think so, handles are not COM objects and making MSI thread safe
(if it's not already) should be some other Janitorial task.
===

rsaenh/handle.c:    lpObject->refcount++;
rsaenh/handle.c:    pObject->refcount--;

setupapi/virtcopy.c:            vhstrlist[n]->refcount++;
setupapi/virtcopy.c:    vhstrlist[vhstr]->refcount--;

Mike about setupapi:
====
Unlikely.
====

Included are the remarks (the only ones I got so far) from Mike Hearn.

So any suggestions/remarks? If not, I (or if somebody else beats me)
will produce patches for all of the above.

Cheers,

Paul.

[Mike Hearn]

On Mon, 24 Jan 2005 15:42:34 +0100, Paul Vriens wrote:
> wine/server/object.c:    obj->refcount++;
> wine/server/object.c:    if (!--obj->refcount)

No. The wineserver is not multithreaded.
> and in dll's:
>
> advapi32/crypt.c:       pProv->refcount++;
> advapi32/crypt.c:       pProv->refcount--;
> advapi32/service.c:    if (--hdr->ref_count)

Not sure, maybe. There's no context to these, you really do have to read
the code and MSDN to figure out whether it needs to be thread safe or not.
> msi/table.c:    table->ref_count --;
> msi/table.c:        if ( --t->ref_count )
> msi/table.c:        (*ptable)->ref_count++;
> msi/table.c:    (*ptable)->ref_count++;
>
> Mike about msi:
> ====
> I don't think so, handles are not COM objects and making MSI thread safe
> (if it's not already) should be some other Janitorial task.
> ===

I'd leave MSI to Mike and Aric at the moment.

thanks -mike

[Robert Reif]

Paul Vriens wrote:
> Hi,
>
> while going through some leftover TRACE's of the Interlocked cleanup, I
> found the following refcounting in dsound/buffer.c and dsound.c:
>
> dsound/buffer.c:                        This->buffer->ref--;
> dsound/buffer.c:                This->buffer->ref--;
> dsound/dsound.c:        dsb->buffer->ref++;
>
> Is there a need to clean (i.e. use Interlocked*) this up?
>
> Cheers,
>
> Paul.

buffer is not a COM object.  Using Interlocked* functions will not
make it thread safe.  This needs to be put under the dsb lock to be
correct but the odds of a program duplicating a buffer in one thread
while deleting a duplicated buffer in another are slim.  I'll look
into fixing this when I get a chance.