On 10.08.2017 08:45, Nikolay Sivov wrote:
Based on patch by Anton Romanov <theli.ua(a)gmail.com>
Signed-off-by: Nikolay Sivov <nsivov(a)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#l2... Best regards, Sebastian