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

Give the barrier pool its own mutex to avoid a deadlock with transfer workers. #99066

Merged

Conversation

DarioSamo
Copy link
Contributor

Fixes #99043.

Sharing a single mutex for the barriers vector that is used before the start of each frame led into a possible deadlock if during the process of acquiring a transfer worker, there's no available workers due to a low processor count (or an big queue of threaded buffer/texture loading), so an existing worker must be waited on and flushed. This could lead into a deadlock if the frame was also waiting on said worker to finish processing before going ahead with drawing the frame, as the pool mutex was locked.

The fix itself makes sense and should be a pretty safe merge.

@Saul2022
Copy link

I do wonder though, could this help with #98330 (comment) , though it works well on pc, it´s on android where it quits.

Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Seems fine to me

@clayjohn
Copy link
Member

clayjohn commented Nov 12, 2024

I do wonder though, could this help with #98330 (comment) , though it works well on pc, it´s on android where it quits.

This PR fixes a regression from the Ubershader PR which wasn't merged until after dev3. If you can reproduce your issue in dev3 then it likely won't be fixed by this PR.

@Saul2022
Copy link

issue in dev3 then it likely won't be fixed by this PR.

Yea it seems like not , but now on dev 4 it gives this error

  drivers/vulkan/rendering_device_driver_vulkan.cpp:2614 - Condition "err != VK_SUCCESS && err != VK_SUBOPTIMAL_KHR" is true. Returning: FAILED
--- De ```

@Repiteo Repiteo merged commit 76fa7b2 into godotengine:master Nov 13, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 13, 2024

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.

Main thread sometimes deadlocks when allocating textures on a separate thread
6 participants