[PATCH] qedit: Remove "ugly" work around

Zebediah Figura zfigura at codeweavers.com
Fri Jul 8 13:57:58 CDT 2022


On 7/8/22 01:28, Brendan McGrath wrote:
> 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 <brendan at redmandi.com>
> ---
> 
> 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.
> 

I'd very much like to remove this if it's incorrect, but obviously it 
was put there for a reason, and I don't feel comfortable just removing 
it in that case. It'd be nice if we can test with whatever original 
application—I've CC'd the original author just in case he's still 
responsive and can remember what was affected—but in lieu of that I'd 
rather see tests justifying this change in behaviour.



More information about the wine-devel mailing list