From 01ec256c7ebd9e6e541edba3dc7dff046b6b051a Mon Sep 17 00:00:00 2001 From: "Michael S. Fischer" Date: Mon, 31 Aug 2015 16:33:09 -0700 Subject: [PATCH] lock.go: fix another race condition The previous fix to `consul lock` (commit 6875e8d) didn't completely eliminate the race that could occur if the lock was acquired around the same time SIGTERM was received: It was still possible for Run() to spawn the process via startChild() after killChild() had released the shared mutex. Now, when SIGTERM is received, we acquire a mutex that prevents spawning a new process and never release it. We've tested this fix pretty thoroughly and believe it completely resolves the issue. --- command/lock.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/command/lock.go b/command/lock.go index aa44abe0c..f40fe2d76 100644 --- a/command/lock.go +++ b/command/lock.go @@ -185,7 +185,10 @@ func (c *LockCommand) Run(args []string) int { goto RELEASE } - // Kill the child + // Prevent starting a new child. The lock is never released + // after this point. + c.childLock.Lock() + // Kill any existing child if err := c.killChild(childDone); err != nil { c.Ui.Error(fmt.Sprintf("%s", err)) } @@ -318,9 +321,7 @@ func (c *LockCommand) startChild(script string, doneCh chan struct{}, passStdin // on the first attempt. func (c *LockCommand) killChild(childDone chan struct{}) error { // Get the child process - c.childLock.Lock() child := c.child - c.childLock.Unlock() // If there is no child process (failed to start), we can quit early if child == nil {