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: Polish yielding (fix corner case, remove misleading warning) #90809

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

RandomShaper
Copy link
Member

@RandomShaper RandomShaper commented Apr 17, 2024

Intended to avoid a crash when a daemon task has already finished but it still exists and, before its awaited so it gets erased, notify_yield_over() is called with its id. The pool_thread_index would be -1. This PR handles such a situation gracefully.

Oh, and the warning is removed because it can legitly happen that a task notifies itself to stop yielding. A case of that is that a daemon task started yielding and thus started a collaborative wait in which it can run other tasks on top of its stack frame. Its correct that such an additional task wants to awake the underlying daemon.

@RandomShaper RandomShaper added this to the 4.3 milestone Apr 17, 2024
@RandomShaper RandomShaper requested a review from a team as a code owner April 17, 2024 16:39
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

I can't fully evaluate this myself, but trust RandomShaper :D
Let's merge and see if the CI stops having spurious errors.

@akien-mga akien-mga merged commit 26d8ae0 into godotengine:master Apr 17, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

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