implement DllCanUnloadNow for dmusic

[James Hawkins]

On Sat, 04 Dec 2004 15:13:47 -0600, Robert Shearman <rob at codeweavers.com> wrote:
> James Hawkins wrote:
> 
> >Changelog
> >   * properly implement DllCanUnloadNow ref counting
> >
> >
> >
> 
> Sorry, still isn't correct. Please see my comments below.
> 
> >------------------------------------------------------------------------
> >
> >Index: dlls/dmusic/buffer.c
> >===================================================================
> >RCS file: /home/wine/wine/dlls/dmusic/buffer.c,v
> >retrieving revision 1.8
> >diff -u -r1.8 buffer.c
> >--- dlls/dmusic/buffer.c       6 Sep 2004 21:34:25 -0000       1.8
> >+++ dlls/dmusic/buffer.c       4 Dec 2004 20:52:16 -0000
> >@@ -38,6 +38,9 @@
> > ULONG WINAPI IDirectMusicBufferImpl_AddRef (LPDIRECTMUSICBUFFER iface) {
> >       IDirectMusicBufferImpl *This = (IDirectMusicBufferImpl *)iface;
> >       TRACE("(%p): AddRef from %ld\n", This, This->ref);
> >+
> >+      DMUSIC_LockModule();
> >+
> >       return ++(This->ref);
> > }
> >
> >@@ -45,9 +48,13 @@
> >       IDirectMusicBufferImpl *This = (IDirectMusicBufferImpl *)iface;
> >       ULONG ref = --This->ref;
> >       TRACE("(%p): ReleaseRef to %ld\n", This, This->ref);
> >+
> >       if (ref == 0) {
> >               HeapFree(GetProcessHeap(), 0, This);
> >       }
> >+
> >+      DMUSIC_UnlockModule();
> >+
> >       return ref;
> > }
> >
> >@@ -160,6 +167,8 @@
> >       }
> >       dmbuff->lpVtbl = &DirectMusicBuffer_Vtbl;
> >       dmbuff->ref = 0; /* will be inited by QueryInterface */
> >+
> >+      DMUSIC_LockModule();
> >
> >       return IDirectMusicBufferImpl_QueryInterface ((LPDIRECTMUSICBUFFER)dmbuff, lpcGUID, ppobj);
> > }
> >
> >
> 
> Ok, think about this sequence of events that should leave the DLL ref
> count at 0:
> 1. Create an IDirectMusicBufferImpl object.
> 1a. Calls LockModule.
> 1b. Calls QueryInterface which calls LockModule.
> 2. Call Release, which calls LockModule.
> 
> The solution is to either remove the LockModule from the constructor, or
> to remove the LockModule from the AddRef and only call UnlockModule in
> the destructor part of Release. This issue is also present in the other
> files in your patch (not quoted here).
> 
> I should have thought about this before when I commented on your last
> patch. I guess I need to put some tests together for this feature.
> 
> Rob
> 

MSDN about QueryInterface:
"Returns a pointer to a specified interface on an object to which a
client currently holds an interface pointer. This function must call
IUnknown::AddRef on the pointer it returns."

QueryInterface should be calling AddRef, so if any constructor calls
QueryInterface (I'm guessing without looking that they all do) then
there shouldn't be a call to LockModule in that constructor.  Is that
about right?

-- 
James Hawkins