[v3 PATCH 1/2] dwrite: Protect cached fontface list when accessed from multiple threads
Sebastian Lackner
sebastian at fds-team.de
Thu Aug 10 03:14:09 CDT 2017
On 10.08.2017 08:45, Nikolay Sivov wrote:
> Based on patch by Anton Romanov <theli.ua at gmail.com>
>
> Signed-off-by: Nikolay Sivov <nsivov at codeweavers.com>
> ---
> dlls/dwrite/dwrite_private.h | 6 ++++--
> dlls/dwrite/font.c | 24 ++++++++++++---------
> dlls/dwrite/main.c | 50 +++++++++++++++++++++++++++++++++-----------
> 3 files changed, 56 insertions(+), 24 deletions(-)
>
Isn't it also necessary to protect factory_cache_fontface()?
By the way: An alternative method to implement proper synchronization (which does
not require locking critical sections before InterlockedDecrement) would be to do
something like:
In the destructor:
--- snip ---
if (!ref)
{
EnterCriticalSection(...);
list_remove(...);
LeaveCriticalSection(...);
...
}
--- snip ---
And when querying cached font faces:
--- snip ---
EnterCriticalSection(...);
LIST_FOR_EACH_ENTRY(...)
{
...
/* the following code could also be hidden in QueryInterface */
if (InterlockedIncrement( &obj->refcount ) == 1)
{
InterlockedDecrement( &obj->refcount );
continue;
}
/* return it */
}
LeaveCriticalSection(...);
--- snip ---
This is also what is done in Wines threadpool implementation [1], and has the
advantage that the CS is not needed for every Release, only when the object is
really destroyed. But in the end it is a matter of taste, for rpcrt4 Jacek
also picked the "hold CS during InterlockedDecrement" method.
[1] https://source.winehq.org/git/wine.git/blob/HEAD:/dlls/ntdll/threadpool.c#l2564
Best regards,
Sebastian
More information about the wine-devel
mailing list