[PATCH 1/2] mfreadwrite/reader: Make the stream sample allocator callback own a reference to the source reader.

[Giovanni Mascellani]

The stream sample allocator callback NotifyRelease() method assumes that
the source reader is valid. Therefore it must own a reference to it,
otherwise it might get called while the source reader is being destroyed,
causing a crash.

Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com>
---
 dlls/mfreadwrite/reader.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/dlls/mfreadwrite/reader.c b/dlls/mfreadwrite/reader.c
index bdf2e7f5a6f..2d3da19db10 100644
--- a/dlls/mfreadwrite/reader.c
+++ b/dlls/mfreadwrite/reader.c
@@ -2282,12 +2282,14 @@ static HRESULT WINAPI stream_sample_allocator_cb_QueryInterface(IMFVideoSampleAl
 
 static ULONG WINAPI stream_sample_allocator_cb_AddRef(IMFVideoSampleAllocatorNotify *iface)
 {
-    return 2;
+    struct media_stream *stream = impl_stream_from_IMFVideoSampleAllocatorNotify(iface);
+    return source_reader_addref(stream->reader);
 }
 
 static ULONG WINAPI stream_sample_allocator_cb_Release(IMFVideoSampleAllocatorNotify *iface)
 {
-    return 1;
+    struct media_stream *stream = impl_stream_from_IMFVideoSampleAllocatorNotify(iface);
+    return source_reader_release(stream->reader);
 }
 
 static HRESULT WINAPI stream_sample_allocator_cb_NotifyRelease(IMFVideoSampleAllocatorNotify *iface)
-- 
2.35.1

[PATCH 2/2] mfreadwrite/reader: Unset sample allocator callbacks while destroying the source reader.

[Giovanni Mascellani]

The sample allocator callbacks own a reference to the source reader,
which creates a reference loop and prevents the source reader from
being properly destroyed.

Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com>
---
 dlls/mfreadwrite/reader.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/dlls/mfreadwrite/reader.c b/dlls/mfreadwrite/reader.c
index 2d3da19db10..1c86a10a824 100644
--- a/dlls/mfreadwrite/reader.c
+++ b/dlls/mfreadwrite/reader.c
@@ -1430,6 +1430,25 @@ static ULONG WINAPI src_reader_Release(IMFSourceReader *iface)
 
             LeaveCriticalSection(&reader->cs);
         }
+
+        for (i = 0; i < reader->stream_count; ++i)
+        {
+            struct media_stream *stream = &reader->streams[i];
+            IMFVideoSampleAllocatorCallback *callback;
+
+            if (!stream->allocator)
+                continue;
+
+            if (SUCCEEDED(IMFVideoSampleAllocatorEx_QueryInterface(stream->allocator, &IID_IMFVideoSampleAllocatorCallback, (void **)&callback)))
+            {
+                HRESULT hr;
+
+                if (FAILED(hr = IMFVideoSampleAllocatorCallback_SetCallback(callback, NULL)))
+                    WARN("Cannot unset allocator callback, hr %#x.\n", hr);
+                IMFVideoSampleAllocatorCallback_Release(callback);
+            }
+        }
+
         source_reader_release(reader);
     }
 
-- 
2.35.1

[PATCH 2/2] mfreadwrite/reader: Unset sample allocator callbacks while destroying the source reader.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=112247

Your paranoid android.


=== debian11 (build log) ===

../wine/dlls/mfreadwrite/reader.c:1434:14: error: ‘i’ undeclared (first use in this function)
Task: The win32 Wine build failed

=== debian11 (build log) ===

../wine/dlls/mfreadwrite/reader.c:1434:14: error: ‘i’ undeclared (first use in this function)
Task: The wow64 Wine build failed