git/version.go: replace sync.Mutex usage with sync.Once

ttaylorr · ttaylorr · commit 4a992015e57e · 2019-01-23T15:42:32.000-08:00
We use a sync.Mutex to synchronize access to the string pointer
`gitVersion`, which indicates the version of Git used by the system
running Git LFS.

Our basic usage of the sync.Mutex is not incorrect, but we can improve
the readability by instead using a sync.Once. A sync.Once determines
very quickly (and in an atomic, goroutine-safe fashion) whether or not
_any_ function has been run, and if it hasn't, run it.

By doing this, we can--at the first request--produce a value for the
result of running 'git version', and then return it later to the caller.
This has the following benefits:

  - If 'git version' has already been run, we do not need to hold the
    lock for the entire duration of the function.

  - If 'git version' has not already been run, we only run it once,
    retaining the existing behavior.

Only one change, which is the introduction of the `gitVersionErr`
variable. This is a consequence of executing the 'git version' call in a
closure: since we're in a new stack frame, we can't return from our
parent.

Instead, we retain the value for all time, and return _it_, along with
whatever value we got from running 'git version' in the first place.
diff --git a/git/version.go b/git/version.go
@@ -10,21 +10,17 @@ import (
 )
 
 var (
-	gitVersion   *string
-	gitVersionMu sync.Mutex
+	gitVersionOnce sync.Once
+	gitVersion     string
+	gitVersionErr  error
 )
 
 func Version() (string, error) {
-	gitVersionMu.Lock()
-	defer gitVersionMu.Unlock()
-
-	if gitVersion == nil {
-		v, err := subprocess.SimpleExec("git", "version")
-		gitVersion = &v
-		return v, err
-	}
-
-	return *gitVersion, nil
+	gitVersionOnce.Do(func() {
+		gitVersion, gitVersionErr =
+			subprocess.SimpleExec("git", "version")
+	})
+	return gitVersion, gitVersionErr
 }
 
 // IsVersionAtLeast returns whether the git version is the one specified or higher
