Skip to content

Commit 6fc59e7

Browse files
committed
p Fix concurrent use of lock files
1 parent 9dfdb6c commit 6fc59e7

2 files changed

Lines changed: 35 additions & 13 deletions

File tree

cmd/script.go

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,15 @@ func (s *Script) EnsureEnv(deleteOldEnv bool) error {
7777

7878
if !readOperationOnly {
7979
err = lockEnv(s.EnvDir)
80+
if errors.Is(err, ErrEnvAlreadyLocked) {
81+
// Another process acquired the lock between our check and our
82+
// lock attempt. Wait for it to finish and use the environment.
83+
err = waitUntilEnvIsUnlocked(s.EnvDir)
84+
if err != nil {
85+
return err
86+
}
87+
return nil
88+
}
8089
if err != nil {
8190
return err
8291
}
@@ -195,16 +204,23 @@ func (s *Script) InstallRequirementsInEnv() error {
195204
return err
196205
}
197206

198-
// RemoveEnv removes the virtual environment for the script. Also removes the lockfile
207+
// RemoveEnv removes the virtual environment for the script. Also removes the lockfile.
208+
// The lockfile is only removed if the directory removal succeeds, so that a broken
209+
// environment is detected and recreated on the next run.
199210
func (s *Script) RemoveEnv() error {
200211
if flagDebug {
201212
loggerErr.Println("Deleting virtual environment...")
202213
}
203214

204215
// Remove the virtual environment directory
205-
err1 := removeDir(s.EnvDir)
206-
err2 := unlockEnv(s.EnvDir)
207-
return errors.Join(err1, err2)
216+
err := removeDir(s.EnvDir)
217+
if err != nil {
218+
// Do not unlock the environment if the directory removal failed.
219+
// This ensures that the next run detects the stale lock and
220+
// recreates the environment instead of using a broken one.
221+
return err
222+
}
223+
return unlockEnv(s.EnvDir)
208224
}
209225

210226
// NewScript creates a new Script instance

cmd/utils.go

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -80,24 +80,30 @@ func isEnvLocked(envDir string) bool {
8080
return true
8181
}
8282

83+
// ErrEnvAlreadyLocked is returned when the environment is already locked by
84+
// another process
85+
var ErrEnvAlreadyLocked = fmt.Errorf("environment is already locked")
86+
8387
func lockEnv(envDir string) error {
8488
if flagDebug {
8589
loggerErr.Println("Locking virtual environment...")
8690
}
8791
lockFileName := generateLockFileName(envDir)
88-
_, err := os.Stat(lockFileName)
89-
if err == nil {
90-
// Already locked
91-
return nil
92+
if err := os.MkdirAll(path.Dir(lockFileName), 0755); err != nil {
93+
return err
9294
}
93-
if os.IsNotExist(err) {
94-
if err = os.MkdirAll(path.Dir(lockFileName), 0755); err != nil {
95-
return err
95+
// Use O_CREATE|O_EXCL to atomically create the lock file. This ensures
96+
// that only one process can acquire the lock. If the file already exists,
97+
// another process holds the lock.
98+
f, err := os.OpenFile(lockFileName, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0644)
99+
if err != nil {
100+
if os.IsExist(err) {
101+
return ErrEnvAlreadyLocked
96102
}
97-
_, err = os.Create(lockFileName)
98103
return err
99104
}
100-
return err
105+
f.Close()
106+
return nil
101107
}
102108

103109
func unlockEnv(envDir string) error {

0 commit comments

Comments
 (0)