Regression: Crash when user opens directory instead of file on USD import #105160

New Issue

Matt McLin · 2023-02-24T02:41:29+01:00

Matt McLin commented

2023-02-24 02:41:29 +01:00

System Information
Operating system: macOS 13.3
Graphics card: M1 Max

Blender Version
Broken: 3.5, 1e6ed77896, main, 2023-02-13, debug build
Worked: 3.5, 3.4.x release builds

Short description of error
I am consistently hitting a crash in Blender on macOS if I (originally accidentally, and now intentionally) misuse the USD import dialog by attempting to import a directory instead of a .usd file. However, it seems I am only able to reproduce the issue with debug builds of Blender.

Exact steps for others to reproduce the error

start Blender
File->Import, Universal Scene Description
browse to any directory
click the Import USD button (without a file selected)

Analysis of bug
There is a specific sequence of events that appears to inevitably and logically lead to the crash:

At conclusion of the failed USD import, the main thread is calling usd::import_endjob() from wm_job_end(), which is being executed in the context of iterating through a linked list of timers in wm->timers in wm_window_timer(), via LISTBASE_FOREACH_MUTABLE(wmTimer *, wt, &wm->timers). As part of this linked list iteration, it stores the next value to evaluate in wt_iter_next.
Inside usd::import_endjob() we call WM_report() to report a helpful error message to the user.
As part of WM_report_banner_show(), the report timer is reset and removed.
WM_event_remove_timer() calls BLI_remlink() to remove the report timer from wm->timers.
The timer that is removed is in fact the same timer that was already assigned to wt_iter_next in step 1.
The memory for that timer is deallocated via MEM_freeN(wt);
Very soon, still in the same main thread, we arrive back at the LISTBASE_FOREACH_MUTABLE loop, where the now deallocated wt_iter_next has an invalid value and is assigned as the next wt to evaluate. This macro attempts to dereference wt->next, which causes an access violation, and Blender crashes.

It is not yet clear to me how the release builds seem to get away with this bug without crashing, but my expectation is that this is likely due to the debug-build deallocator overwriting memory that the release-build deallocator does not touch. In that case the release build is just getting lucky.

See attached screenshot of crash. See also attached screenshot showing the problematic bit of code just prior to the crash.

**System Information** Operating system: macOS 13.3 Graphics card: M1 Max **Blender Version** Broken: 3.5, 1e6ed778969, main, 2023-02-13, debug build Worked: 3.5, 3.4.x release builds **Short description of error** I am consistently hitting a crash in Blender on macOS if I (originally accidentally, and now intentionally) misuse the USD import dialog by attempting to import a directory instead of a .usd file. However, it seems I am only able to reproduce the issue with debug builds of Blender. **Exact steps for others to reproduce the error** - start Blender - File->Import, Universal Scene Description - browse to any directory - click the Import USD button (without a file selected) **Analysis of bug** There is a specific sequence of events that appears to inevitably and logically lead to the crash: 1. At conclusion of the failed USD import, the main thread is calling `usd::import_endjob()` from `wm_job_end()`, which is being executed in the context of iterating through a linked list of timers in `wm->timers` in `wm_window_timer()`, via `LISTBASE_FOREACH_MUTABLE(wmTimer *, wt, &wm->timers)`. As part of this linked list iteration, it stores the next value to evaluate in `wt_iter_next`. 2. Inside `usd::import_endjob()` we call `WM_report()` to report a helpful error message to the user. 3. As part of `WM_report_banner_show()`, the report timer is reset and removed. 4. `WM_event_remove_timer()` calls `BLI_remlink()` to remove the report timer from `wm->timers`. 5. The timer that is removed is in fact the same timer that was already assigned to `wt_iter_next` in step 1. 6. The memory for that timer is deallocated via `MEM_freeN(wt);` 7. Very soon, still in the same main thread, we arrive back at the `LISTBASE_FOREACH_MUTABLE` loop, where the now deallocated `wt_iter_next` has an invalid value and is assigned as the next `wt` to evaluate. This macro attempts to dereference `wt->next`, which causes an access violation, and Blender crashes. It is not yet clear to me how the release builds seem to get away with this bug without crashing, but my expectation is that this is likely due to the debug-build deallocator overwriting memory that the release-build deallocator does not touch. In that case the release build is just getting lucky. See attached screenshot of crash. See also attached screenshot showing the problematic bit of code just prior to the crash.

blender_wmTimer_crash.png

481 KiB

blender_wmTimer_crash_perpetrator.png

701 KiB