Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WorkerThreadPool: Fix end-of-yield logic potentially leading to deadlocks #96225

Merged
merged 1 commit into from
Aug 28, 2024

Conversation

RandomShaper
Copy link
Member

@RandomShaper RandomShaper commented Aug 28, 2024

Without this, certain situations with much thread interactions can deadlock. The specific case I run into was multi-threaded resource loading from the editor with progress dialog running iterations and also separate-threaded physics.

If the flag was unconditionanlly reset, this could happen:

  • A worker thread (A) is used by a server.
  • Thread A starts yielding.
  • Thread A is awaken to start processing a task.
  • Thrad B triggers end-of-yield on thread A so the relevant flag is set.
  • Thread A ends processing its current task and clears the flag.
  • Thread A goes back to yielding without a chance of seeing the flag set so it keeps waiting forever.

@RandomShaper RandomShaper added bug topic:core cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release labels Aug 28, 2024
@RandomShaper RandomShaper added this to the 4.4 milestone Aug 28, 2024
@RandomShaper RandomShaper requested a review from a team as a code owner August 28, 2024 13:28
@akien-mga akien-mga merged commit e439154 into godotengine:master Aug 28, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks!

@RandomShaper RandomShaper deleted the wtp_fix_yield branch August 28, 2024 16:45
@akien-mga
Copy link
Member

Cherry-picked for 4.3.1.

@akien-mga akien-mga removed the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants