[PATCH v6 0/7] MR140: qasf: More ASF reader filter implementation.

Rémi Bernon (@rbernon) wine at gitlab.winehq.org
Tue Jul 5 14:02:06 CDT 2022


> We don't wait for the streaming thread while holding stream_cs, though. 
> Do you mean the reader CS? If we're taking the reader CS from the stream 
> thread, that's also a bug that'll need to be solved somehow.

We wait for the streaming thread, in WMReader_Stop, so while holding the
filter_cs. Which may block the streaming thread if the callbacks need
to enter the filter_cs as well.

> There's a (unfortunately as yet unwritten) rule that nothing from the 
> streaming thread is allowed to ever grab the filter CS.
> 
> I failed to notice this during review the first time, but that means 
> that patch 3/7 is broken. It shouldn't be necessary to take the filter 
> CS, though, since pin connection state can't change while the filter is 
> running.

It is completely non obvious, and imho very brittle. I don't think
the WMReader_Stop call should ever block and wait for the streaming
thread. The reader is asynchronous after all, it should notify the
thread and the callbacks would be called eventually.

The tests clearly suggest that native doesn't block there, as there's
an event and a WaitForSingleObject after the Stop call. Changing the
timeout also shows that the call is fully async.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/140#note_3380



More information about the wine-devel mailing list