From: Brendan McGrath Subject: [PATCH] qedit: Remove "ugly" work around Message-Id: <20220708062811.124776-1-brendan@redmandi.com> Date: Fri, 8 Jul 2022 16:28:11 +1000 This patch removes a work around that causes a crash in Unravel Two. There is a callback in Unravel Two that appears to add a reference to a IMediaSample, which this work around treats as a leak and releases. However, the application also releases the reference causing a negative overflow and an assertion failure on line 345 of dlls/quartz/memallocator.c. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51616 Signed-off-by: Brendan McGrath --- Although I'm not convinced just deleting 12 yr old code is a great idea, the original author did describe this work around as "ugly". I would describe it as unsafe. See Wine Bug 51616. I think it may also be possible for the work around to fail in a multi-threaded scenario, where the IMediaSample is accessed in parallel to the callback; potentially with a negative overflow of its own (and looping 4294967295 times). Although I'm not familiar enough with the uses of qedit to understand if this is even plausible. Unfortunately, I've only tested the removal of this work around with the one application; Unravel Two. Also note that I've only been able to test this "fix" with a local build of Proton 7 Experimental. I couldn't get the application to run in vanilla Wine (as the Origin launcher failed to install). Definitely interested in thoughts. dlls/qedit/samplegrabber.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/dlls/qedit/samplegrabber.c b/dlls/qedit/samplegrabber.c index 9b9780e564a8..cefb2f370785 100644 --- a/dlls/qedit/samplegrabber.c +++ b/dlls/qedit/samplegrabber.c @@ -164,9 +164,6 @@ static void SampleGrabber_callback(struct sample_grabber *This, IMediaSample *sa if (ref) { ERR("(%p) Callback referenced sample %p by %lu\n", This, sample, ref); - /* ugly as hell but some apps are sooo buggy */ - while (ref--) - IMediaSample_Release(sample); } } break; -- 2.34.1