[PATCH 0/1] MR517: xactengine3_7: Remove Entry after FAudio Destroyed call

[Alistair Leslie-Hughes (＠alesliehughes)]

FACTWaveBank_Destroy will invoke the callback which we
attempt to lookup the wavebank.

The callback must have a pointer, help states it isn't valid but still
points to the wavebank that was destroyed.

Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com>

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/517

[PATCH 1/1] xactengine3_7: Remove Entry after FAudio Destroyed call

[Alistair Leslie-Hughes]

From: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com>

FACTWaveBank_Destroy will invoke the callback which we
attempt to lookup the wavebank.

The callback must have a pointer, help states it isn't valid but still
points to the wavebank that was destroyed.

Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com>
---
 dlls/xactengine3_7/tests/xact3.c |  6 ++-
 dlls/xactengine3_7/xact_dll.c    | 70 +++++++++++++++++---------------
 2 files changed, 42 insertions(+), 34 deletions(-)

diff --git a/dlls/xactengine3_7/tests/xact3.c b/dlls/xactengine3_7/tests/xact3.c
index 5daa18ca81b..6b66af59ff0 100644
--- a/dlls/xactengine3_7/tests/xact3.c
+++ b/dlls/xactengine3_7/tests/xact3.c
@@ -172,9 +172,11 @@ static void WINAPI notification_cb(const XACT_NOTIFICATION *notification)
     data->received = TRUE;
     ok(notification->type == data->type,
             "Unexpected notification type %u\n", notification->type);
-    todo_wine_if(notification->type == XACTNOTIFICATIONTYPE_WAVEBANKDESTROYED)
-    ok(notification->waveBank.pWaveBank == data->wave_bank, "Unexpected wave bank %p instead of %p\n",
+    if(notification->type == XACTNOTIFICATIONTYPE_WAVEBANKDESTROYED)
+    {
+        ok(notification->waveBank.pWaveBank == data->wave_bank, "Unexpected wave bank %p instead of %p\n",
             notification->waveBank.pWaveBank, data->wave_bank);
+    }
     ok(thread_id == data->thread_id, "Unexpected thread id %#lx instead of %#lx\n", thread_id, data->thread_id);
 }
 
diff --git a/dlls/xactengine3_7/xact_dll.c b/dlls/xactengine3_7/xact_dll.c
index f7be480656d..ee559dd4b07 100644
--- a/dlls/xactengine3_7/xact_dll.c
+++ b/dlls/xactengine3_7/xact_dll.c
@@ -60,6 +60,21 @@ struct wrapper_lookup
     void *xact;
 };
 
+typedef struct _XACT3EngineImpl {
+    IXACT3Engine IXACT3Engine_iface;
+
+    FACTAudioEngine *fact_engine;
+
+    XACT_READFILE_CALLBACK pReadFile;
+    XACT_GETOVERLAPPEDRESULT_CALLBACK pGetOverlappedResult;
+    XACT_NOTIFICATION_CALLBACK notification_callback;
+
+    void *wb_prepared_context;
+    void *wb_destroyed_context;
+    struct wine_rb_tree wb_wrapper_lookup;
+    CRITICAL_SECTION wb_wrapper_lookup_cs;
+} XACT3EngineImpl;
+
 static int wrapper_lookup_compare(const void *key, const struct wine_rb_entry *entry)
 {
     struct wrapper_lookup *lookup = WINE_RB_ENTRY_VALUE(entry, struct wrapper_lookup, entry);
@@ -74,20 +89,30 @@ static void wrapper_lookup_destroy(struct wine_rb_entry *entry, void *context)
     HeapFree(GetProcessHeap(), 0, lookup);
 }
 
-typedef struct _XACT3EngineImpl {
-    IXACT3Engine IXACT3Engine_iface;
+static void wrapper_remove_entry(XACT3EngineImpl *engine, void *key)
+{
+    struct wrapper_lookup *lookup;
+    struct wine_rb_entry *entry;
 
-    FACTAudioEngine *fact_engine;
+    EnterCriticalSection(&engine->wb_wrapper_lookup_cs);
 
-    XACT_READFILE_CALLBACK pReadFile;
-    XACT_GETOVERLAPPEDRESULT_CALLBACK pGetOverlappedResult;
-    XACT_NOTIFICATION_CALLBACK notification_callback;
+    entry = wine_rb_get(&engine->wb_wrapper_lookup, key);
+    if (!entry)
+    {
+        LeaveCriticalSection(&engine->wb_wrapper_lookup_cs);
 
-    void *wb_prepared_context;
-    void *wb_destroyed_context;
-    struct wine_rb_tree wb_wrapper_lookup;
-    CRITICAL_SECTION wb_wrapper_lookup_cs;
-} XACT3EngineImpl;
+        WARN("cannot find key in wrapper lookup\n");
+    }
+    else
+    {
+        wine_rb_remove(&engine->wb_wrapper_lookup, entry);
+
+        LeaveCriticalSection(&engine->wb_wrapper_lookup_cs);
+
+        lookup = WINE_RB_ENTRY_VALUE(entry, struct wrapper_lookup, entry);
+        HeapFree(GetProcessHeap(), 0, lookup);
+    }
+}
 
 typedef struct _XACT3CueImpl {
     IXACT3Cue IXACT3Cue_iface;
@@ -591,33 +616,14 @@ static inline XACT3WaveBankImpl *impl_from_IXACT3WaveBank(IXACT3WaveBank *iface)
 static HRESULT WINAPI IXACT3WaveBankImpl_Destroy(IXACT3WaveBank *iface)
 {
     XACT3WaveBankImpl *This = impl_from_IXACT3WaveBank(iface);
-    struct wrapper_lookup *lookup;
-    struct wine_rb_entry *entry;
     HRESULT hr;
 
     TRACE("(%p)\n", This);
 
-    EnterCriticalSection(&This->engine->wb_wrapper_lookup_cs);
-
-    entry = wine_rb_get(&This->engine->wb_wrapper_lookup, This->fact_wavebank);
-
-    if (!entry)
-    {
-        LeaveCriticalSection(&This->engine->wb_wrapper_lookup_cs);
-
-        WARN("cannot find wave bank in wrapper lookup\n");
-    }
-    else
-    {
-        wine_rb_remove(&This->engine->wb_wrapper_lookup, entry);
+    hr = FACTWaveBank_Destroy(This->fact_wavebank);
 
-        LeaveCriticalSection(&This->engine->wb_wrapper_lookup_cs);
+    wrapper_remove_entry(This->engine, This->fact_wavebank);
 
-        lookup = WINE_RB_ENTRY_VALUE(entry, struct wrapper_lookup, entry);
-        HeapFree(GetProcessHeap(), 0, lookup);
-    }
-
-    hr = FACTWaveBank_Destroy(This->fact_wavebank);
     HeapFree(GetProcessHeap(), 0, This);
     return hr;
 }
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/517

[PATCH 1/1] xactengine3_7: Remove Entry after FAudio Destroyed call

[Zebediah Figura]

On 7/23/22 02:49, Alistair Leslie-Hughes wrote:
> diff --git a/dlls/xactengine3_7/tests/xact3.c b/dlls/xactengine3_7/tests/xact3.c
> index 5daa18ca81b..6b66af59ff0 100644
> --- a/dlls/xactengine3_7/tests/xact3.c
> +++ b/dlls/xactengine3_7/tests/xact3.c
> @@ -172,9 +172,11 @@ static void WINAPI notification_cb(const XACT_NOTIFICATION *notification)
>       data->received = TRUE;
>       ok(notification->type == data->type,
>               "Unexpected notification type %u\n", notification->type);
> -    todo_wine_if(notification->type == XACTNOTIFICATIONTYPE_WAVEBANKDESTROYED)
> -    ok(notification->waveBank.pWaveBank == data->wave_bank, "Unexpected wave bank %p instead of %p\n",
> +    if(notification->type == XACTNOTIFICATIONTYPE_WAVEBANKDESTROYED)
> +    {
> +        ok(notification->waveBank.pWaveBank == data->wave_bank, "Unexpected wave bank %p instead of %p\n",
>               notification->waveBank.pWaveBank, data->wave_bank);
> +    }

Did you intend to restrict this ok() call to the destroyed type?