From 5160578e186230aecb251bdcfb0219cac35c9230 Mon Sep 17 00:00:00 2001
From: Paul Hinze <phinze@phinze.com>
Date: Fri, 4 Mar 2016 11:20:48 -0600
Subject: [PATCH] helper/resource: restore retval of resource.Retry on timeout

In #4700 while fixing a data race I made an incorrect assumption about
the return value of StateChangeConf, and ended up changing the behavior
in the timeout case to always return:

> timeout while waiting for state to become '[success]'

When it used to capture the "most recent error" from the function
itself.

It's much more useful to see that error bubbling up, so here we revert
to pulling it out of the function as we did before, and we protect
against the data race with a good old fashioned mutex.
---
 helper/resource/wait.go | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/helper/resource/wait.go b/helper/resource/wait.go
index 4167c7db7..cf5f72f3f 100644
--- a/helper/resource/wait.go
+++ b/helper/resource/wait.go
@@ -1,6 +1,7 @@
 package resource
 
 import (
+	"sync"
 	"time"
 )
 
@@ -10,6 +11,11 @@ type RetryFunc func() error
 // Retry is a basic wrapper around StateChangeConf that will just retry
 // a function until it no longer returns an error.
 func Retry(timeout time.Duration, f RetryFunc) error {
+	// These are used to pull the error out of the function; need a mutex to
+	// avoid a data race.
+	var resultErr error
+	var resultErrMu sync.Mutex
+
 	c := &StateChangeConf{
 		Pending:    []string{"error"},
 		Target:     []string{"success"},
@@ -21,17 +27,25 @@ func Retry(timeout time.Duration, f RetryFunc) error {
 				return 42, "success", nil
 			}
 
+			resultErrMu.Lock()
+			defer resultErrMu.Unlock()
+			resultErr = err
 			if rerr, ok := err.(RetryError); ok {
-				err = rerr.Err
-				return nil, "quit", err
+				resultErr = rerr.Err
+				return nil, "quit", rerr.Err
 			}
 
 			return 42, "error", nil
 		},
 	}
 
-	_, err := c.WaitForState()
-	return err
+	c.WaitForState()
+
+	// Need to acquire the lock here to be able to avoid race using resultErr as
+	// the return value
+	resultErrMu.Lock()
+	defer resultErrMu.Unlock()
+	return resultErr
 }
 
 // RetryError, if returned, will quit the retry immediately with the