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

Consider a thread done if current_work is null #49682

Merged
merged 1 commit into from
Jun 17, 2021

Conversation

lyuma
Copy link
Contributor

@lyuma lyuma commented Jun 17, 2021

Fixes #48257
Addresses one of the causes of #49324

I forgot to make a PR for this fix, so here it is finally. I was hoping someone with more knowledge of the threaded importer would make a comment, but it seems to work well for me so far.

Read #48257 for more detail

@lyuma lyuma requested a review from a team as a code owner June 17, 2021 12:27
@akien-mga akien-mga added this to the 4.0 milestone Jun 17, 2021
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.

Makes sense to me. Let's get it validated by @reduz but I can see how returning false here would lead to troubles.

@lyuma
Copy link
Contributor Author

lyuma commented Jun 17, 2021

I would also ask that you check the theory in this comment by @fasterthansound:
#48257 (comment)

There could be an underlying issue that causes current_work to be null.

@akien-mga
Copy link
Member

akien-mga commented Jun 17, 2021

There could be an underlying issue that causes current_work to be null.

Definitely, but this change doesn't hide it, it will still throw an error message. So this will need to be fixed anyway eventually, but at least it shouldn't freeze anymore (but there might still be non-working functionality due to this error).

Edit: But indeed if this PR closes #49324, we'll probably want a new issue focused on the fact that current_work is still unexpectedly null and triggers this error condition.

@akien-mga
Copy link
Member

Approved by @reduz on RC.

@akien-mga akien-mga merged commit 7e4c466 into godotengine:master Jun 17, 2021
@akien-mga
Copy link
Member

Thanks!

@lyuma lyuma deleted the thread_work_done branch May 19, 2023 10:46
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.

Infinite loop in reimport_files, Condition "current_work == nullptr" is true.
2 participants