VSE text strips can attempt to load fonts from threaded context, which is not allowed. #114592

New Issue

Bastien Montagne · 2023-11-07T18:37:50+01:00

Bastien Montagne commented

2023-11-07 18:37:50 +01:00

System Information
Operating system: N/A
Graphics card: N/A

Blender Version
Broken: current main
Worked: Never ?

Short description of error

The VSE text strip tries to load VFont data in several places, including from code called from non-main threads.

E.g. copy_text_effect callback is called as part of Scene 'CoW' copying during depsgraph evaluation, which can be done from a worker thread.

BLF loading code is not thread safe in any way.

Commit 2f8499415b is a quick band-aid fix to address this issue, but the actual problem is that a strip can try to load a missing font from depsgraph evaluation code. Proper solution is likely rather to store the fact that a font could not be loaded somehow, and avoid trying to reload it over and over.

Exact steps for others to reproduce the error

Make a debug build of current main with 2f8499415b reverted (or neutralized with snippet patch below).

--- a/source/blender/sequencer/intern/effects.cc
+++ b/source/blender/sequencer/intern/effects.cc
@@ -3251,7 +3251,8 @@ void SEQ_effect_text_font_load(TextVars *data, const bool do_id_user)
   else {
     char filepath[FILE_MAX];
     STRNCPY(filepath, vfont->filepath);
-    if (BLI_thread_is_main()) {
+    if (BLI_thread_is_main() || true) {
+      BLI_assert(BLI_thread_is_main());
       /* FIXME: This is a band-aid fix. A proper solution has to be worked on by the VSE team.
        *
        * This code can be called from non-main thread, e.g. when copying sequences as part of

Open vse_crash_on_missing_fonts.blend.
In the VSE of scene Scene.001, attempt to add a Scene strip of Scene.
In debug build Blender will crash immediately on the assert about being in main thread.

**System Information** Operating system: N/A Graphics card: N/A **Blender Version** Broken: current main Worked: Never ? **Short description of error** The VSE text strip tries to load VFont data in several places, including from code called from non-main threads. E.g. `copy_text_effect` callback is called as part of Scene 'CoW' copying during depsgraph evaluation, which can be done from a worker thread. BLF loading code is not thread safe in any way. Commit 2f8499415b is a quick band-aid fix to address this issue, but the actual problem is that a strip can try to load a missing font from depsgraph evaluation code. Proper solution is likely rather to store the fact that a font could not be loaded somehow, and avoid trying to reload it over and over. **Exact steps for others to reproduce the error** Make a debug build of current main with 2f8499415b reverted (or neutralized with snippet patch below). ``` --- a/source/blender/sequencer/intern/effects.cc +++ b/source/blender/sequencer/intern/effects.cc @@ -3251,7 +3251,8 @@ void SEQ_effect_text_font_load(TextVars *data, const bool do_id_user) else { char filepath[FILE_MAX]; STRNCPY(filepath, vfont->filepath); - if (BLI_thread_is_main()) { + if (BLI_thread_is_main() || true) { + BLI_assert(BLI_thread_is_main()); /* FIXME: This is a band-aid fix. A proper solution has to be worked on by the VSE team. * * This code can be called from non-main thread, e.g. when copying sequences as part of ``` * Open [vse_crash_on_missing_fonts.blend](/attachments/35fb982b-609f-4c72-a35c-cba8b08c1763). * In the VSE of scene `Scene.001`, attempt to add a `Scene` strip of `Scene`. * In debug build Blender will crash immediately on the assert about being in main thread.

vse_crash_on_missing_fonts.blend

128 KiB

Bastien Montagne added the

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

VSE text strips can attempt to load fonts from threaded context, which is not allowed. #114592