Skip to content

Commit 9441128

Browse files
committed
gfx/font_driver: plug renderer-state leak on OOM in font_driver_init_first
font_driver_init_first starts by calling font_init_first (or video_thread_font_init in the threaded-video case), which on success populates font_driver and font_handle. That handle owns the raster font's allocated state - glyph atlas texture, GL names, D3D COM objects, etc. - allocated and held inside the chosen renderer (gl2_raster_font / d3d11_font / vulkan_raster_font / ...). Only after that succeeds do we malloc the small wrapper struct (font_data_t). Before this commit, if that wrapper malloc failed, the function returned NULL directly and the entire raster-font state was orphaned with no path to free it. Cores that hit OOM during font init would leak that state on every failed attempt. Also refactors font_driver_free's renderer-side teardown into a shared helper, font_driver_release_renderer_state, so both the normal teardown path and the new OOM cleanup path handle the threaded-vs-non-threaded dispatch the same way. This matters: renderer->free cannot be called on the main thread when threaded video is active, because it touches GPU state owned by the video thread (GL context, D3D ImmediateContext, D3D12 fence, Vulkan command buffer recording). The existing teardown already dispatches via video_thread_texture_handle + font_driver_free_wrap; the OOM path now takes the same route. Before the refactor, font_driver_free had its own inline thread- dispatch block with the safety rationale as a comment. That comment is preserved verbatim on the shared helper so the knowledge isn't lost. Thread-safety: - Non-threaded video: renderer->free runs synchronously on the caller's thread. Identical to the previous behaviour for both call sites. - Threaded video: renderer->free is dispatched to the video thread via video_thread_texture_handle. For the OOM cleanup path this is a behaviour fix (previous code didn't free at all), not a change to the teardown path itself. video_thread_texture_handle is documented self-safe: falls back to direct call if the wrapper isn't active, calls func directly if already on the video thread. Control flow in font_driver_free is slightly reorganised - the original code had an early 'free(font); return;' inside the threaded branch and let the non-threaded path fall through. The refactor unifies the post-renderer-free cleanup (NULL ivars + free(font)) at the end of the function body. Equivalent in behaviour, easier to read. No functional change to font_driver_init_first's success path; the wrapper struct allocation, field assignment, and return are unchanged.
1 parent c0f6fc4 commit 9441128

1 file changed

Lines changed: 67 additions & 47 deletions

File tree

gfx/font_driver.c

Lines changed: 67 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -918,63 +918,71 @@ static uintptr_t font_driver_free_wrap(void *data)
918918
}
919919
#endif
920920

921+
/* Free the renderer-owned state (glyph atlas / GPU textures / etc.)
922+
* behind a (renderer, handle) pair. Shared between the normal
923+
* font_driver_free teardown path and the OOM cleanup path in
924+
* font_driver_init_first; keeping the thread-dispatch logic in one
925+
* place avoids the two call sites drifting out of sync.
926+
*
927+
* When threaded video is active, font resources (GPU textures, GL
928+
* names, D3D COM objects) belong to the video thread's rendering
929+
* context. Freeing them on the main thread races with the video
930+
* thread's draw calls:
931+
*
932+
* - GL: context is single-threaded; gl2_raster_font_free
933+
* calls make_current to steal the context, but the video
934+
* thread may be mid-frame.
935+
* - D3D11: ImmediateContext is not thread-safe; Release on
936+
* the main thread while the video thread draws is UB.
937+
* - D3D12: fenceValue++ from the main thread races with
938+
* the video thread's own fence signalling.
939+
* - Vulkan: vkQueueWaitIdle under queue_lock only drains
940+
* submitted work, not command buffers being recorded.
941+
*
942+
* Dispatch renderer->free to the video thread via
943+
* video_thread_texture_handle so it runs serialised with
944+
* the video thread's frame rendering. This is the same
945+
* pattern used by texture load/unload.
946+
*
947+
* video_thread_texture_handle is self-safe: if the wrapper
948+
* is not active (VIDEO_FLAG_THREAD_WRAPPER_ACTIVE not set),
949+
* it falls back to calling func(data) on the current thread.
950+
* If called from the video thread itself, it calls func
951+
* directly (no deadlock). */
952+
static void font_driver_release_renderer_state(
953+
const font_renderer_t *renderer, void *renderer_data,
954+
bool is_threaded)
955+
{
956+
if (!renderer || !renderer->free)
957+
return;
958+
959+
#ifdef HAVE_THREADS
960+
if (is_threaded)
961+
{
962+
font_free_cmd_t cmd;
963+
cmd.renderer = renderer;
964+
cmd.renderer_data = renderer_data;
965+
cmd.is_threaded = is_threaded;
966+
video_thread_texture_handle(&cmd, font_driver_free_wrap);
967+
return;
968+
}
969+
#endif
970+
971+
renderer->free(renderer_data, is_threaded);
972+
}
973+
921974
void font_driver_free(font_data_t *font)
922975
{
923976
if (font)
924977
{
925-
const font_renderer_t *renderer;
926978
bool is_threaded = false;
927979
#ifdef HAVE_THREADS
928980
bool *is_threaded_tmp = video_driver_get_threaded();
929981
is_threaded = *is_threaded_tmp;
930982
#endif
931-
renderer = font ? font->renderer : NULL;
932983

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-
976-
if (renderer && renderer->free)
977-
renderer->free(font->renderer_data, is_threaded);
984+
font_driver_release_renderer_state(font->renderer,
985+
font->renderer_data, is_threaded);
978986

979987
font->renderer = NULL;
980988
font->renderer_data = NULL;
@@ -1014,6 +1022,18 @@ font_data_t *font_driver_init_first(
10141022
font->size = font_size;
10151023
return font;
10161024
}
1025+
1026+
/* Wrapper malloc failed after font_init_first (or
1027+
* video_thread_font_init) had already succeeded. The raster
1028+
* font's init path allocates the glyph atlas / GPU textures /
1029+
* COM objects behind font_handle; returning NULL here without
1030+
* releasing them would leak the entire raster-font state and,
1031+
* on subsequent re-init attempts, accumulate. Dispatch via
1032+
* the shared helper so threaded-video builds free GPU state
1033+
* on the video thread, matching the normal teardown path. */
1034+
font_driver_release_renderer_state(
1035+
(const font_renderer_t*)font_driver,
1036+
font_handle, is_threaded);
10171037
}
10181038

10191039
return NULL;

0 commit comments

Comments
 (0)