fix: Resolve RecursionError crash when deleting multiple armatures #432

Merged
Ghost merged 0 commits from refs/pull/432/head into blender-5x-dev 2025-12-09 13:56:23 +00:00
Ghost commented 2025-12-09 07:15:31 +00:00 (Migrated from git.disroot.org)

Fixes #431

I made heavy use of AI when diagnosing this issue and working on a fix, and am not intimately familiar with the code (new to CATS overall). So a little scrutiny from y'all would be much appreciated~

Summary

This PR resolves a crash in Blender 5.0 where deleting multiple objects with armatures causes infinite recursion in the enum property handling code, leading to RecursionError and complete Blender crashes.

Changes

Primary Fix: Remove Direct Property Assignment

  • Commit: 60743bb
  • Removed the direct setattr() attempt in _schedule_enum_fix() that caused recursion
  • All enum property corrections now happen asynchronously via Blender's timer system
  • Improved timer task to properly handle nested property paths using path_resolve()
  • This commit alone was enough to resolve my crashes.

Defense-in-Depth: Add Recursion Guard

  • Commit: d432251
  • Added recursion detection to wrapped_items_func()
  • Tracks currently-processing properties to prevent re-entry
  • Returns minimal valid result if recursion is detected
  • Proper cleanup with try/finally to prevent memory leaks

Technical Details

Root Cause:
When armatures are deleted, the Scene.armature EnumProperty can reference a deleted object. The fix mechanism attempted to update the enum value directly using setattr(), which triggered Blender's enum validation, which called the items callback again, creating an infinite loop.

The Recursion Chain:

User deletes armatures
→ Blender UI accesses Scene.armature
→ wrapped_items_func() called
→ Detects out-of-bounds value
→ _schedule_enum_fix() attempts setattr()
→ Blender validates enum → calls wrapped_items_func() again
→ INFINITE RECURSION → crash

Testing

  • Tested on macOS Sequoia (15.x)
  • Blender 5.0.0
  • CATS 5.0.2 (HEAD of blender-5x-dev branch)
  • Verified with real VRChat avatar project where my crashes originally occurred
    • Was unable to reproduce with a minimal test case, so I couldn't verify outside of my actual project.

I would appreciate help with testing this change with CATS' various use cases (like making sure this doesn't break something else).

Notes

  • This issue only affects Blender 5.0+ due to EnumProperty behavior changes (integer indices vs. string identifiers)
  • Difficult to reproduce in synthetic test cases, but consistently crashes in real VRChat avatar projects
  • Previous commit 3e1af6b partially addressed enum handling but left this recursion path intact
  • The first commit (60743bb) was enough to prevent my crashes, the second commit (d432251) is precautionary.
Fixes #431 I made heavy use of AI when diagnosing this issue and working on a fix, and am not intimately familiar with the code (new to CATS overall). So a little scrutiny from y'all would be much appreciated~ ## Summary This PR resolves a crash in Blender 5.0 where deleting multiple objects with armatures causes infinite recursion in the enum property handling code, leading to `RecursionError` and complete Blender crashes. ## Changes ### Primary Fix: Remove Direct Property Assignment - **Commit:** 60743bb - Removed the direct `setattr()` attempt in `_schedule_enum_fix()` that caused recursion - All enum property corrections now happen asynchronously via Blender's timer system - Improved timer task to properly handle nested property paths using `path_resolve()` - This commit alone was enough to resolve my crashes. ### Defense-in-Depth: Add Recursion Guard - **Commit:** d432251 - Added recursion detection to `wrapped_items_func()` - Tracks currently-processing properties to prevent re-entry - Returns minimal valid result if recursion is detected - Proper cleanup with try/finally to prevent memory leaks ## Technical Details **Root Cause:** When armatures are deleted, the `Scene.armature` EnumProperty can reference a deleted object. The fix mechanism attempted to update the enum value directly using `setattr()`, which triggered Blender's enum validation, which called the items callback again, creating an infinite loop. **The Recursion Chain:** ``` User deletes armatures → Blender UI accesses Scene.armature → wrapped_items_func() called → Detects out-of-bounds value → _schedule_enum_fix() attempts setattr() → Blender validates enum → calls wrapped_items_func() again → INFINITE RECURSION → crash ``` ## Testing - Tested on macOS Sequoia (15.x) - Blender 5.0.0 - CATS 5.0.2 (HEAD of `blender-5x-dev` branch) - Verified with real VRChat avatar project where my crashes originally occurred - Was unable to reproduce with a minimal test case, so I couldn't verify outside of my actual project. I would appreciate help with testing this change with CATS' various use cases (like making sure this doesn't break something else). ## Notes - This issue only affects Blender 5.0+ due to EnumProperty behavior changes (integer indices vs. string identifiers) - Difficult to reproduce in synthetic test cases, but consistently crashes in real VRChat avatar projects - Previous commit 3e1af6b partially addressed enum handling but left this recursion path intact - The first commit (60743bb) was enough to prevent my crashes, the second commit (d432251) is precautionary. ## Related - Blender API changes: [EnumProperty now returns int](https://projects.blender.org/blender/blender/issues/115908)
Ghost (Migrated from git.disroot.org) reviewed 2025-12-09 07:15:31 +00:00
Ghost (Migrated from git.disroot.org) reviewed 2025-12-09 07:22:22 +00:00
Ghost (Migrated from git.disroot.org) commented 2025-12-09 07:22:22 +00:00

This comment suggests that there was a reason we were trying to set this directly (for Scene properties), are there any concerns with dropping this (or past context/nuance I'm missing)?

This comment suggests that there was a reason we were trying to set this directly (for Scene properties), are there any concerns with dropping this (or past context/nuance I'm missing)?
Ghost commented 2025-12-09 10:27:44 +00:00 (Migrated from git.disroot.org)

The code looks fine. However, I have two small concerns, the first is a potential performance bottleneck. The current code immediately fixes non-UI contexts, where as your code always schedules a timer, though this should not have much of an impact tbh. The second concern is that multiple rapid deletions might schedule multiple timers, causing a race condition. However, it seems the scheduled_property_set check should handle this issue. I will do some further testing and review the code fully later to ensure nothing breaks.

The code looks fine. However, I have two small concerns, the first is a potential performance bottleneck. The current code immediately fixes non-UI contexts, where as your code always schedules a timer, though this should not have much of an impact tbh. The second concern is that multiple rapid deletions might schedule multiple timers, causing a race condition. However, it seems the scheduled_property_set check should handle this issue. I will do some further testing and review the code fully later to ensure nothing breaks.
Ghost commented 2025-12-09 10:48:25 +00:00 (Migrated from git.disroot.org)

The performance impact seems negligible (maybe 1 second for larger models). This seems fine, i just have to test more models and will merge later if everything is fine.

The performance impact seems negligible (maybe 1 second for larger models). This seems fine, i just have to test more models and will merge later if everything is fine.
Ghost commented 2025-12-09 13:56:16 +00:00 (Migrated from git.disroot.org)

This seems fine, not found any glaring issues, merged and mark for the 5.0.2.1 release which will be on friday.

This seems fine, not found any glaring issues, merged and mark for the 5.0.2.1 release which will be on friday.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Kneelawk/Cats-Blender-Plugin!432
No description provided.