Based on a patch by Jinoh Kang:
https://gitlab.winehq.org/wine/wine/-/merge_requests/155
This patch simplifies the comment somewhat; I don't see it as necessary to fully
explain the concept of memory barriers and acquire/release everywhere that they
are used. In my opinion, it's necessary and also sufficient to explain why these
specific places need an acquire/release pair, what that pair is protecting, and
to cross-reference the parts of the pair with each other.
I've also removed the IOSB aliasing patches. Personally I think that there's no
harm in making our code more endianness-conscious, for things like Winelib, but
on the other hand Windows never supported big-endian, and we've resisted adding
support for architectures Windows doesn't support (and see also [1]). Since it's
not clear to me they're desirable, and they're orthogonal to the purpose of this
patch set, I've removed them; they can be submitted later if there is a
favourable consensus.
[1] https://www.winehq.org/pipermail/wine-devel/2021-July/191600.html
--
v2: ntdll: Use an acquire/release pair on the IOSB status.
https://gitlab.winehq.org/wine/wine/-/merge_requests/1342
On Mon Nov 14 12:05:16 2022 +0000, Zebediah Figura wrote:
> Side note that just occurs to me, we should probably add thread
> descriptions for any new threads, though I don't mind deferring that.
Good idea, done.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/1311#note_15681
On Fri Nov 11 21:23:39 2022 +0000, Zebediah Figura wrote:
> > First because it's how it's supposed to be.
> I initially thought that we weren't actually running any visible
> callbacks on these read threads, but I guess the allocation callbacks
> are coming from here. Windows really spawns two threads for each stream? Sheesh.
Yes.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/1311#note_15679
On Fri Nov 11 21:26:14 2022 +0000, Zebediah Figura wrote:
> I think we'd still need that GetStreamSelected() check, though, given
> we're looping over all the streams here?
I don't think we need to check it here anymore with the latest version. Deselected streams will never have pending read requests (or they will shortly complete it after deselection, with either error or a last sample).
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/1311#note_15680
On Fri Nov 11 21:17:44 2022 +0000, Zebediah Figura wrote:
> Hmm, I never tested selecting streams while running. If that's supposed
> to work it may be better just to leave it as is.
Yeah, starting / stopping threads dynamically didn't seem like a good idea and it's easier to check the correctness with all stream threads started on open / stopped on close.
So instead I keep the thread running but only emit read requests to selected streams. When a stream is deselected, eventually dynamically, it will either fail its read request with `NS_E_NO_MORE_SAMPLES`, or returns a last sample if that happened before, but will then stop receiving read requests.
When a stream is selected again, a read request will be emitted and it will resume reading.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/1311#note_15678
On Fri Nov 11 21:18:28 2022 +0000, Zebediah Figura wrote:
> Yeah, if we need to do this to prevent a deadlock, that sounds to me
> like something we need to solve on the sync reader side.
I dropped it for now.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/1311#note_15677