st/nine: Protect dtors with mutex

When the flag D3DCREATE_MULTITHREAD is set, a global mutex is used
to protect nine calls.
However for performance reasons, AddRef and Release didn't hold the mutex,
and instead used atomics.

Unfortunately at item release, the item can be destroyed, and that
destruction path should be protected by a mutex (at least for
some objects).

Without this patch, it is possible an app thread is in a dtor
while another thread is making gallium nine calls. It is possible
that two threads are using the same gallium pipe, which is forbiden.
The problem has been made worse with csmt, because it can cause hang,
since nine_csmt_process is not threadsafe.

Fixes Hitman hang, and possibly others.

Signed-off-by: Axel Davy <axel.davy@ens.fr>

diff --git a/src/gallium/state_trackers/nine/iunknown.c b/src/gallium/state_trackers/nine/iunknown.c
index eae4997aa1c2cd399128852dca25416932301af7..d76d64478969d025a707d635f20bb22c847a90aa 100644 (file)
--- a/src/gallium/state_trackers/nine/iunknown.c
+++ b/src/gallium/state_trackers/nine/iunknown.c
@@ -26,6 +26,7 @@
 
 #include "nine_helpers.h"
 #include "nine_pdata.h"
+#include "nine_lock.h"
 
 #define DBG_CHANNEL DBG_UNKNOWN
 
@@ -135,6 +136,31 @@ NineUnknown_Release( struct NineUnknown *This )
     return r;
 }
 
+/* No need to lock the mutex protecting nine (when D3DCREATE_MULTITHREADED)
+ * for AddRef and Release, except for dtor as some of the dtors require it. */
+ULONG NINE_WINAPI
+NineUnknown_ReleaseWithDtorLock( struct NineUnknown *This )
+{
+    if (This->forward)
+        return NineUnknown_ReleaseWithDtorLock(This->container);
+
+    ULONG r = p_atomic_dec_return(&This->refs);
+
+    if (r == 0) {
+        if (This->device) {
+            if (NineUnknown_ReleaseWithDtorLock(NineUnknown(This->device)) == 0)
+                return r; /* everything's gone */
+        }
+        /* Containers (here with !forward) take care of item destruction */
+        if (!This->container && This->bind == 0) {
+            NineLockGlobalMutex();
+            This->dtor(This);
+            NineUnlockGlobalMutex();
+        }
+    }
+    return r;
+}
+
 HRESULT NINE_WINAPI
 NineUnknown_GetDevice( struct NineUnknown *This,
                        IDirect3DDevice9 **ppDevice )

diff --git a/src/gallium/state_trackers/nine/iunknown.h b/src/gallium/state_trackers/nine/iunknown.h
index 4b9edaa355e45b342f63f79fc25b363112e17238..f9ce7b50c977e74314b8aa21d07a1381ae7262a4 100644 (file)
--- a/src/gallium/state_trackers/nine/iunknown.h
+++ b/src/gallium/state_trackers/nine/iunknown.h
@@ -100,6 +100,9 @@ NineUnknown_AddRef( struct NineUnknown *This );
 ULONG NINE_WINAPI
 NineUnknown_Release( struct NineUnknown *This );
 
+ULONG NINE_WINAPI
+NineUnknown_ReleaseWithDtorLock( struct NineUnknown *This );
+
 HRESULT NINE_WINAPI
 NineUnknown_GetDevice( struct NineUnknown *This,
                        IDirect3DDevice9 **ppDevice );

diff --git a/src/gallium/state_trackers/nine/nine_lock.c b/src/gallium/state_trackers/nine/nine_lock.c
index fb24400778aaad77b4967cbd7b28288ef2f7a2c5..1136dad4942d0b66a2407f388c3e671b2b73970c 100644 (file)
--- a/src/gallium/state_trackers/nine/nine_lock.c
+++ b/src/gallium/state_trackers/nine/nine_lock.c
@@ -43,12 +43,25 @@
 #include "volumetexture9.h"
 
 #include "d3d9.h"
+#include "nine_lock.h"
 
 #include "os/os_thread.h"
 
 /* Global mutex as described by MSDN */
 pipe_static_mutex(d3dlock_global);
 
+void
+NineLockGlobalMutex()
+{
+    pipe_mutex_lock(d3dlock_global);
+}
+
+void
+NineUnlockGlobalMutex()
+{
+    pipe_mutex_unlock(d3dlock_global);
+}
+
 static HRESULT NINE_WINAPI
 LockAuthenticatedChannel9_GetCertificateSize( struct NineAuthenticatedChannel9 *This,
                                               UINT *pCertificateSize )
@@ -114,7 +127,7 @@ LockAuthenticatedChannel9_Configure( struct NineAuthenticatedChannel9 *This,
 IDirect3DAuthenticatedChannel9Vtbl LockAuthenticatedChannel9_vtable = {
     (void *)NineUnknown_QueryInterface,
     (void *)NineUnknown_AddRef,
-    (void *)NineUnknown_Release,
+    (void *)NineUnknown_ReleaseWithDtorLock,
     (void *)LockAuthenticatedChannel9_GetCertificateSize,
     (void *)LockAuthenticatedChannel9_GetCertificate,
     (void *)LockAuthenticatedChannel9_NegotiateKeyExchange,
@@ -398,7 +411,7 @@ LockCryptoSession9_GetEncryptionBltKey( struct NineCryptoSession9 *This,
 IDirect3DCryptoSession9Vtbl LockCryptoSession9_vtable = {
     (void *)NineUnknown_QueryInterface,
     (void *)NineUnknown_AddRef,
-    (void *)NineUnknown_Release,
+    (void *)NineUnknown_ReleaseWithDtorLock,
     (void *)LockCryptoSession9_GetCertificateSize,
     (void *)LockCryptoSession9_GetCertificate,
     (void *)LockCryptoSession9_NegotiateKeyExchange,
@@ -481,7 +494,7 @@ LockCubeTexture9_AddDirtyRect( struct NineCubeTexture9 *This,
 IDirect3DCubeTexture9Vtbl LockCubeTexture9_vtable = {
     (void *)NineUnknown_QueryInterface,
     (void *)NineUnknown_AddRef,
-    (void *)NineUnknown_Release,
+    (void *)NineUnknown_ReleaseWithDtorLock,
     (void *)NineUnknown_GetDevice, /* actually part of Resource9 iface */
     (void *)LockUnknown_SetPrivateData,
     (void *)LockUnknown_GetPrivateData,
@@ -1943,7 +1956,7 @@ LockDevice9_CreateQuery( struct NineDevice9 *This,
 IDirect3DDevice9Vtbl LockDevice9_vtable = {
     (void *)NineUnknown_QueryInterface,
     (void *)NineUnknown_AddRef,
-    (void *)NineUnknown_Release,
+    (void *)NineUnknown_ReleaseWithDtorLock,
     (void *)LockDevice9_TestCooperativeLevel,
     (void *)LockDevice9_GetAvailableTextureMem,
     (void *)LockDevice9_EvictManagedResources,
@@ -2270,7 +2283,7 @@ LockDevice9Ex_GetDisplayModeEx( struct NineDevice9Ex *This,
 IDirect3DDevice9ExVtbl LockDevice9Ex_vtable = {
     (void *)NineUnknown_QueryInterface,
     (void *)NineUnknown_AddRef,
-    (void *)NineUnknown_Release,
+    (void *)NineUnknown_ReleaseWithDtorLock,
     (void *)LockDevice9_TestCooperativeLevel,
     (void *)LockDevice9_GetAvailableTextureMem,
     (void *)LockDevice9_EvictManagedResources,
@@ -2447,7 +2460,7 @@ LockDevice9Video_CreateCryptoSession( struct NineDevice9Video *This,
 IDirect3DDevice9VideoVtbl LockDevice9Video_vtable = {
     (void *)NineUnknown_QueryInterface,
     (void *)NineUnknown_AddRef,
-    (void *)NineUnknown_Release,
+    (void *)NineUnknown_ReleaseWithDtorLock,
     (void *)LockDevice9Video_GetContentProtectionCaps,
     (void *)LockDevice9Video_CreateAuthenticatedChannel,
     (void *)LockDevice9Video_CreateCryptoSession
@@ -2493,7 +2506,7 @@ LockIndexBuffer9_GetDesc( struct NineIndexBuffer9 *This,
 IDirect3DIndexBuffer9Vtbl LockIndexBuffer9_vtable = {
     (void *)NineUnknown_QueryInterface,
     (void *)NineUnknown_AddRef,
-    (void *)NineUnknown_Release,
+    (void *)NineUnknown_ReleaseWithDtorLock,
     (void *)NineUnknown_GetDevice, /* actually part of Resource9 iface */
     (void *)LockUnknown_SetPrivateData,
     (void *)LockUnknown_GetPrivateData,
@@ -2535,7 +2548,7 @@ LockPixelShader9_GetFunction( struct NinePixelShader9 *This,
 IDirect3DPixelShader9Vtbl LockPixelShader9_vtable = {
     (void *)NineUnknown_QueryInterface,
     (void *)NineUnknown_AddRef,
-    (void *)NineUnknown_Release,
+    (void *)NineUnknown_ReleaseWithDtorLock,
     (void *)NineUnknown_GetDevice,
     (void *)LockPixelShader9_GetFunction
 };
@@ -2604,7 +2617,7 @@ LockQuery9_GetData( struct NineQuery9 *This,
 IDirect3DQuery9Vtbl LockQuery9_vtable = {
     (void *)NineUnknown_QueryInterface,
     (void *)NineUnknown_AddRef,
-    (void *)NineUnknown_Release,
+    (void *)NineUnknown_ReleaseWithDtorLock,
     (void *)NineUnknown_GetDevice, /* actually part of Query9 iface */
     (void *)NineQuery9_GetType, /* immutable */
     (void *)NineQuery9_GetDataSize, /* immutable */
@@ -2648,7 +2661,7 @@ LockStateBlock9_Apply( struct NineStateBlock9 *This )
 IDirect3DStateBlock9Vtbl LockStateBlock9_vtable = {
     (void *)NineUnknown_QueryInterface,
     (void *)NineUnknown_AddRef,
-    (void *)NineUnknown_Release,
+    (void *)NineUnknown_ReleaseWithDtorLock,
     (void *)NineUnknown_GetDevice, /* actually part of StateBlock9 iface */
     (void *)LockStateBlock9_Capture,
     (void *)LockStateBlock9_Apply
@@ -2727,7 +2740,7 @@ LockSurface9_ReleaseDC( struct NineSurface9 *This,
 IDirect3DSurface9Vtbl LockSurface9_vtable = {
     (void *)NineUnknown_QueryInterface,
     (void *)NineUnknown_AddRef,
-    (void *)NineUnknown_Release,
+    (void *)NineUnknown_ReleaseWithDtorLock,
     (void *)NineUnknown_GetDevice, /* actually part of Resource9 iface */
     (void *)LockUnknown_SetPrivateData,
     (void *)LockUnknown_GetPrivateData,
@@ -2832,7 +2845,7 @@ LockSwapChain9_GetPresentParameters( struct NineSwapChain9 *This,
 IDirect3DSwapChain9Vtbl LockSwapChain9_vtable = {
     (void *)NineUnknown_QueryInterface,
     (void *)NineUnknown_AddRef,
-    (void *)NineUnknown_Release,
+    (void *)NineUnknown_ReleaseWithDtorLock,
     (void *)LockSwapChain9_Present,
     (void *)LockSwapChain9_GetFrontBufferData,
     (void *)LockSwapChain9_GetBackBuffer,
@@ -2879,7 +2892,7 @@ LockSwapChain9Ex_GetDisplayModeEx( struct NineSwapChain9Ex *This,
 IDirect3DSwapChain9ExVtbl LockSwapChain9Ex_vtable = {
     (void *)NineUnknown_QueryInterface,
     (void *)NineUnknown_AddRef,
-    (void *)NineUnknown_Release,
+    (void *)NineUnknown_ReleaseWithDtorLock,
     (void *)LockSwapChain9_Present,
     (void *)LockSwapChain9_GetFrontBufferData,
     (void *)LockSwapChain9_GetBackBuffer,
@@ -2959,7 +2972,7 @@ LockTexture9_AddDirtyRect( struct NineTexture9 *This,
 IDirect3DTexture9Vtbl LockTexture9_vtable = {
     (void *)NineUnknown_QueryInterface,
     (void *)NineUnknown_AddRef,
-    (void *)NineUnknown_Release,
+    (void *)NineUnknown_ReleaseWithDtorLock,
     (void *)NineUnknown_GetDevice, /* actually part of Resource9 iface */
     (void *)LockUnknown_SetPrivateData,
     (void *)LockUnknown_GetPrivateData,
@@ -3021,7 +3034,7 @@ LockVertexBuffer9_GetDesc( struct NineVertexBuffer9 *This,
 IDirect3DVertexBuffer9Vtbl LockVertexBuffer9_vtable = {
     (void *)NineUnknown_QueryInterface,
     (void *)NineUnknown_AddRef,
-    (void *)NineUnknown_Release,
+    (void *)NineUnknown_ReleaseWithDtorLock,
     (void *)NineUnknown_GetDevice, /* actually part of Resource9 iface */
     (void *)LockUnknown_SetPrivateData,
     (void *)LockUnknown_GetPrivateData,
@@ -3063,7 +3076,7 @@ LockVertexDeclaration9_GetDeclaration( struct NineVertexDeclaration9 *This,
 IDirect3DVertexDeclaration9Vtbl LockVertexDeclaration9_vtable = {
     (void *)NineUnknown_QueryInterface,
     (void *)NineUnknown_AddRef,
-    (void *)NineUnknown_Release,
+    (void *)NineUnknown_ReleaseWithDtorLock,
     (void *)NineUnknown_GetDevice, /* actually part of VertexDecl9 iface */
     (void *)LockVertexDeclaration9_GetDeclaration
 };
@@ -3096,7 +3109,7 @@ LockVertexShader9_GetFunction( struct NineVertexShader9 *This,
 IDirect3DVertexShader9Vtbl LockVertexShader9_vtable = {
     (void *)NineUnknown_QueryInterface,
     (void *)NineUnknown_AddRef,
-    (void *)NineUnknown_Release,
+    (void *)NineUnknown_ReleaseWithDtorLock,
     (void *)NineUnknown_GetDevice,
     (void *)LockVertexShader9_GetFunction
 };
@@ -3165,7 +3178,7 @@ LockVolume9_UnlockBox( struct NineVolume9 *This )
 IDirect3DVolume9Vtbl LockVolume9_vtable = {
     (void *)NineUnknown_QueryInterface,
     (void *)NineUnknown_AddRef,
-    (void *)NineUnknown_Release,
+    (void *)NineUnknown_ReleaseWithDtorLock,
     (void *)NineUnknown_GetDevice, /* actually part of Volume9 iface */
     (void *)NineUnknown_SetPrivateData,
     (void *)NineUnknown_GetPrivateData,
@@ -3243,7 +3256,7 @@ LockVolumeTexture9_AddDirtyBox( struct NineVolumeTexture9 *This,
 IDirect3DVolumeTexture9Vtbl LockVolumeTexture9_vtable = {
     (void *)NineUnknown_QueryInterface,
     (void *)NineUnknown_AddRef,
-    (void *)NineUnknown_Release,
+    (void *)NineUnknown_ReleaseWithDtorLock,
     (void *)NineUnknown_GetDevice, /* actually part of Resource9 iface */
     (void *)LockUnknown_SetPrivateData,
     (void *)LockUnknown_GetPrivateData,

diff --git a/src/gallium/state_trackers/nine/nine_lock.h b/src/gallium/state_trackers/nine/nine_lock.h
index 9738a203c144d2721d7fb91fa50d9559c76168ca..4f4a5f14e548faed4340999be01408a16b16d59f 100644 (file)
--- a/src/gallium/state_trackers/nine/nine_lock.h
+++ b/src/gallium/state_trackers/nine/nine_lock.h
@@ -48,4 +48,7 @@ extern IDirect3DVolumeTexture9Vtbl LockVolumeTexture9_vtable;
 extern IDirect3DVolumeTexture9Vtbl LockVolumeTexture9_vtable;
 extern ID3DAdapter9Vtbl LockAdapter9_vtable;
 
+void NineLockGlobalMutex(void);
+void NineUnlockGlobalMutex(void);
+
 #endif /* _NINE_LOCK_H_ */