August 2023 - Wine-GitLab - list.winehq.org

Re: [PATCH v3 0/1] MR3491: winegstreamer: Leave media source critical section before unlocking workqueue.
by Rémi Bernon Aug. 4, 2023

Aug. 4, 2023

On Fri Aug 4 21:02:44 2023 +0000, Zebediah Figura wrote: > > We could maybe grab the Queue id in the cs, 0 it, and use the local > copy for unlocking. Does that sound better? > That does seem like an improvement; I can't say I see a better way to > solve the problem after looking, at least. We should probably mention in > a comment why we're releasing outside of the lock; it is not obvious at > all from the documentation that MFUnlockWorkQueue() can block. Nothing can call PutWorkItem or use the queue after Shutdown has been called. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3491#note_41432

1 0

[PATCH v2 0/3] MR3428: Avoid relying on linux/ipx.h to define SOL_IPX.
by Jinoh Kang (＠iamahuman) Aug. 4, 2023

Aug. 4, 2023

Today, Wine fails to build with musl libc and Linux kernel 5.14 uAPI header files. musl libc doesn't supply any definitions for IPX, such as the SOL_IPX macro. However, it still provides linux/ipx.h from Linux uAPI header files if it exists. Linux kernel wouldn't drop linux/ipx.h from uAPI headers until 5.15, although IPX support has already been marked obsolete since 2018. Fix this by not defining HAS_IPX if linux/ipx.h has been included but nothing defines the SOL_IPX macro. Status of IPX support from other libcs are noted below: - bionic: netipx/ipx.h does not exist. linux/ipx.h may or may not exist. Note that sys/socket.h defines SOL_IPX even if linux/ipx.h is missing. - glibc: netipx/ipx.h exists. In this case, Wine assumes IPX support even if the operating system does not support it in runtime. - BSD variants: netipx/ipx.h may or may not exist. linux/ipx.h does not exist. Some BSDs supply SO_DEFAULT_HEADERS instead of SOL_IPX. -- v2: server: Avoid relying on linux/ipx.h to define SOL_IPX. ws2_32: Avoid relying on linux/ipx.h to define SOL_IPX. ntdll: Avoid relying on linux/ipx.h to define SOL_IPX. https://gitlab.winehq.org/wine/wine/-/merge_requests/3428

4 5

Re: [PATCH v3 0/1] MR3491: winegstreamer: Leave media source critical section before unlocking workqueue.
by Zebediah Figura (＠zfigura) Aug. 4, 2023

Aug. 4, 2023

> We could maybe grab the Queue id in the cs, 0 it, and use the local copy for unlocking. Does that sound better? That does seem like an improvement; I can't say I see a better way to solve the problem after looking, at least. We should probably mention in a comment why we're releasing outside of the lock; it is not obvious at all from the documentation that MFUnlockWorkQueue() can block. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3491#note_41430

1 0

Re: [PATCH v3 0/1] MR3491: winegstreamer: Leave media source critical section before unlocking workqueue.
by Bernhard Kölbl Aug. 4, 2023

Aug. 4, 2023

On Fri Aug 4 20:16:23 2023 +0000, Zebediah Figura wrote: > > It doesn't matter much if the handle is invalid, as the PutWorkItem > call will just fail. > But the handle could in theory be reallocated for a different queue. > Also, if queues are supposed to be refcounted, it looks quite wrong that > a thread which doesn't have a reference to a handle is accessing it; > that's the sort of thing that generally causes memory errors. > It might be the case that our current implementation is preventing any > of this from being a problem in practice, but it's fragile and > unidiomatic, and it is rather confusing to anyone reading the code; it > looks like an error. We could maybe grab the Queue id in the cs, 0 it, and use the local copy for unlocking. Does that sound better? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3491#note_41426

1 0

Re: [PATCH v3 0/1] MR3491: winegstreamer: Leave media source critical section before unlocking workqueue.
by Zebediah Figura (＠zfigura) Aug. 4, 2023

Aug. 4, 2023

> It doesn't matter much if the handle is invalid, as the PutWorkItem call will just fail. But the handle could in theory be reallocated for a different queue. Also, if queues are supposed to be refcounted, it looks quite wrong that a thread which doesn't have a reference to a handle is accessing it; that's the sort of thing that generally causes memory errors. It might be the case that our current implementation is preventing any of this from being a problem in practice, but it's fragile and unidiomatic, and it is rather confusing to anyone reading the code; it looks like an error. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3491#note_41425

1 0

Re: [PATCH v3 0/1] MR3491: winegstreamer: Leave media source critical section before unlocking workqueue.
by Bernhard Kölbl Aug. 4, 2023

Aug. 4, 2023

On Fri Aug 4 19:58:51 2023 +0000, Zebediah Figura wrote: > Not sure why I wasn't added as reviewer, but this looks wrong, and I > think this doesn't fully solve the problem, either. > As far as I can tell, if there's something else [what?] that pushes to > the queue executing concurrently with Shutdown(), it'll end up pushing > to an invalid queue handle after Shutdown() returns. This was already > the case, but now it's worse, because you can't just fix it by setting > source->async_commands_queue to NULL. 1. You were added as reviewer. 2. It doesn't matter much if the handle is invalid, as the PutWorkItem call will just fail. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3491#note_41424

1 0

[PATCH 0/1] MR3469: d3dx11_43/tests: Add a trailing '\n' to ok() calls.
by Alistair Leslie-Hughes (＠alesliehughes) Aug. 4, 2023

Aug. 4, 2023

-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3469

3 2

[PATCH 0/4] MR3514: sapi: CoInitializeEx fix and trace improvement.
by Shaun Ren (＠shaunren) Aug. 4, 2023

Aug. 4, 2023

This MR fixes a bug in Far Cry 6 where the necessary objects cannot be initialized in its speech synthesis thread. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3514

3 5

[PATCH v2 0/1] MR3507: win32u: Copy an incoming CB_GETCOMBOBOXINFO when packing it for the client.
by Tim Clem (＠tclem) Aug. 4, 2023

Aug. 4, 2023

The only important field at this point is cbSize, but if it's zero, the message will not be processed. --- The other option would be to zero the buffer and then do the equivalent of `((COMBOBOXINFO *)buffer)->cbSize = sizeof(COMBOBOXINFO)`, rather than copying the incoming lparam. That doesn't seem better to me, but I'm no expert. -- v2: win32u: Copy an incoming COMBOBOXINFO when packing CB_GETCOMBOBOXINFO for clients. https://gitlab.winehq.org/wine/wine/-/merge_requests/3507

3 4

Re: [PATCH v3 0/5] MR3498: d3d tests: Use compare_uint() in compare_float() instead of abs(). - approved
by Zebediah Figura (＠zfigura) Aug. 4, 2023

Aug. 4, 2023

This merge request was approved by Zebediah Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3498

1 0