On 8/11/22 12:19, Rémi Bernon (@rbernon) wrote:
On Thu Aug 11 17:09:14 2022 +0000, **** wrote:
Zebediah Figura (she/her) replied on the mailing list:
On 8/11/22 11:56, Rémi Bernon (@rbernon) wrote: > On Thu Aug 11 16:53:21 2022 +0000, **** wrote: >> Zebediah Figura (she/her) replied on the mailing list: >> \`\`\` >> On 8/11/22 11:45, Rémi Bernon wrote: >>> @@ -225,6 +248,20 @@ static HRESULT asf_reader_cleanup_stream(struct >> strmbase_filter *iface) >>> >>> TRACE("iface %p\n", iface); >>> >>> + while (filter->status == -1) >>> + SleepConditionVariableCS(&filter->status_cv, >> &filter->filter.filter_cs, INFINITE); >> We're always waiting for state changes to complete after making them; >> why do we need this? >> \`\`\` > I think there's always a possibility that Start / Stop gets called concurrently, and because we're leaving the filter CS while waiting for the change it's then possible to initiate another state change concurrently. > I missed this the first time, but we really shouldn't be doing that. We should use a separate CS to synchronize with the streaming thread. _______________________________________________ wine-gitlab mailing list -- wine-gitlab@winehq.org To unsubscribe send an email to wine-gitlab-leave@winehq.orgI don't think the same, we need to enter the filter CS in the callbacks to safely access the filter streams and other members in an explicit way.
Although maybe there's some hidden guarantee that we safely can, after `init_stream` and before `cleanup_stream`, I think it's not explicit and therefore risky.
If there's members we need to share with the streaming thread we should use something other than filter_cs to do it—whether that means using a different mutex, or relying on the fact that the stream list can't change while the filter is running.
Somewhat awkwardly, other quartz code violates this rule, but that's also something we should fix.
I do not think it is at all workable to make state change methods any less than fully serialized.