runtime: unlock the alive lock only once #3208
Conversation
openshift-ci-robot
added
approved
|
@giuseppe PTAL |
| @@ -891,7 +888,6 @@ func makeRuntime(ctx context.Context, runtime *Runtime) (err error) { | |||
| // no containers running. Create immediately a namespace, as | |||
| // we will need to access the storage. | |||
| if os.Geteuid() != 0 { | |||
| aliveLock.Unlock() | |||
There was a problem hiding this comment.
I think think will cause a deadlock. BecomeRootInUserNS will cause a re-exec, and will wait for the new process to terminate. But that won't happen as the new process will also try to get a lock on the "alive" file
There was a problem hiding this comment.
Ouch, that sounds like deadlock. Thanks for the hint! I'll fix that up and ping after repush.
There was a problem hiding this comment.
do we need to set locked = false to avoid the double unlock?
There was a problem hiding this comment.
Should be better now. WDYT?
|
Code LGTM. Gating's exploding though. |
Unlock the alive lock only once in the deferred func call. Fixes: containers#3207 Signed-off-by: Valentin Rothberg <[email protected]>
| @@ -891,7 +888,6 @@ func makeRuntime(ctx context.Context, runtime *Runtime) (err error) { | |||
| // no containers running. Create immediately a namespace, as | |||
| // we will need to access the storage. | |||
| if os.Geteuid() != 0 { | |||
| aliveLock.Unlock() | |||
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, vrothberg 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 |
|
Given the two LGTMs... |
|
@vrothberg: you cannot LGTM your own PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/lgtm |
Ha, if the bot would know ... |
github-actions
bot
added
the
locked - please file new issue/PR
Unlock the alive lock only once in the deferred func call.
Fixes: #3207
Signed-off-by: Valentin Rothberg [email protected]