diff --git a/act/common/git/git.go b/act/common/git/git.go index f59eddd1..cc33183a 100644 --- a/act/common/git/git.go +++ b/act/common/git/git.go @@ -32,12 +32,21 @@ var ( githubHTTPRegex = regexp.MustCompile(`^https?://.*github.com.*/(.+)/(.+?)(?:.git)?$`) githubSSHRegex = regexp.MustCompile(`github.com[:/](.+)/(.+?)(?:.git)?$`) - cloneLock sync.Mutex + cloneLocks sync.Map // key: clone target directory; value: *sync.Mutex ErrShortRef = errors.New("short SHA references are not supported") ErrNoRepo = errors.New("unable to find git repo") ) +// acquireCloneLock returns an unlock function after locking the per-directory mutex for dir. +// Only concurrent operations targeting the same directory are erialized; clones into different directories run in parallel. +func acquireCloneLock(dir string) func() { + v, _ := cloneLocks.LoadOrStore(dir, &sync.Mutex{}) + mu := v.(*sync.Mutex) + mu.Lock() + return mu.Unlock +} + type Error struct { err error commit string @@ -301,8 +310,7 @@ func NewGitCloneExecutor(input NewGitCloneExecutorInput) common.Executor { logger.Infof(" \u2601 git clone '%s' # ref=%s", input.URL, input.Ref) logger.Debugf(" cloning %s to %s", input.URL, input.Dir) - cloneLock.Lock() - defer cloneLock.Unlock() + defer acquireCloneLock(input.Dir)() refName := plumbing.ReferenceName("refs/heads/" + input.Ref) r, err := CloneIfRequired(ctx, refName, input, logger) diff --git a/act/common/git/git_test.go b/act/common/git/git_test.go index ff9e4c5b..cfcdd09a 100644 --- a/act/common/git/git_test.go +++ b/act/common/git/git_test.go @@ -11,8 +11,10 @@ import ( "os/exec" "path/filepath" "strings" + "sync" "syscall" "testing" + "time" log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" @@ -303,3 +305,61 @@ func gitCmd(args ...string) error { } return nil } + +func TestAcquireCloneLock(t *testing.T) { + t.Run("same directory serializes", func(t *testing.T) { + dir := t.TempDir() + + unlock1 := acquireCloneLock(dir) + + secondAcquired := make(chan struct{}) + go func() { + unlock := acquireCloneLock(dir) + close(secondAcquired) + unlock() + }() + + select { + case <-secondAcquired: + t.Fatal("second acquire should block while first holds the lock") + case <-time.After(50 * time.Millisecond): + } + + unlock1() + + select { + case <-secondAcquired: + case <-time.After(time.Second): + t.Fatal("second acquire should proceed after first releases the lock") + } + }) + + t.Run("different directories do not block", func(t *testing.T) { + dirA := t.TempDir() + dirB := t.TempDir() + + unlockA := acquireCloneLock(dirA) + defer unlockA() + + done := make(chan struct{}) + go func() { + unlock := acquireCloneLock(dirB) + unlock() + close(done) + }() + + select { + case <-done: + case <-time.After(time.Second): + t.Fatal("acquire on a different directory must not block") + } + }) + + t.Run("same directory reuses the same mutex", func(t *testing.T) { + dir := t.TempDir() + + v1, _ := cloneLocks.LoadOrStore(dir, &sync.Mutex{}) + v2, _ := cloneLocks.LoadOrStore(dir, &sync.Mutex{}) + require.Same(t, v1, v2) + }) +}