ssh: Specifically designate a master multiplex connection

SSHTransfer.Shutdown() was attempting to first shut down the first connection it created (which happened to be the master connection), but this deadlocked because the master connection was waiting for the extra connections to shut down. Designate the first connection as the master connection, make sure that it truly is the master connection, and shut it down after shutting down all extra connections. Issue: #5535
git-lfs · Oct 9, 2023 · 448b0c4 · 448b0c4
1 parent e8395df
commit 448b0c4
Show file tree

Hide file tree

Showing 5 changed files with 162 additions and 74 deletions.
diff --git a/lfshttp/ssh.go b/lfshttp/ssh.go
@@ -79,7 +79,7 @@ func (c *sshAuthClient) Resolve(e Endpoint, method string) (sshAuthResponse, err
 		return res, nil
 	}
 
-	exe, args, _ := ssh.GetLFSExeAndArgs(c.os, c.git, &e.SSHMetadata, "git-lfs-authenticate", endpointOperation(e, method), false)
+	exe, args, _, _ := ssh.GetLFSExeAndArgs(c.os, c.git, &e.SSHMetadata, "git-lfs-authenticate", endpointOperation(e, method), false, "")
 	cmd, err := subprocess.ExecCommand(exe, args...)
 	if err != nil {
 		return res, err

diff --git a/ssh/connection.go b/ssh/connection.go
@@ -17,10 +17,11 @@ type SSHTransfer struct {
 	meta         *SSHMetadata
 	operation    string
 	multiplexing bool
+	controlPath  string
 }
 
 func NewSSHTransfer(osEnv config.Environment, gitEnv config.Environment, meta *SSHMetadata, operation string) (*SSHTransfer, error) {
-	conn, multiplexing, err := startConnection(0, osEnv, gitEnv, meta, operation)
+	conn, multiplexing, controlPath, err := startConnection(0, osEnv, gitEnv, meta, operation, "")
 	if err != nil {
 		return nil, err
 	}
@@ -31,28 +32,29 @@ func NewSSHTransfer(osEnv config.Environment, gitEnv config.Environment, meta *S
 		meta:         meta,
 		operation:    operation,
 		multiplexing: multiplexing,
+		controlPath:  controlPath,
 		conn:         []*PktlineConnection{conn},
 	}, nil
 }
 
-func startConnection(id int, osEnv config.Environment, gitEnv config.Environment, meta *SSHMetadata, operation string) (*PktlineConnection, bool, error) {
+func startConnection(id int, osEnv config.Environment, gitEnv config.Environment, meta *SSHMetadata, operation string, multiplexControlPath string) (conn *PktlineConnection, multiplexing bool, controlPath string, err error) {
 	tracerx.Printf("spawning pure SSH connection")
-	exe, args, multiplexing := GetLFSExeAndArgs(osEnv, gitEnv, meta, "git-lfs-transfer", operation, true)
+	exe, args, multiplexing, controlPath := GetLFSExeAndArgs(osEnv, gitEnv, meta, "git-lfs-transfer", operation, true, multiplexControlPath)
 	cmd, err := subprocess.ExecCommand(exe, args...)
 	if err != nil {
-		return nil, false, err
+		return nil, false, "", err
 	}
 	r, err := cmd.StdoutPipe()
 	if err != nil {
-		return nil, false, err
+		return nil, false, "", err
 	}
 	w, err := cmd.StdinPipe()
 	if err != nil {
-		return nil, false, err
+		return nil, false, "", err
 	}
 	err = cmd.Start()
 	if err != nil {
-		return nil, false, err
+		return nil, false, "", err
 	}
 
 	var pl Pktline
@@ -61,7 +63,7 @@ func startConnection(id int, osEnv config.Environment, gitEnv config.Environment
 	} else {
 		pl = pktline.NewPktline(r, w)
 	}
-	conn := &PktlineConnection{
+	conn = &PktlineConnection{
 		cmd: cmd,
 		pl:  pl,
 		r:   r,
@@ -74,7 +76,7 @@ func startConnection(id int, osEnv config.Environment, gitEnv config.Environment
 		cmd.Wait()
 	}
 	tracerx.Printf("pure SSH connection successful")
-	return conn, multiplexing, err
+	return conn, multiplexing, controlPath, err
 }
 
 // Connection returns the nth connection (starting from 0) in this transfer
@@ -123,22 +125,37 @@ func (tr *SSHTransfer) SetConnectionCountAtLeast(n int) error {
 func (tr *SSHTransfer) setConnectionCount(n int) error {
 	count := len(tr.conn)
 	if n < count {
-		for _, item := range tr.conn[n:count] {
+		tn := n
+		if tn == 0 {
+			tn = 1
+		}
+		for _, item := range tr.conn[tn:count] {
 			tracerx.Printf("terminating pure SSH connection (%d -> %d)", count, n)
 			if err := item.End(); err != nil {
 				return err
 			}
 		}
-		tr.conn = tr.conn[0:n]
+		tr.conn = tr.conn[0:tn]
 	} else if n > count {
 		for i := count; i < n; i++ {
-			conn, _, err := startConnection(i, tr.osEnv, tr.gitEnv, tr.meta, tr.operation)
+			conn, _, controlPath, err := startConnection(i, tr.osEnv, tr.gitEnv, tr.meta, tr.operation, tr.controlPath)
 			if err != nil {
 				tracerx.Printf("failed to spawn pure SSH connection: %s", err)
 				return err
 			}
 			tr.conn = append(tr.conn, conn)
+			if i == 0 {
+				tr.controlPath = controlPath
+			}
+		}
+	}
+	if n == 0 && count > 0 {
+		tracerx.Printf("terminating pure SSH connection (%d -> %d)", count, n)
+		if err := tr.conn[0].End(); err != nil {
+			return err
 		}
+		tr.conn = nil
+		tr.controlPath = ""
 	}
 	return nil
 }

diff --git a/ssh/ssh.go b/ssh/ssh.go
@@ -3,11 +3,11 @@ package ssh
 import (
 	"fmt"
 	"os"
+	"path"
 	"path/filepath"
 	"regexp"
 	"runtime"
 	"strings"
-	"syscall"
 
 	"github.com/git-lfs/git-lfs/v3/config"
 	"github.com/git-lfs/git-lfs/v3/subprocess"
@@ -30,20 +30,20 @@ type SSHMetadata struct {
 	Path        string
 }
 
-func FormatArgs(cmd string, args []string, needShell bool, multiplex bool) (string, []string) {
+func FormatArgs(cmd string, args []string, needShell bool, multiplex bool, controlPath string) (string, []string) {
 	if !needShell {
 		return cmd, args
 	}
 
 	return subprocess.FormatForShellQuotedArgs(cmd, args)
 }
 
-func GetLFSExeAndArgs(osEnv config.Environment, gitEnv config.Environment, meta *SSHMetadata, command, operation string, multiplexDesired bool) (string, []string, bool) {
-	exe, args, needShell, multiplexing := GetExeAndArgs(osEnv, gitEnv, meta, multiplexDesired)
+func GetLFSExeAndArgs(osEnv config.Environment, gitEnv config.Environment, meta *SSHMetadata, command, operation string, multiplexDesired bool, multiplexControlPath string) (exe string, args []string, multiplexing bool, controlPath string) {
+	exe, args, needShell, multiplexing, controlPath := GetExeAndArgs(osEnv, gitEnv, meta, multiplexDesired, multiplexControlPath)
 	args = append(args, fmt.Sprintf("%s %s %s", command, meta.Path, operation))
-	exe, args = FormatArgs(exe, args, needShell, multiplexing)
+	exe, args = FormatArgs(exe, args, needShell, multiplexing, controlPath)
 	tracerx.Printf("run_command: %s %s", exe, strings.Join(args, " "))
-	return exe, args, multiplexing
+	return exe, args, multiplexing, controlPath
 }
 
 // Parse command, and if it looks like a valid command, return the ssh binary
@@ -119,22 +119,12 @@ func getControlDir(osEnv config.Environment) (string, error) {
 	if dir == "" {
 		return os.MkdirTemp(tmpdir, pattern)
 	}
-	dir = filepath.Join(dir, "git-lfs")
-	err := os.Mkdir(dir, 0700)
-	if err != nil {
-		// Ideally we would use errors.Is here to check against
-		// os.ErrExist, but that's not available on Go 1.11.
-		perr, ok := err.(*os.PathError)
-		if !ok || perr.Err != syscall.EEXIST {
-			return os.MkdirTemp(tmpdir, pattern)
-		}
-	}
-	return dir, nil
+	return os.MkdirTemp(dir, pattern)
 }
 
 // Return the executable name for ssh on this machine and the base args
 // Base args includes port settings, user/host, everything pre the command to execute
-func GetExeAndArgs(osEnv config.Environment, gitEnv config.Environment, meta *SSHMetadata, multiplexDesired bool) (exe string, baseargs []string, needShell bool, multiplexing bool) {
+func GetExeAndArgs(osEnv config.Environment, gitEnv config.Environment, meta *SSHMetadata, multiplexDesired bool, multiplexControlPath string) (exe string, baseargs []string, needShell bool, multiplexing bool, controlPath string) {
 	var cmd string
 
 	ssh, _ := osEnv.Get("GIT_SSH")
@@ -161,13 +151,20 @@ func GetExeAndArgs(osEnv config.Environment, gitEnv config.Environment, meta *SS
 	}
 
 	multiplexing = false
-	multiplexEnabled := gitEnv.Bool("lfs.ssh.automultiplex", true)
+	multiplexEnabled := gitEnv.Bool("lfs.ssh.automultiplex", runtime.GOOS != "windows")
 	if variant == variantSSH && multiplexDesired && multiplexEnabled {
-		controlPath, err := getControlDir(osEnv)
-		if err == nil {
+		controlMasterArg := "-oControlMaster=no"
+		controlPath = multiplexControlPath
+		if multiplexControlPath == "" {
+			controlMasterArg = "-oControlMaster=yes"
+			controlDir, err := getControlDir(osEnv)
+			if err == nil {
+				controlPath = path.Join(controlDir, "lfs.sock")
+			}
+		}
+		if controlPath != "" {
 			multiplexing = true
-			controlPath = filepath.Join(controlPath, "sock-%C")
-			args = append(args, "-oControlMaster=auto", fmt.Sprintf("-oControlPath=%s", controlPath))
+			args = append(args, controlMasterArg, fmt.Sprintf("-oControlPath=%s", controlPath))
 		}
 	}
 
@@ -198,7 +195,7 @@ func GetExeAndArgs(osEnv config.Environment, gitEnv config.Environment, meta *SS
 		args = append(args, meta.UserAndHost)
 	}
 
-	return cmd, args, needShell, multiplexing
+	return cmd, args, needShell, multiplexing, controlPath
 }
 
 const defaultSSHCmd = "ssh"