[v3 PATCH 1/2] dwrite: Protect cached fontface list when accessed from multiple threads
Nikolay Sivov
bunglehead at gmail.com
Thu Aug 10 08:38:50 CDT 2017
On 10.08.2017 11:14, Sebastian Lackner wrote:
> 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()?
It is.
>
> 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 ---
Yes, that's better I guess, thanks.
>
> 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 ---
In my case, to avoid exposing structure, it's enough to AddRef() == 1,
and release otherwise I think. It's not important to decrement when
we're in a process of destroying the object.
>
> 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