- Wine-GitLab - list.winehq.org

Re: [PATCH v2 0/1] MR9234: ntdll: Call cache_inproc_sync() inside the lock.
by Elizabeth Figura (＠zfigura) Oct. 20, 2025

Oct. 20, 2025

> Hmm... If the cache update is outside of the lock, more than one thread might have retrieved the sync and try to update the cache concurrently. > > I think in the current state this will break as multiple thread may pass the `if (InterlockedCompareExchange( &cache->refcount, 0, 0 ))` check successfully. Oh duh, I can't read. I'll adjust this patch to get rid of the atomic cmpxchg mapping cache memory as we indeed don't need it anymore with the lock. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9234#note_119104

1 0

Re: [PATCH v2 0/1] MR9234: ntdll: Call cache_inproc_sync() inside the lock.
by Rémi Bernon Oct. 20, 2025

Oct. 20, 2025

On Mon Oct 20 21:11:35 2025 +0000, Rémi Bernon wrote: > Hmm... If the cache update is outside of the lock, more than one thread > might have retrieved the sync and try to update the cache concurrently. > I think in the current state this will break as multiple thread may pass > the `if (InterlockedCompareExchange( &cache->refcount, 0, 0 ))` check successfully. With a separate lock, we still have to only allow one of the thread to cache its sync, and let the others on the stack. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9234#note_119102

1 0

Re: [PATCH v2 0/1] MR9234: ntdll: Call cache_inproc_sync() inside the lock.
by Rémi Bernon Oct. 20, 2025

Oct. 20, 2025

On Mon Oct 20 21:05:58 2025 +0000, Elizabeth Figura wrote: > > Nvm, I guess the entire sync retrieval and caching needs to be atomic. > Does it? I'm not sure I see how; I think you're right that the mmap > doesn't need to be in the lock. I don't know that it's really important, > given how rare it'll be in practice, but theoretically it could be > outside the lock. Hmm... If the cache update is outside of the lock, more than one thread might have retrieved the sync and try to update the cache concurrently. I think in the current state this will break as multiple thread may pass the `if (InterlockedCompareExchange( &cache->refcount, 0, 0 ))` check successfully. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9234#note_119101

1 0

Re: [PATCH v2 0/1] MR9234: ntdll: Call cache_inproc_sync() inside the lock.
by Elizabeth Figura (＠zfigura) Oct. 20, 2025

Oct. 20, 2025

> Nvm, I guess the entire sync retrieval and caching needs to be atomic. Does it? I'm not sure I see how; I think you're right that the mmap doesn't need to be in the lock. I don't know that it's really important, given how rare it'll be in practice, but theoretically it could be outside the lock. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9234#note_119099

1 0

Re: [PATCH v2 0/1] MR9234: ntdll: Call cache_inproc_sync() inside the lock.
by Rémi Bernon Oct. 20, 2025

Oct. 20, 2025

On Mon Oct 20 20:56:40 2025 +0000, Rémi Bernon wrote: > Sorry to have missed that. > Fwiw I think the lock (and probably a different one, there's no need to > lock the fd cache) should be taken again within `cache_inproc_sync` but > only for the things where it's actually necessary. > It does rather costly calls, like `mmap` and there's things like > `InterlockedCompareExchangePointer` in it which are rather pointless otherwise. Nvm, I guess the entire sync retrieval and caching needs to be atomic. Anyway, still I think the compare exchange looks confusing if it always happens inside a lock. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9234#note_119098

1 0

[PATCH 0/1] MR9235: Draft: winex11: Retrieve XIM result as UTF8.
by Bernhard Kölbl Oct. 20, 2025

Oct. 20, 2025

The goal here is to make XIM input locale independent, so changing LC_MESSAGES and LC_CTYPE (to change code pages on PE side) can be done without breaking the IME, when set locale isn't set up on the host system. To achieve this, I switched XmbLookupString to Xutf8LookupString, because it *seems* to achieve what I wrote above. Marked as draft because idk if this makes sense at all. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9235

2 2

Re: [PATCH v2 0/1] MR9234: ntdll: Call cache_inproc_sync() inside the lock.
by Rémi Bernon Oct. 20, 2025

Oct. 20, 2025

Sorry to have missed that. Fwiw I think the lock (and probably a different one, there's no need to lock the fd cache) should be taken again within `cache_inproc_sync` but only for the things where it's actually necessary. It does rather costly calls, like `mmap` and there's things like `InterlockedCompareExchangePointer` in it which are rather pointless otherwise. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9234#note_119096

1 0

Re: [PATCH v2 0/1] MR9234: ntdll: Call cache_inproc_sync() inside the lock.
by Elizabeth Figura (＠zfigura) Oct. 20, 2025

Oct. 20, 2025

The original revision submitted as !7226 did not have this bug; it was introduced in !9091. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9234#note_119091

1 0

[PATCH 0/1] MR9234: ntdll: Call cache_inproc_sync() inside the lock.
by Elizabeth Figura (＠zfigura) Oct. 20, 2025

Oct. 20, 2025

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

2 1

[PATCH 0/1] MR9233: user32: Move GetWindowDisplayAffinity() stub to win32u.
by Paul Gofman (＠gofman) Oct. 20, 2025

Oct. 20, 2025

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

2 1