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

ResourceLoader: Use ThreadLoadTask.thread as the check for "real" threads #74108

Closed
wants to merge 1 commit into from

Conversation

mrTag
Copy link
Contributor

@mrTag mrTag commented Feb 28, 2023

Before PR #73647 using the ThreadLoadTask.semaphore as the check for "real" threads was OK, since only "real" threaded tasks also had a semaphore. After that PR that is no longer the case, since also Tasks that were created by ResourceLoader.load() can get a cond_var (formerly semaphore). So this PR uses ThreadLoadTask.thread as a check for "real" threads instead.

Only "real" threaded tasks should change thread_waiting_count, thread_loading_count, thread_suspended_count and thread_load_semaphore. So all the places where these are changed/used get the check for ThreadLoadTask.thread.

@mrTag mrTag marked this pull request as ready for review February 28, 2023 11:46
@mrTag mrTag requested a review from a team as a code owner February 28, 2023 11:46
@RandomShaper
Copy link
Member

Thank you!

I don't think we are in time to include this in the next release, which I think will be 4.0-stable. I'll let @akien-mga decide the milestone.

@akien-mga akien-mga added this to the 4.1 milestone Feb 28, 2023
@akien-mga
Copy link
Member

For the record, your commit seems not to be linked to your GitHub account. See: Why are my commits linked to the wrong user? for more info.

@mrTag
Copy link
Contributor Author

mrTag commented Feb 28, 2023

Thanks @akien-mga ! Added my work email to the github account and all is well. That has been wrong for a few years now 😆

@akien-mga
Copy link
Member

Superseded by #74405.

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.

3 participants