Idempotent queue_free() - #1404
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. WalkthroughThe deletion flag now uses atomic access. ChangesDeletion queue handling
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to Repeated queue_free() requests now enqueue an object only once while preserving thread-safe deletion-state access. No merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Makes Object queuing for deletion through
queue_free()idempotent.Currently, an object for can be queued for deletion twice, leaving to the
SceneTreeto check through the instance ID if the object already was deleted or not just before deleting it. This mechanism is still prone to race conditions, even with the mutex lock.The
_is_queued_for_deletionflag needs to be atomic, so that the flagging is effective on the first call and always verifiable, irrespective of which thread queries this state.Surprisingly,
SceneTreenever checked for this flag before pushing the object for deletion. A simple check before pushing the object pointer to the queue makes the request idempotent.Summary by CodeRabbit