On Thu, Aug 17, 2017 at 11:14 AM, Nikolay Sivov bunglehead@gmail.com wrote:
On 17.08.2017 10:40, Sebastian Lackner wrote:
On 17.08.2017 07:11, Anton Romanov wrote:
On Wed, Aug 16, 2017 at 10:05 PM, Anton Romanov theli.ua@gmail.com wrote:
On Tue, Aug 15, 2017 at 4:40 AM, Nikolay Sivov nsivov@codeweavers.com wrote:
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com
dlls/dwrite/dwrite_private.h | 15 ++++++++--- dlls/dwrite/font.c | 30 +++++++++++++-------- dlls/dwrite/main.c | 63 +++++++++++++++++++++++++++++++------------- 3 files changed, 74 insertions(+), 34 deletions(-)
So I dived a bit deeper into that and it looks like there is now a race for fontfile:
004c:trace:dwrite:dwritefontface_Release (0x189c09a0)->(0) 005c:trace:dwrite:dwritefontface_GetSimulations (0x189c09a0) 005c:trace:dwrite:dwritefontface_GetFiles (0x189c09a0)->(0xa5fe400 0xa5fe408) 005c:trace:dwrite:dwritefontfile_AddRef (0x15cf7f00)->(3) 005c:trace:dwrite:dwritefontfile_GetReferenceKey (0x15cf7f00)->(0xa5fe404, 0xa5fe3fc) 005c:trace:dwrite:dwritefontfile_Release (0x15cf7f00)->(2) 005c:trace:dwrite:dwritefontface_QueryInterface (0x189c09a0)->({27f2a904-4eb8-441d-9678-0563f53e3e2f} 0xa5fe57c) 005c:warn:dwrite:factory_get_cached_fontface Failed to get {27f2a904-4eb8-441d-9678-0563f53e3e2f} from fontface, hr 0x80004005. 005c:trace:dwrite:factory_get_cached_fontface returning cached fontface 0x189c09a0 005c:trace:dwrite:shareddwritefactory_AddRef (0x3a0cf18) 004c:trace:dwrite_file:localfontfilestream_Release (0x1633b128)->(0) 005c:trace:dwrite:dwritefontfile_GetLoader (0x15cf7f00)->(0xa5fe408) 005c:trace:dwrite:localfontfileloader_AddRef (0x15cf5108)->(225) 005c:trace:dwrite:dwritefontfile_GetReferenceKey (0x15cf7f00)->(0xa5fe410, 0xa5fe40c) 005c:trace:dwrite:localfontfileloader_CreateStreamFromKey (0x15cf5108)->(0x15cf8fc0, 64, 0x1633aff8) 005c:trace:dwrite:localfontfileloader_CreateStreamFromKey name: L"C:\windows\fonts\Ariali.TTF" 005c:trace:dwrite_file:localfontfilestream_AddRef (0x1633b128)->(1) 005c:trace:dwrite:localfontfileloader_Release (0x15cf5108)->(224) 005c:trace:dwrite:get_stream_from_file 0x15cf7f00 -> 0x1633b128 005c:trace:dwrite:dwritefontfile_AddRef (0x15cf7f00)->(3) 005c:trace:dwrite_file:localfontfilestream_ReadFileFragment (0x1633b128)->(0xa5fe318, 0x0, 0xc, 0xa5fe31c)
The crash is in thread 5c with backtrace being: =>0 0x7d442918 opentype_get_font_table+0xa8(stream_desc=0xa5fe410, tag=0x322f534f, table_data=0xa5fe39c, table_context=0xa5fe38c, table_size=0x0(nil), found=0x0(nil)) [/home/theli/Projects/wine-build/32/dlls/dwrite/../../../../wine/dlls/dwrite/opentype.c:1075] in dwrite (0x0a5fe338) 1 0x7d442f0e opentype_get_font_metrics+0x5d(stream_desc=<is not available>, metrics=<is not available>, caret=<is not available>) [/home/theli/Projects/wine-build/32/dlls/dwrite/../../../../wine/dlls/dwrite/opentype.c:1209] in dwrite (0x0a5fe3b8) 2 0x7d42a089 create_fontface+0x2c8(desc=0xa5fe470, cached_list=0x3a0cf34, ret=0xa5fe57c) [/home/theli/Projects/wine-build/32/dlls/dwrite/../../../../wine/dlls/dwrite/font.c:4354] in dwrite (0x0a5fe438) 3 0x7d42a396 get_fontface_from_font.isra+0xb5() in dwrite (0x0a5fe4a8) 4 0x7d42a420 dwritefont3_CreateFontFace+0x5f(iface=<couldn't compute location>, fontface=<couldn't compute location>) [/home/theli/Projects/wine-build/32/dlls/dwrite/../../../../wine/dlls/dwrite/font.c:1674] in dwrite (0x0a5fe4d8) 5 0x7d41d42b dwritefont_CreateFontFace+0x5a(iface=<couldn't compute location>, fontface=<couldn't compute location>) [/home/theli/Projects/wine-build/32/dlls/dwrite/../../include/dwrite_3.h:1208] in dwrite (0x0a5fe518)
And the reason it didn't crash with v1 is that the whole chain of releases (from fontface to factory, file, etc) was protected by lock. While this version releases the lock after removing fontface from the cache.
Also with this patch the line TRACE("returning cached fontface %p\n", cached->fontface); Should probably be under the else branch as it outputs this regardless of whether it actually acquired a reference.