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

panic in OnPotentialDeadlock callback #29

Open
atishaya11 opened this issue Aug 8, 2023 · 3 comments
Open

panic in OnPotentialDeadlock callback #29

atishaya11 opened this issue Aug 8, 2023 · 3 comments

Comments

@atishaya11
Copy link

Hi,

Raising a request to check if there are plans to support panic in OnPotentialDeadlock callback. Default behaviour is a os.Exit. If I override the callback function, panicking in callback is not supported as internally unlock for lockOrder mutex is not deferred which creates issues with further execution of the program.

Also checking if it was a conscious call to not support panic in callback.

My usecase is to achieve something as below so that all the go routine are not impacted apart from the one where the deadlock could have come up.

deadlock.Opts.OnPotentialDeadlock = onPotentialDeadlock
func onPotentialDeadlock () {
	panic("potential deadlock detected")
}

Please let me know if there are any follow up questions.

Thanks

@atishaya11
Copy link
Author

Hi @sasha-s, Can you please help here ?

@sasha-s
Copy link
Owner

sasha-s commented Aug 17, 2023

How do you expect panic to work from within OnPotentialDeadlock?

It's being called from a separate goroutine when DeadlockTimeout is triggered, so in that case panic would terminate the program.

What do you want to do after panic? The mutexes (in your code) that were involved in a deadlock will stay locked, so next time you try to lock them, you will get into a new deadlock.

@atishaya11
Copy link
Author

Thanks for your response @sasha-s.

So we were using earlier version of the library in which I feel the OnPotentialDeadlock callback is called on the go routine which is trying to take lock. Please correct me if I am missing something.

The thought process is if we are able to panic the go routine and appropriately recover it, only a particular go routine will be impacted avoiding the impact of os.Exit(). Definitely, not a solution but just a fallback to reduce the blast radius.

Also my understanding was that OnPotentialDeadlock is called when the deadlock could have occurred because the lock function wouldn't have acquired the lock.(Considering version v0.2.0 of the library) in all three cases:

  • Recursive lock: Since it is being called in preLock itself
  • Inconsistent Ordering: Same as above
  • Timeout: one go routine tries to take the lock and waits for TimeoutSeconds. If it is unable to, the callback is called on the same go routine.

Please do let me know there is anything being missed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants