[PATCH 0/3] MR6763: dplayx fixes

[Alistair Leslie-Hughes (＠alesliehughes)]

This patchset does includes a patch from another patchset, which I'll rebase once this one is accepted.

[PATCH 1/3] dplayx: Use a single reference count for IDirectPlay interfaces.

[Alistair Leslie-Hughes]

From: Alistair Leslie-Hughes

---
 dlls/dplayx/dplay.c        | 92 +++++++++++++-------------------------
 dlls/dplayx/dplay_global.h |  3 +-
 dlls/dplayx/tests/dplayx.c | 25 -----------
 3 files changed, 33 insertions(+), 87 deletions(-)

diff --git a/dlls/dplayx/dplay.c b/dlls/dplayx/dplay.c
index 99d50f53d6c..3e5f7e77f2d 100644
--- a/dlls/dplayx/dplay.c
+++ b/dlls/dplayx/dplay.c
@@ -555,9 +555,6 @@ static ULONG WINAPI IDirectPlayImpl_AddRef( IDirectPlay *iface )

TRACE( "(%p) ref=%ld\n", This, ref );

-    if ( ref == 1 )
-        InterlockedIncrement( &This->numIfaces );
-
     return ref;
 }

@@ -568,7 +565,7 @@ static ULONG WINAPI IDirectPlayImpl_Release( IDirectPlay *iface )

TRACE( "(%p) ref=%ld\n", This, ref );

-    if ( !ref && !InterlockedDecrement( &This->numIfaces ) )
+    if ( !ref )
         dplay_destroy( This );

return ref;
@@ -869,12 +866,9 @@ static HRESULT WINAPI IDirectPlay4Impl_QueryInterface( IDirectPlay4 *iface, REFI
 static ULONG WINAPI IDirectPlay2AImpl_AddRef( IDirectPlay2A *iface )
 {
     IDirectPlayImpl *This = impl_from_IDirectPlay2A( iface );
-    ULONG ref = InterlockedIncrement( &This->ref2A );
-
-    TRACE( "(%p) ref2A=%ld\n", This, ref );
+    ULONG ref = InterlockedIncrement( &This->ref );

-    if ( ref == 1 )
-        InterlockedIncrement( &This->numIfaces );
+    TRACE( "(%p) ref=%ld\n", This, ref );

return ref;
 }
@@ -882,12 +876,9 @@ static ULONG WINAPI IDirectPlay2AImpl_AddRef( IDirectPlay2A *iface )
 static ULONG WINAPI IDirectPlay2Impl_AddRef( IDirectPlay2 *iface )
 {
     IDirectPlayImpl *This = impl_from_IDirectPlay2( iface );
-    ULONG ref = InterlockedIncrement( &This->ref2 );
-
-    TRACE( "(%p) ref2=%ld\n", This, ref );
+    ULONG ref = InterlockedIncrement( &This->ref );

-    if ( ref == 1 )
-        InterlockedIncrement( &This->numIfaces );
+    TRACE( "(%p) ref=%ld\n", This, ref );

return ref;
 }
@@ -895,12 +886,9 @@ static ULONG WINAPI IDirectPlay2Impl_AddRef( IDirectPlay2 *iface )
 static ULONG WINAPI IDirectPlay3AImpl_AddRef( IDirectPlay3A *iface )
 {
     IDirectPlayImpl *This = impl_from_IDirectPlay3A( iface );
-    ULONG ref = InterlockedIncrement( &This->ref3A );
-
-    TRACE( "(%p) ref3A=%ld\n", This, ref );
+    ULONG ref = InterlockedIncrement( &This->ref );

-    if ( ref == 1 )
-        InterlockedIncrement( &This->numIfaces );
+    TRACE( "(%p) ref=%ld\n", This, ref );

return ref;
 }
@@ -908,12 +896,9 @@ static ULONG WINAPI IDirectPlay3AImpl_AddRef( IDirectPlay3A *iface )
 static ULONG WINAPI IDirectPlay3Impl_AddRef( IDirectPlay3 *iface )
 {
     IDirectPlayImpl *This = impl_from_IDirectPlay3( iface );
-    ULONG ref = InterlockedIncrement( &This->ref3 );
-
-    TRACE( "(%p) ref3=%ld\n", This, ref );
+    ULONG ref = InterlockedIncrement( &This->ref );

-    if ( ref == 1 )
-        InterlockedIncrement( &This->numIfaces );
+    TRACE( "(%p) ref=%ld\n", This, ref );

return ref;
 }
@@ -921,12 +906,9 @@ static ULONG WINAPI IDirectPlay3Impl_AddRef( IDirectPlay3 *iface )
 static ULONG WINAPI IDirectPlay4AImpl_AddRef(IDirectPlay4A *iface)
 {
     IDirectPlayImpl *This = impl_from_IDirectPlay4A( iface );
-    ULONG ref = InterlockedIncrement( &This->ref4A );
-
-    TRACE( "(%p) ref4A=%ld\n", This, ref );
+    ULONG ref = InterlockedIncrement( &This->ref );

-    if ( ref == 1 )
-        InterlockedIncrement( &This->numIfaces );
+    TRACE( "(%p) ref=%ld\n", This, ref );

return ref;
 }
@@ -934,12 +916,9 @@ static ULONG WINAPI IDirectPlay4AImpl_AddRef(IDirectPlay4A *iface)
 static ULONG WINAPI IDirectPlay4Impl_AddRef(IDirectPlay4 *iface)
 {
     IDirectPlayImpl *This = impl_from_IDirectPlay4( iface );
-    ULONG ref = InterlockedIncrement( &This->ref4 );
-
-    TRACE( "(%p) ref4=%ld\n", This, ref );
+    ULONG ref = InterlockedIncrement( &This->ref );

-    if ( ref == 1 )
-        InterlockedIncrement( &This->numIfaces );
+    TRACE( "(%p) ref=%ld\n", This, ref );

return ref;
 }
@@ -947,11 +926,11 @@ static ULONG WINAPI IDirectPlay4Impl_AddRef(IDirectPlay4 *iface)
 static ULONG WINAPI IDirectPlay2AImpl_Release( IDirectPlay2A *iface )
 {
     IDirectPlayImpl *This = impl_from_IDirectPlay2A( iface );
-    ULONG ref = InterlockedDecrement( &This->ref2A );
+    ULONG ref = InterlockedDecrement( &This->ref );

-    TRACE( "(%p) ref2A=%ld\n", This, ref );
+    TRACE( "(%p) ref=%ld\n", This, ref );

-    if ( !ref && !InterlockedDecrement( &This->numIfaces ) )
+    if ( !ref )
         dplay_destroy( This );

return ref;
@@ -960,11 +939,11 @@ static ULONG WINAPI IDirectPlay2AImpl_Release( IDirectPlay2A *iface )
 static ULONG WINAPI IDirectPlay2Impl_Release( IDirectPlay2 *iface )
 {
     IDirectPlayImpl *This = impl_from_IDirectPlay2( iface );
-    ULONG ref = InterlockedDecrement( &This->ref2 );
+    ULONG ref = InterlockedDecrement( &This->ref );

-    TRACE( "(%p) ref2=%ld\n", This, ref );
+    TRACE( "(%p) ref=%ld\n", This, ref );

-    if ( !ref && !InterlockedDecrement( &This->numIfaces ) )
+    if ( !ref  )
         dplay_destroy( This );

return ref;
@@ -973,11 +952,11 @@ static ULONG WINAPI IDirectPlay2Impl_Release( IDirectPlay2 *iface )
 static ULONG WINAPI IDirectPlay3AImpl_Release( IDirectPlay3A *iface )
 {
     IDirectPlayImpl *This = impl_from_IDirectPlay3A( iface );
-    ULONG ref = InterlockedDecrement( &This->ref3A );
+    ULONG ref = InterlockedDecrement( &This->ref );

-    TRACE( "(%p) ref3A=%ld\n", This, ref );
+    TRACE( "(%p) ref=%ld\n", This, ref );

-    if ( !ref && !InterlockedDecrement( &This->numIfaces ) )
+    if ( !ref )
         dplay_destroy( This );

return ref;
@@ -986,11 +965,11 @@ static ULONG WINAPI IDirectPlay3AImpl_Release( IDirectPlay3A *iface )
 static ULONG WINAPI IDirectPlay3Impl_Release( IDirectPlay3 *iface )
 {
     IDirectPlayImpl *This = impl_from_IDirectPlay3( iface );
-    ULONG ref = InterlockedDecrement( &This->ref3 );
+    ULONG ref = InterlockedDecrement( &This->ref );

-    TRACE( "(%p) ref3=%ld\n", This, ref );
+    TRACE( "(%p) ref=%ld\n", This, ref );

-    if ( !ref && !InterlockedDecrement( &This->numIfaces ) )
+    if ( !ref )
         dplay_destroy( This );

return ref;
@@ -999,11 +978,11 @@ static ULONG WINAPI IDirectPlay3Impl_Release( IDirectPlay3 *iface )
 static ULONG WINAPI IDirectPlay4AImpl_Release(IDirectPlay4A *iface)
 {
     IDirectPlayImpl *This = impl_from_IDirectPlay4A( iface );
-    ULONG ref = InterlockedDecrement( &This->ref4A );
+    ULONG ref = InterlockedDecrement( &This->ref );

-    TRACE( "(%p) ref4A=%ld\n", This, ref );
+    TRACE( "(%p) ref=%ld\n", This, ref );

-    if ( !ref && !InterlockedDecrement( &This->numIfaces ) )
+    if ( !ref )
         dplay_destroy( This );

return ref;
@@ -1012,11 +991,11 @@ static ULONG WINAPI IDirectPlay4AImpl_Release(IDirectPlay4A *iface)
 static ULONG WINAPI IDirectPlay4Impl_Release(IDirectPlay4 *iface)
 {
     IDirectPlayImpl *This = impl_from_IDirectPlay4( iface );
-    ULONG ref = InterlockedDecrement( &This->ref4 );
+    ULONG ref = InterlockedDecrement( &This->ref );

-    TRACE( "(%p) ref4=%ld\n", This, ref );
+    TRACE( "(%p) ref=%ld\n", This, ref );

-    if ( !ref && !InterlockedDecrement( &This->numIfaces ) )
+    if ( !ref )
         dplay_destroy( This );

return ref;
@@ -6085,14 +6064,7 @@ HRESULT dplay_create( REFIID riid, void **ppv )
     obj->IDirectPlay3_iface.lpVtbl = &dp3_vt;
     obj->IDirectPlay4A_iface.lpVtbl = &dp4A_vt;
     obj->IDirectPlay4_iface.lpVtbl = &dp4_vt;
-    obj->numIfaces = 1;
-    obj->ref = 0;
-    obj->ref2A = 0;
-    obj->ref2 = 0;
-    obj->ref3A = 0;
-    obj->ref3 = 0;
-    obj->ref4A = 0;
-    obj->ref4 = 1;
+    obj->ref = 1;

InitializeCriticalSectionEx( &obj->lock, 0, RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO );
     obj->lock.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": IDirectPlayImpl.lock");
diff --git a/dlls/dplayx/dplay_global.h b/dlls/dplayx/dplay_global.h
index cf3bbd0d592..b011c183c7d 100644
--- a/dlls/dplayx/dplay_global.h
+++ b/dlls/dplayx/dplay_global.h
@@ -203,8 +203,7 @@ typedef struct IDirectPlayImpl
   IDirectPlay3 IDirectPlay3_iface;
   IDirectPlay4A IDirectPlay4A_iface;
   IDirectPlay4  IDirectPlay4_iface;
-  LONG numIfaces; /* "in use interfaces" refcount */
-  LONG ref, ref2A, ref2, ref3A, ref3, ref4A, ref4;
+  LONG ref;
   CRITICAL_SECTION lock;
   DirectPlay2Data *dp2;
 } IDirectPlayImpl;
diff --git a/dlls/dplayx/tests/dplayx.c b/dlls/dplayx/tests/dplayx.c
index c3d4dac7bc6..a2336d6744f 100644
--- a/dlls/dplayx/tests/dplayx.c
+++ b/dlls/dplayx/tests/dplayx.c
@@ -8938,61 +8938,36 @@ static void test_COM(void)
     hr = CoCreateInstance(&CLSID_DirectPlay, NULL, CLSCTX_INPROC_SERVER, &IID_IDirectPlay4,
             (void**)&dp4);
     ok(hr == S_OK, "DirectPlay create failed: %08lx, expected S_OK\n", hr);
-    refcount = IDirectPlayX_AddRef(dp4);
-    ok(refcount == 2, "refcount == %lu, expected 2\n", refcount);

hr = IDirectPlayX_QueryInterface(dp4, &IID_IDirectPlay2A, (void**)&dp2A);
     ok(hr == S_OK, "QueryInterface for IID_IDirectPlay2A failed: %08lx\n", hr);
-    refcount = IDirectPlay2_AddRef(dp2A);
-    ok(refcount == 2, "refcount == %lu, expected 2\n", refcount);
     IDirectPlay2_Release(dp2A);

hr = IDirectPlayX_QueryInterface(dp4, &IID_IDirectPlay2, (void**)&dp2);
     ok(hr == S_OK, "QueryInterface for IID_IDirectPlay2 failed: %08lx\n", hr);
-    refcount = IDirectPlay2_AddRef(dp2);
-    ok(refcount == 2, "refcount == %lu, expected 2\n", refcount);
     IDirectPlay2_Release(dp2);

hr = IDirectPlayX_QueryInterface(dp4, &IID_IDirectPlay3A, (void**)&dp3A);
     ok(hr == S_OK, "QueryInterface for IID_IDirectPlay3A failed: %08lx\n", hr);
-    refcount = IDirectPlay3_AddRef(dp3A);
-    ok(refcount == 2, "refcount == %lu, expected 2\n", refcount);
     IDirectPlay3_Release(dp3A);

hr = IDirectPlayX_QueryInterface(dp4, &IID_IDirectPlay3, (void**)&dp3);
     ok(hr == S_OK, "QueryInterface for IID_IDirectPlay3 failed: %08lx\n", hr);
-    refcount = IDirectPlay3_AddRef(dp3);
-    ok(refcount == 2, "refcount == %lu, expected 2\n", refcount);
     IDirectPlay3_Release(dp3);

hr = IDirectPlayX_QueryInterface(dp4, &IID_IDirectPlay4A, (void**)&dp4A);
     ok(hr == S_OK, "QueryInterface for IID_IDirectPlay4A failed: %08lx\n", hr);
-    refcount = IDirectPlayX_AddRef(dp4A);
-    ok(refcount == 2, "refcount == %lu, expected 2\n", refcount);
     IDirectPlayX_Release(dp4A);

/* IDirectPlay and IUnknown share a refcount */
     hr = IDirectPlayX_QueryInterface(dp4, &IID_IDirectPlay, (void**)&dp);
     ok(hr == S_OK, "QueryInterface for IID_IDirectPlay failed: %08lx\n", hr);
-    refcount = IDirectPlayX_AddRef(dp);
-    ok(refcount == 2, "refcount == %lu, expected 2\n", refcount);
     IDirectPlay_Release(dp);

hr = IDirectPlayX_QueryInterface(dp4, &IID_IUnknown, (void**)&unk);
     ok(hr == S_OK, "QueryInterface for IID_IUnknown failed: %08lx\n", hr);
-    refcount = IUnknown_AddRef(unk);
-    ok(refcount == 3, "refcount == %lu, expected 3\n", refcount);
-    refcount = IUnknown_Release(unk);
-    ok(refcount == 2, "refcount == %lu, expected 2\n", refcount);

IUnknown_Release(unk);
-    IDirectPlay_Release(dp);
-    IDirectPlayX_Release(dp4A);
-    IDirectPlay3_Release(dp3);
-    IDirectPlay3_Release(dp3A);
-    IDirectPlay2_Release(dp2);
-    IDirectPlay2_Release(dp2A);
-    IDirectPlayX_Release(dp4);
     refcount = IDirectPlayX_Release(dp4);
     ok(refcount == 0, "refcount == %lu, expected 0\n", refcount);
 }

[PATCH 2/3] dplayx: IDirectPlayLobby remove numIfaces variable

[Alistair Leslie-Hughes]

From: Alistair Leslie-Hughes

---
 dlls/dplayx/dplobby.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/dlls/dplayx/dplobby.c b/dlls/dplayx/dplobby.c
index bf932dd25f9..5351e2b9fd4 100644
--- a/dlls/dplayx/dplobby.c
+++ b/dlls/dplayx/dplobby.c
@@ -65,7 +65,6 @@ typedef struct IDirectPlayLobbyImpl
 {
     IDirectPlayLobby3 IDirectPlayLobby3_iface;
     IDirectPlayLobby3A IDirectPlayLobby3A_iface;
-    LONG numIfaces; /* "in use interfaces" refcount */
     LONG ref;
     CRITICAL_SECTION lock;
     HKEY cbkeyhack;
@@ -139,9 +138,6 @@ static ULONG WINAPI IDirectPlayLobby3AImpl_AddRef(IDirectPlayLobby3A *iface)

TRACE( "(%p) ref3=%ld\n", This, ref );

-    if ( ref == 1 )
-        InterlockedIncrement( &This->numIfaces );
-
     return ref;
 }

@@ -152,9 +148,6 @@ static ULONG WINAPI IDirectPlayLobby3Impl_AddRef(IDirectPlayLobby3 *iface)

TRACE( "(%p) ref3=%ld\n", This, ref );

-    if ( ref == 1 )
-        InterlockedIncrement( &This->numIfaces );
-
     return ref;
 }

@@ -165,7 +158,7 @@ static ULONG WINAPI IDirectPlayLobby3AImpl_Release(IDirectPlayLobby3A *iface)

TRACE( "(%p) ref=%ld\n", This, ref );

-    if ( !ref && !InterlockedDecrement( &This->numIfaces ) )
+    if ( !ref )
         dplobby_destroy( This );

return ref;
@@ -178,7 +171,7 @@ static ULONG WINAPI IDirectPlayLobby3Impl_Release(IDirectPlayLobby3 *iface)

TRACE( "(%p) ref=%ld\n", This, ref );

-    if ( !ref && !InterlockedDecrement( &This->numIfaces ) )
+    if ( !ref )
         dplobby_destroy( This );

return ref;
@@ -1419,7 +1412,6 @@ HRESULT dplobby_create( REFIID riid, void **ppv )

obj->IDirectPlayLobby3_iface.lpVtbl = &dpl3_vt;
     obj->IDirectPlayLobby3A_iface.lpVtbl = &dpl3A_vt;
-    obj->numIfaces = 1;
     obj->msgtid = 0;
     obj->ref = 1;

[PATCH 3/3] dplayx: Use default DllCanUnloadNow implementation.

[Alistair Leslie-Hughes]

From: Alistair Leslie-Hughes

---
 dlls/dplayx/dplayx_main.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/dlls/dplayx/dplayx_main.c b/dlls/dplayx/dplayx_main.c
index 7b8fbcc48c1..a32fd8be697 100644
--- a/dlls/dplayx/dplayx_main.c
+++ b/dlls/dplayx/dplayx_main.c
@@ -96,18 +96,3 @@ BOOL WINAPI DllMain( HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved )
   return TRUE;
 }

-/***********************************************************************
- *              DllCanUnloadNow (DPLAYX.@)
- */
-HRESULT WINAPI DllCanUnloadNow(void)
-{
-  HRESULT hr = ( gdwDPlaySPRefCount > 0 ) ? S_FALSE : S_OK;
-
-  /* FIXME: Should I be putting a check in for class factory objects
-   *        as well?
-   */
-
-  TRACE( ": returning %#lx\n", hr );
-
-  return hr;
-}

[PATCH 3/3] dplayx: Use default DllCanUnloadNow implementation.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=149445

Your paranoid android.

=== w7u_2qxl (32 bit report) ===

dplayx:
dplayx.c:3483: Test failed: recv() returned 52.
dplayx.c:3495: Test failed: recv() returned 52.

=== w10pro64 (32 bit report) ===

dplayx:
dplayx.c:2775: Test failed: got flags 0x10.