[v2 PATCH 1/2] dwrite: Protect cached fontface list when accessed from multiple threads

Nikolay Sivov nsivov at codeweavers.com
Thu Aug 10 01:23:52 CDT 2017


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(-)

diff --git a/dlls/dwrite/dwrite_private.h b/dlls/dwrite/dwrite_private.h
index 57731a9069..ae9cf543fd 100644
--- a/dlls/dwrite/dwrite_private.h
+++ b/dlls/dwrite/dwrite_private.h
@@ -189,8 +189,8 @@ extern HRESULT create_matching_font(IDWriteFontCollection*,const WCHAR*,DWRITE_F
     IDWriteFont**) DECLSPEC_HIDDEN;
 extern HRESULT create_fontfacereference(IDWriteFactory5*,IDWriteFontFile*,UINT32,DWRITE_FONT_SIMULATIONS,
     IDWriteFontFaceReference**) DECLSPEC_HIDDEN;
-extern HRESULT factory_get_cached_fontface(IDWriteFactory5*,IDWriteFontFile*const*,UINT32,DWRITE_FONT_SIMULATIONS,IDWriteFontFace**,
-    struct list**) DECLSPEC_HIDDEN;
+extern HRESULT factory_get_cached_fontface(IDWriteFactory5*,IDWriteFontFile*const*,UINT32,DWRITE_FONT_SIMULATIONS,
+        struct list**,REFIID,void**) DECLSPEC_HIDDEN;
 extern void factory_detach_fontcollection(IDWriteFactory5*,IDWriteFontCollection1*) DECLSPEC_HIDDEN;
 extern void factory_detach_gdiinterop(IDWriteFactory5*,IDWriteGdiInterop1*) DECLSPEC_HIDDEN;
 extern struct fontfacecached *factory_cache_fontface(struct list*,IDWriteFontFace4*) DECLSPEC_HIDDEN;
@@ -199,6 +199,8 @@ extern void    get_logfont_from_font(IDWriteFont*,LOGFONTW*) DECLSPEC_HIDDEN;
 extern void    get_logfont_from_fontface(IDWriteFontFace*,LOGFONTW*) DECLSPEC_HIDDEN;
 extern HRESULT create_gdiinterop(IDWriteFactory5*,IDWriteGdiInterop1**) DECLSPEC_HIDDEN;
 extern void fontface_detach_from_cache(IDWriteFontFace4*) DECLSPEC_HIDDEN;
+extern void factory_lock(IDWriteFactory5*) DECLSPEC_HIDDEN;
+extern void factory_unlock(IDWriteFactory5*) DECLSPEC_HIDDEN;
 
 /* Opentype font table functions */
 struct dwrite_font_props {
diff --git a/dlls/dwrite/font.c b/dlls/dwrite/font.c
index be0131ac75..9002e7269c 100644
--- a/dlls/dwrite/font.c
+++ b/dlls/dwrite/font.c
@@ -482,11 +482,14 @@ static ULONG WINAPI dwritefontface_AddRef(IDWriteFontFace4 *iface)
 static ULONG WINAPI dwritefontface_Release(IDWriteFontFace4 *iface)
 {
     struct dwrite_fontface *This = impl_from_IDWriteFontFace4(iface);
-    ULONG ref = InterlockedDecrement(&This->ref);
+    IDWriteFactory5 *factory = This->factory;
+    ULONG ref;
 
-    TRACE("(%p)->(%d)\n", This, ref);
+    TRACE("(%p)->(%d)\n", This, This->ref - 1);
 
-    if (!ref) {
+    factory_lock(factory);
+
+    if ((ref = InterlockedDecrement(&This->ref)) == 0) {
         UINT32 i;
 
         if (This->cmap.context)
@@ -514,11 +517,14 @@ static ULONG WINAPI dwritefontface_Release(IDWriteFontFace4 *iface)
         freetype_notify_cacheremove(iface);
         if (This->cached)
             factory_release_cached_fontface(This->cached);
-        if (This->factory)
-            IDWriteFactory5_Release(This->factory);
         heap_free(This);
     }
 
+    factory_unlock(factory);
+
+    if (ref == 0)
+        IDWriteFactory5_Release(factory);
+
     return ref;
 }
 
@@ -1378,11 +1384,9 @@ static HRESULT get_fontface_from_font(struct dwrite_font *font, IDWriteFontFace4
     *fontface = NULL;
 
     hr = factory_get_cached_fontface(font->family->collection->factory, &data->file, data->face_index,
-        font->data->simulations, (IDWriteFontFace **)fontface, &cached_list);
-    if (hr == S_OK) {
-        IDWriteFontFace4_AddRef(*fontface);
+            font->data->simulations, &cached_list, &IID_IDWriteFontFace4, (void **)fontface);
+    if (hr == S_OK)
         return hr;
-    }
 
     desc.factory = font->family->collection->factory;
     desc.face_type = data->face_type;
@@ -4325,6 +4329,7 @@ HRESULT create_fontface(const struct fontface_desc *desc, struct list *cached_li
     fontface->colr.exists = TRUE;
     fontface->index = desc->index;
     fontface->simulations = desc->simulations;
+    IDWriteFactory5_AddRef(fontface->factory = desc->factory);
 
     for (i = 0; i < fontface->file_count; i++) {
         hr = get_stream_from_file(desc->files[i], &fontface->streams[i]);
@@ -4393,7 +4398,6 @@ HRESULT create_fontface(const struct fontface_desc *desc, struct list *cached_li
     }
 
     fontface->cached = factory_cache_fontface(cached_list, &fontface->IDWriteFontFace4_iface);
-    IDWriteFactory5_AddRef(fontface->factory = desc->factory);
 
     *ret = &fontface->IDWriteFontFace4_iface;
     return S_OK;
diff --git a/dlls/dwrite/main.c b/dlls/dwrite/main.c
index 36d8613f1e..6de1f5e69f 100644
--- a/dlls/dwrite/main.c
+++ b/dlls/dwrite/main.c
@@ -553,6 +553,8 @@ struct dwritefactory {
 
     struct list collection_loaders;
     struct list file_loaders;
+
+    CRITICAL_SECTION cs;
 };
 
 static inline struct dwritefactory *impl_from_IDWriteFactory5(IDWriteFactory5 *iface)
@@ -586,7 +588,10 @@ static void release_dwritefactory(struct dwritefactory *factory)
 
     if (factory->localfontfileloader)
         IDWriteLocalFontFileLoader_Release(factory->localfontfileloader);
+
+    EnterCriticalSection(&factory->cs);
     release_fontface_cache(&factory->localfontfaces);
+    LeaveCriticalSection(&factory->cs);
 
     LIST_FOR_EACH_ENTRY_SAFE(loader, loader2, &factory->collection_loaders, struct collectionloader, entry) {
         list_remove(&loader->entry);
@@ -603,6 +608,9 @@ static void release_dwritefactory(struct dwritefactory *factory)
         IDWriteFontCollection1_Release(factory->eudc_collection);
     if (factory->fallback)
         release_system_fontfallback(factory->fallback);
+
+    factory->cs.DebugInfo->Spare[0] = 0;
+    DeleteCriticalSection(&factory->cs);
     heap_free(factory);
 }
 
@@ -818,8 +826,20 @@ static HRESULT WINAPI dwritefactory_CreateCustomFontFileReference(IDWriteFactory
     return create_font_file(loader, reference_key, key_size, font_file);
 }
 
-HRESULT factory_get_cached_fontface(IDWriteFactory5 *iface, IDWriteFontFile * const *font_files,
-    UINT32 index, DWRITE_FONT_SIMULATIONS simulations, IDWriteFontFace **font_face, struct list **cached_list)
+void factory_lock(IDWriteFactory5 *iface)
+{
+    struct dwritefactory *factory = impl_from_IDWriteFactory5(iface);
+    EnterCriticalSection(&factory->cs);
+}
+
+void factory_unlock(IDWriteFactory5 *iface)
+{
+    struct dwritefactory *factory = impl_from_IDWriteFactory5(iface);
+    LeaveCriticalSection(&factory->cs);
+}
+
+HRESULT factory_get_cached_fontface(IDWriteFactory5 *iface, IDWriteFontFile * const *font_files, UINT32 index,
+        DWRITE_FONT_SIMULATIONS simulations, struct list **cached_list, REFIID riid, void **obj)
 {
     struct dwritefactory *factory = impl_from_IDWriteFactory5(iface);
     struct fontfacecached *cached;
@@ -829,7 +849,7 @@ HRESULT factory_get_cached_fontface(IDWriteFactory5 *iface, IDWriteFontFile * co
     const void *key;
     HRESULT hr;
 
-    *font_face = NULL;
+    *obj = NULL;
     *cached_list = NULL;
 
     hr = IDWriteFontFile_GetReferenceKey(*font_files, &key, &key_size);
@@ -854,6 +874,8 @@ HRESULT factory_get_cached_fontface(IDWriteFactory5 *iface, IDWriteFontFile * co
 
     *cached_list = fontfaces;
 
+    EnterCriticalSection(&factory->cs);
+
     /* search through cache list */
     LIST_FOR_EACH_ENTRY(cached, fontfaces, struct fontfacecached, entry) {
         UINT32 cached_key_size, count = 1, cached_face_index;
@@ -870,21 +892,24 @@ HRESULT factory_get_cached_fontface(IDWriteFactory5 *iface, IDWriteFontFile * co
 
         hr = IDWriteFontFace4_GetFiles(cached->fontface, &count, &file);
         if (FAILED(hr))
-            return hr;
+            break;
 
         hr = IDWriteFontFile_GetReferenceKey(file, &cached_key, &cached_key_size);
         IDWriteFontFile_Release(file);
         if (FAILED(hr))
-            return hr;
+            break;
 
         if (cached_key_size == key_size && !memcmp(cached_key, key, key_size)) {
             TRACE("returning cached fontface %p\n", cached->fontface);
-            *font_face = (IDWriteFontFace*)cached->fontface;
-            return S_OK;
+            if (FAILED(hr = IDWriteFontFace4_QueryInterface(cached->fontface, riid, obj)))
+                WARN("Failed to get %s from fontface, hr %#x.\n", debugstr_guid(riid), hr);
+            break;
         }
     }
 
-    return S_FALSE;
+    LeaveCriticalSection(&factory->cs);
+
+    return *obj ? S_OK : S_FALSE;
 }
 
 struct fontfacecached *factory_cache_fontface(struct list *fontfaces, IDWriteFontFace4 *fontface)
@@ -948,10 +973,8 @@ static HRESULT WINAPI dwritefactory_CreateFontFace(IDWriteFactory5 *iface, DWRIT
     if (face_type != req_facetype)
         return DWRITE_E_FILEFORMAT;
 
-    hr = factory_get_cached_fontface(iface, font_files, index, simulations, fontface, &fontfaces);
-    if (hr == S_OK)
-        IDWriteFontFace_AddRef(*fontface);
-
+    hr = factory_get_cached_fontface(iface, font_files, index, simulations, &fontfaces,
+            &IID_IDWriteFontFace, (void **)fontface);
     if (hr != S_FALSE)
         return hr;
 
@@ -1749,6 +1772,9 @@ static void init_dwritefactory(struct dwritefactory *factory, DWRITE_FACTORY_TYP
     list_init(&factory->collection_loaders);
     list_init(&factory->file_loaders);
     list_init(&factory->localfontfaces);
+
+    InitializeCriticalSection(&factory->cs);
+    factory->cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": dwritefactory.lock");
 }
 
 void factory_detach_fontcollection(IDWriteFactory5 *iface, IDWriteFontCollection1 *collection)
-- 
2.13.2




More information about the wine-patches mailing list