-
Notifications
You must be signed in to change notification settings - Fork 371
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
Fix deadlock in recursive metric locks #581
Fix deadlock in recursive metric locks #581
Conversation
RecordMetrics() grabs a mutex and calls recordCancelMetric(), which wants to grab the same mutex. Go mutexes are not recursive, so recordCancelMetric blocks forever. recordCancelMetric should not grab the mutex, it can be sure that the caller did it already.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jsafrane The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/assign @ggriffiths |
/test pull-kubernetes-csi-external-snapshotter-1-21-on-kubernetes-1-21 |
/lgtm If this does not get rid of the deadlocks, feel free to revert my original PR and I can take a closer look after my PTO. |
Looks like the pull-kubernetes-csi-external-snapshotter-1-21-on-kubernetes-1-21 job is not blocking the merge. The job is still pending. |
What type of PR is this?
/kind bug
/kind flake
What this PR does / why we need it:
RecordMetrics()
grabs a mutex and callsrecordCancelMetric()
, which wants to grab the same mutex. Go mutexes are not recursive, sorecordCancelMetric
blocks forever.recordCancelMetric
should not grab the mutex, all its callers already have the mutex.Which issue(s) this PR fixes:
Fixes #580
Does this PR introduce a user-facing change?: