Skip to content

Commit b762cd1

Browse files
committed
GFX/FONT: Route font_driver_free through video thread when threaded video active
font_driver_free calls renderer->free (e.g. d3d12_font_free, gl2_raster_font_free, d3d11_font_free) directly on the calling thread. Menu drivers call this from context_destroy or context_reset_internal on the main thread, while the video thread is still rendering frames. This produces driver-specific races: GL1/GL2/GL3: The GL context can only be current on one thread. gl2_raster_font_free calls make_current(true) to steal the context from the video thread, then calls glDeleteTextures. If the video thread is mid-frame, its subsequent GL calls are invalid until context is restored. D3D11: The ImmediateContext is explicitly not thread-safe per Microsoft's docs. d3d11_font_free calls Release on the font texture's COM objects while the video thread draws with the same context — undefined behaviour. D3D10: Same as D3D11, though slightly less severe because D3D10's device has an internal mutex. Release still races with pending GPU draws. D3D12: d3d12_font_free uses a GPU fence wait with ++fenceValue. The non-atomic increment from the main thread races with the video thread's own fence signalling, potentially skipping or duplicating fence values. Vulkan: vulkan_destroy_texture (called from font free) uses vkQueueWaitIdle under queue_lock, which drains submitted work but not command buffers currently being recorded on the video thread. Metal: Blit command buffer access can race (same as texture load). However, font glyph textures in the Metal driver use replaceRegion: (thread-safe) rather than the blit command encoder, so this is lower risk. Fix: in font_driver_free, when threaded video is active, dispatch the renderer->free call to the video thread via video_thread_texture_handle. This serialises the font resource destruction with the video thread's frame rendering, the same pattern used for texture load/unload. This is a single-point fix in font_driver.c that covers ALL video driver backends at once, rather than patching each driver's font_free individually. The font_driver_free function is the natural place for this because it already checks is_threaded and is the single entry point for all font destruction. Implementation: - font_free_cmd_t packages the renderer pointer, renderer_data, and is_threaded flag. - font_driver_free_wrap is the CMD_CUSTOM_COMMAND callback. Calls renderer->free on the video thread. - When is_threaded is true and renderer->free exists, dispatch via video_thread_texture_handle. The font_data_t struct itself (renderer/renderer_data pointers + malloc'd memory) is cleaned up on the main thread after the dispatch returns -- only the GPU-touching renderer->free needs serialisation. - video_thread_texture_handle is self-safe (falls back to direct call when the wrapper is not active, and handles same-thread calls without deadlock). - Non-threaded path is unchanged. This fixes the mid-session font scale change crash on D3D12 that was previously only mitigated by XMB's pending_context_reset counter. It also eliminates the GL context-stealing race in gl2_raster_font_free. Depends on: 8d55232 (VIDEO/THREAD: barrier + wrapper hardening). No behavioural change on non-threaded video.
1 parent ee3eae3 commit b762cd1

1 file changed

Lines changed: 60 additions & 0 deletions

File tree

gfx/font_driver.c

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -901,6 +901,23 @@ int font_driver_get_line_centre_offset(font_data_t *font, float scale)
901901
return (int)roundf((1.58f * 0.5f * (float)font_driver_get_message_width(font, "a", 1, scale) / 0.6f) / 2.0f);
902902
}
903903

904+
#ifdef HAVE_THREADS
905+
typedef struct
906+
{
907+
const font_renderer_t *renderer;
908+
void *renderer_data;
909+
bool is_threaded;
910+
} font_free_cmd_t;
911+
912+
static int font_driver_free_wrap(void *data)
913+
{
914+
font_free_cmd_t *cmd = (font_free_cmd_t*)data;
915+
if (cmd->renderer && cmd->renderer->free)
916+
cmd->renderer->free(cmd->renderer_data, cmd->is_threaded);
917+
return 0;
918+
}
919+
#endif
920+
904921
void font_driver_free(font_data_t *font)
905922
{
906923
if (font)
@@ -913,6 +930,49 @@ void font_driver_free(font_data_t *font)
913930
#endif
914931
renderer = font ? font->renderer : NULL;
915932

933+
#ifdef HAVE_THREADS
934+
/* When threaded video is active, font resources (GPU
935+
* textures, GL names, D3D COM objects) belong to the
936+
* video thread's rendering context. Freeing them on
937+
* the main thread races with the video thread's draw
938+
* calls:
939+
*
940+
* - GL: context is single-threaded; gl2_raster_font_free
941+
* calls make_current to steal the context, but the video
942+
* thread may be mid-frame.
943+
* - D3D11: ImmediateContext is not thread-safe; Release on
944+
* the main thread while the video thread draws is UB.
945+
* - D3D12: fenceValue++ from the main thread races with
946+
* the video thread's own fence signalling.
947+
* - Vulkan: vkQueueWaitIdle under queue_lock only drains
948+
* submitted work, not command buffers being recorded.
949+
*
950+
* Dispatch renderer->free to the video thread via
951+
* video_thread_texture_handle so it runs serialised with
952+
* the video thread's frame rendering. This is the same
953+
* pattern used by texture load/unload.
954+
*
955+
* video_thread_texture_handle is self-safe: if the wrapper
956+
* is not active (VIDEO_FLAG_THREAD_WRAPPER_ACTIVE not set),
957+
* it falls back to calling func(data) on the current thread.
958+
* If called from the video thread itself, it calls func
959+
* directly (no deadlock). */
960+
if (is_threaded && renderer && renderer->free)
961+
{
962+
font_free_cmd_t cmd;
963+
cmd.renderer = renderer;
964+
cmd.renderer_data = font->renderer_data;
965+
cmd.is_threaded = is_threaded;
966+
video_thread_texture_handle(&cmd, font_driver_free_wrap);
967+
968+
font->renderer = NULL;
969+
font->renderer_data = NULL;
970+
971+
free(font);
972+
return;
973+
}
974+
#endif
975+
916976
if (renderer && renderer->free)
917977
renderer->free(font->renderer_data, is_threaded);
918978

0 commit comments

Comments
 (0)