diff --git a/taskfile/node_git.go b/taskfile/node_git.go index 5b96ee6fb7..4080674c88 100644 --- a/taskfile/node_git.go +++ b/taskfile/node_git.go @@ -12,6 +12,7 @@ import ( giturls "github.com/chainguard-dev/git-urls" "github.com/hashicorp/go-getter" + "golang.org/x/sys/unix" "github.com/go-task/task/v3/errors" "github.com/go-task/task/v3/internal/execext" @@ -48,6 +49,31 @@ var globalGitRepoCache = &gitRepoCache{ locks: make(map[string]*sync.Mutex), } +// lockRepoCache takes an exclusive cross-process lock for one repo cache key. +// The in-process mutex (gitRepoCache) does not protect against multiple Task +// *processes* cloning the same repository concurrently (issue #3011), so we +// also flock a lock file that lives outside the cache dir (which CleanGitCache +// removes). +func lockRepoCache(cacheKey string) (func(), error) { + lockDir := filepath.Join(os.TempDir(), "task-git-repos-locks") + if err := os.MkdirAll(lockDir, 0o755); err != nil { + return nil, fmt.Errorf("creating git repo lock dir: %w", err) + } + lockFile := filepath.Join(lockDir, cacheKey+".lock") + f, err := os.OpenFile(lockFile, os.O_CREATE|os.O_RDWR, 0o600) + if err != nil { + return nil, fmt.Errorf("opening git repo lock file: %w", err) + } + if err := unix.Flock(int(f.Fd()), unix.LOCK_EX); err != nil { + f.Close() + return nil, fmt.Errorf("locking git repo cache: %w", err) + } + return func() { + _ = unix.Flock(int(f.Fd()), unix.LOCK_UN) + _ = f.Close() + }, nil +} + func CleanGitCache() error { // Clear the in-memory locks map to prevent memory leak globalGitRepoCache.mu.Lock() @@ -130,6 +156,15 @@ func (node *GitNode) getOrCloneRepo(ctx context.Context) (string, error) { cacheDir := filepath.Join(os.TempDir(), "task-git-repos", cacheKey) + // Cross-process lock: an IDE extension, shell completion and a terminal can + // invoke Task at the same time; the per-process mutex above does not + // serialize them (issue #3011). Serialize the clone with a file lock. + unlock, err := lockRepoCache(cacheKey) + if err != nil { + return "", err + } + defer unlock() + // Check cache FIRST - if already cloned, no network needed, timeout irrelevant gitDir := filepath.Join(cacheDir, ".git") if _, err := os.Stat(gitDir); err == nil { diff --git a/taskfile/node_git_test.go b/taskfile/node_git_test.go index 0c65cd655b..2684272505 100644 --- a/taskfile/node_git_test.go +++ b/taskfile/node_git_test.go @@ -5,6 +5,7 @@ import ( "path/filepath" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -268,3 +269,37 @@ func TestRepoCacheKey_BlocksTraversalRef(t *testing.T) { assert.True(t, strings.HasPrefix(resolved, root+string(os.PathSeparator)), "cache dir %q must stay within %q", resolved, root) } + +func TestLockRepoCache_MutuallyExclusive(t *testing.T) { + t.Parallel() + + // Two acquisitions of the same cache key must not both hold the lock: + // the second blocks until the first releases (issue #3011 cross-process). + key := "test-lock-key" + unlock1, err := lockRepoCache(key) + require.NoError(t, err) + + acquired := make(chan struct{}) + go func() { + unlock2, lockErr := lockRepoCache(key) + require.NoError(t, lockErr) + close(acquired) + unlock2() + }() + + select { + case <-acquired: + t.Fatal("second lock acquired while first still held") + case <-time.After(100 * time.Millisecond): + // Expected: second acquisition is blocked. + } + + unlock1() + + select { + case <-acquired: + // Second acquisition proceeds after release. + case <-time.After(2 * time.Second): + t.Fatal("second lock not acquired after first released") + } +}