implement DllCanUnloadNow for dmusic

Robert Shearman rob at codeweavers.com
Sat Dec 4 15:13:47 CST 2004


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



More information about the wine-devel mailing list