> 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
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
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
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.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/3491#note_41413