fix(pkg/sshd/server.go): wrap the git receive logic in an RLock function

Author: Aaron Schlesinger

pkg/sshd/server.go

@@ -8,6 +8,7 @@
 package sshd
 
 import (
+	"errors"
 	"fmt"
 	"io"
 	"net"
@@ -179,6 +180,12 @@ func sendExitStatus(status uint32, channel ssh.Channel) error {
 	return err
 }
 
+func (s *server) withCleanerRLock(f func() error) error {
+	s.cleanerRef.RLock()
+	defer s.cleanerRef.RUnlock()
+	return f()
+}
+
 // answer handles answering requests and channel requests
 //
 // Currently, an exec must be either "ping", "git-receive-pack" or
@@ -232,40 +239,45 @@ func (s *server) answer(channel ssh.Channel, requests <-chan *ssh.Request, sshCo
 				}
 
 				repoName := parts[1]
-				s.cleanerRef.RLock()
-				if err := s.pushLock.Lock(repoName, time.Duration(0)); err != nil {
+				errConcurrentPush := errors.New("concurrent push")
+				err := s.withCleanerRLock(func() error {
+					if err := s.pushLock.Lock(repoName, time.Duration(0)); err != nil {
+						return errConcurrentPush
+					}
+
+					req.Reply(true, nil) // We processed. Yay.
+
+					cxt.Put("channel", channel)
+					cxt.Put("request", req)
+					cxt.Put("operation", parts[0])
+					cxt.Put("repository", parts[1])
+					sshGitReceive := cxt.Get("route.sshd.sshGitReceive", "sshGitReceive").(string)
+					err := router.HandleRequest(sshGitReceive, cxt, true)
+					if err := s.pushLock.Unlock(repoName, time.Duration(0)); err != nil {
+						log.Errf(s.c, "unable to unlock repository lock for %s (%s)", repoName, err)
+						// TODO: this is an important error case that needs to be covered
+						// Probably the best solution is to change the lock into a lease so that even on unlock failures, RepositoryLock will eventually yield
+					}
+					return err
+				})
+
+				if err == errConcurrentPush {
 					log.Errf(s.c, multiplePush)
 					// The error must be in git format
 					if err := gitPktLine(channel, fmt.Sprintf("ERR %v\n", multiplePush)); err != nil {
 						log.Errf(s.c, "Failed to write to channel: %s", err)
 					}
 					sendExitStatus(1, channel)
 					req.Reply(false, nil)
-					s.cleanerRef.RUnlock()
 					return nil
 				}
 
-				req.Reply(true, nil) // We processed. Yay.
-
-				cxt.Put("channel", channel)
-				cxt.Put("request", req)
-				cxt.Put("operation", parts[0])
-				cxt.Put("repository", parts[1])
-				sshGitReceive := cxt.Get("route.sshd.sshGitReceive", "sshGitReceive").(string)
-				err := router.HandleRequest(sshGitReceive, cxt, true)
-				if err := s.pushLock.Unlock(repoName, time.Duration(0)); err != nil {
-					log.Errf(s.c, "unable to unlock repository lock for %s (%s)", repoName, err)
-					// TODO: this is an important error case that needs to be covered
-					// Probably the best solution is to change the lock into a lease so that even on unlock failures, RepositoryLock will eventually yield
-				}
-				s.cleanerRef.RUnlock()
 				var xs uint32
 				if err != nil {
 					log.Errf(s.c, "Failed git receive: %v", err)
 					xs = 1
 				}
 				sendExitStatus(xs, channel)
-
 				return nil
 			default:
 				log.Warnf(s.c, "Illegal command is '%s'\n", clean)