From: Nikolay Sivov Subject: [PATCH 1/2] dwrite: Protect cached fontface list when accessed from multiple threads Message-Id: <20170810055215.18757-1-nsivov@codeweavers.com> Date: Thu, 10 Aug 2017 08:52:14 +0300 Signed-off-by: Nikolay Sivov --- 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