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

Add tests for threaded resource loading #73825

Closed
wants to merge 1 commit into from
Closed

Add tests for threaded resource loading #73825

wants to merge 1 commit into from

Conversation

myaaaaaaaaa
Copy link
Contributor

For #73647

I've only been able to test the non-threaded sections, so if this fails (which it probably will), it would be due to incorrect usage of ResourceLoader's threading functions.

@RandomShaper Present for you 🙂

@RandomShaper
Copy link
Member

Playing with this plus my #71280 at https://github.com/RandomShaper/godot/tree/fix_deadlock_windows_with_tests.

@RandomShaper
Copy link
Member

RandomShaper commented Feb 28, 2023

I've done a mashup of threading fixing and testing PRs. Thread sanitizer seems OK now!
https://github.com/RandomShaper/godot/commits/fix_deadlock_windows_with_tests

After 4.0 is released, I guess all these PRs can be merged. They will be good for 4.0.1 and also set a good basis for the threading safety overhaul that will happen on 4.1.

@mrTag
Copy link
Contributor

mrTag commented Mar 9, 2023

I wrote a unit test to help @RandomShaper with the threaded (or dreaded? 😄) resource loading fixes #74405 (comment)
Have a look at the code here: https://gist.github.com/mrTag/117ba6737ab9cee789cb4835dc702fe2 maybe it would make sense to include it in this PR? Or maybe your "[Resource] Thread safety test" does something similar? It freezes on my system and I didn't quite get what it wanted to test.

@YuriSizov YuriSizov modified the milestones: 4.1, 4.2 Jun 14, 2023
@myaaaaaaaaa myaaaaaaaaa marked this pull request as ready for review June 24, 2023 06:34
@myaaaaaaaaa myaaaaaaaaa requested a review from a team as a code owner June 24, 2023 06:34
@akien-mga akien-mga requested a review from RandomShaper June 24, 2023 08:00
@myaaaaaaaaa myaaaaaaaaa closed this by deleting the head repository Oct 6, 2023
@AThousandShips AThousandShips removed this from the 4.2 milestone Oct 6, 2023
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.

6 participants