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

validation: fix deadlock in XrDestroyInstance #23

Conversation

Projects
None yet
2 participants
@pH5
Copy link
Contributor

pH5 commented Mar 28, 2019

GenValidUsageNextXrDestroyInstance locks the non-recursive g_instance_dispatch_mutex and then calls GenValidUsageCleanUpMaps under the lock, which calls EraseAllInstanceTableMapElements, which tries to lock g_instance_dispatch_mutex again, causing a deadlock.

Since EraseAllInstanceTableMapElements is only called from GenValidUsageNextXrDestroyInstance, g_instance_dispatch_mutex is always guaranteed to be locked, so drop the local std::unique_lock.

validation: fix deadlock in XrDestroyInstance
GenValidUsageNextXrDestroyInstance locks the non-recursive
g_instance_dispatch_mutex and then calls GenValidUsageCleanUpMaps under the
lock, which calls EraseAllInstanceTableMapElements, which tries to lock
g_instance_dispatch_mutex again, causing a deadlock.
Since EraseAllInstanceTableMapElements is only called from
GenValidUsageNextXrDestroyInstance, g_instance_dispatch_mutex is always
guaranteed to be locked, so drop the local std::unique_lock.
@rpavlik

This comment has been minimized.

Copy link
Contributor

rpavlik commented Mar 28, 2019

oh jeepers, I totally didn't see this or your other one before submitting #24 , you must not have been in one of the channels where I warned "hey I'm changing a zillion validation things". I'll see if I can port this across. The code is simpler post-refactoring, but still may share the underlying issue of "it's already locked", haven't dug into too far. (I did manage to shrink lock periods substantially in some cases, so there's a chance it's fixed, but not 100% sure esp. since I'm getting #25 )

image

@rpavlik

rpavlik approved these changes Apr 2, 2019

Copy link
Contributor

rpavlik left a comment

Looks good, verified to work.

@rpavlik

This comment has been minimized.

Copy link
Contributor

rpavlik commented Apr 2, 2019

Merging since validation is broken without this.

@rpavlik rpavlik merged commit 977aa36 into KhronosGroup:master Apr 2, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.