Skip to content

Fix ResourceLoader deadlocks#120077

Merged
Repiteo merged 1 commit into
godotengine:masterfrom
hpvb:fix-threaded-loader-reloaded
Jun 8, 2026
Merged

Fix ResourceLoader deadlocks#120077
Repiteo merged 1 commit into
godotengine:masterfrom
hpvb:fix-threaded-loader-reloaded

Conversation

@hpvb

@hpvb hpvb commented Jun 8, 2026

Copy link
Copy Markdown
Member

This fixes several issues:

  • There was a bug in the cycle detection. It was possible for the cycle detection to fail because the current thread isn't in the list. Fix this by just seeing if any of the next threads were already in the cycle.
  • By way of an optimization the thread_waiting_on bookkeeping was done under the same thread_load_mutex as the cycle detection. But there is also an early exit after yields. If we exit through the yield the thread_waiting_on entry is stale and can cause other threads to fail to do cycle detection.
  • When multiple threads end up finishing a resource simultaneously anyway, probably through cycle detection, there's a small chance that the original thread finished more-or-less at the same time if the original thing blocking load completed. We solve this now by letting only one of the threads do the initial registration.

Testing was done with MRPs from:

In addition two projects that were shared with me privately.

No regressions compared to current master were found, and the new issues were fixed.

This fixes several issues:
 * There was a bug in the cycle detection. It was possible for the cycle
   detection to fail because the current thread isn't in the list. Fix
   this by just seeing if any of the next threads were already in the
   cycle.
 * By way of an optimization the thread_waiting_on bookkeeping was done
   under the same thread_load_mutex as the cycle detection. But there is
   also an early exit after yields. If we exit through the yield the
   thread_waiting_on entry is stale and can cause other threads to fail
   to do cycle detection.
 * When multiple threads end up finishing a resource simultaneously
   anyway, probably through cycle detection, there's a small chance that
   the original thread finished more-or-less at the same time if the
   original thing blocking load completed. We solve this now by letting
   only one of the threads do the initial registration.
@hpvb hpvb requested a review from a team as a code owner June 8, 2026 02:58
@hpvb hpvb force-pushed the fix-threaded-loader-reloaded branch from b501b77 to b88a62f Compare June 8, 2026 02:58
@hpvb hpvb added this to the 4.7 milestone Jun 8, 2026
@bruvzg bruvzg self-requested a review June 8, 2026 05:06

@bruvzg bruvzg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to fix the only crash I was still able to reproduce. And none of the previously fixed crashes are reproducible.

@akien-mga akien-mga moved this from Unassessed to Release Blocker in 4.x Release Priorities & Blockers Jun 8, 2026
@Repiteo Repiteo merged commit 526528b into godotengine:master Jun 8, 2026
20 checks passed
@Repiteo

Repiteo commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

4 participants