There is a race condition otherwise between release_gdi_font and find_cached_gdi_font, leading to invalid memory access:
One thread calling release_gdi_font may decrement refcount to 0, then try to enter font_lock. At the same time, another thread may be calling find_cached_gdi_font through select_font, holding the font_lock.
This second thread would find refcount set to 0, and then try to remove unused_entry from its list, although it hasn't been added yet to the unused list.
Signed-off-by: Rémi Bernon rbernon@codeweavers.com ---
The crash can be easily reproduced by running a simple program which only creates a window then exits, in a loop, shutting down the prefix every time. It randomly causes a segmentation fault.
For some reason, this race condition is only present after fd675485be8686ec5e3b4667d3f3e9739a4a07ce, possibly because of the regression described in next patch.
dlls/win32u/font.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/dlls/win32u/font.c b/dlls/win32u/font.c index d2f61526540..f9a0359e92c 100644 --- a/dlls/win32u/font.c +++ b/dlls/win32u/font.c @@ -2576,22 +2576,24 @@ static struct gdi_font *find_cached_gdi_font( const LOGFONTW *lf, const FMAT2 *m static void release_gdi_font( struct gdi_font *font ) { if (!font) return; - if (--font->refcount) return;
TRACE( "font %p\n", font );
/* add it to the unused list */ pthread_mutex_lock( &font_lock ); - list_add_head( &unused_gdi_font_list, &font->unused_entry ); - if (unused_font_count > UNUSED_CACHE_SIZE) + if (!--font->refcount) { - font = LIST_ENTRY( list_tail( &unused_gdi_font_list ), struct gdi_font, unused_entry ); - TRACE( "freeing %p\n", font ); - list_remove( &font->entry ); - list_remove( &font->unused_entry ); - free_gdi_font( font ); + list_add_head( &unused_gdi_font_list, &font->unused_entry ); + if (unused_font_count > UNUSED_CACHE_SIZE) + { + font = LIST_ENTRY( list_tail( &unused_gdi_font_list ), struct gdi_font, unused_entry ); + TRACE( "freeing %p\n", font ); + list_remove( &font->entry ); + list_remove( &font->unused_entry ); + free_gdi_font( font ); + } + else unused_font_count++; } - else unused_font_count++; pthread_mutex_unlock( &font_lock ); }