diff --git a/utils/io/content/contentreader.go b/utils/io/content/contentreader.go index 0ff253f35..fc8b9332f 100644 --- a/utils/io/content/contentreader.go +++ b/utils/io/content/contentreader.go @@ -9,10 +9,11 @@ import ( "io" "os" "reflect" + "runtime" "sort" "sync" + "syscall" - "github.com/jfrog/gofrog/http/retryexecutor" "github.com/jfrog/jfrog-client-go/utils" "github.com/jfrog/jfrog-client-go/utils/errorutils" "github.com/jfrog/jfrog-client-go/utils/log" @@ -98,22 +99,36 @@ func (cr *ContentReader) Reset() { } func removeFileWithRetry(filePath string) error { - // Check if file exists before attempting to remove - if _, err := os.Stat(filePath); os.IsNotExist(err) { - log.Debug("File does not exist: %s", filePath) - return nil - } - log.Debug("Attempting to remove file: %s", filePath) - executor := retryexecutor.RetryExecutor{ + executor := utils.RetryExecutor{ Context: context.Background(), - MaxRetries: 5, + MaxRetries: 10, RetriesIntervalMilliSecs: 100, - ErrorMessage: "Failed to remove file", - LogMsgPrefix: "Attempting removal", + UseExponentialBackoff: true, + MaxBackoffMilliSecs: 1000, + ErrorMessage: "Failed to remove temporary file", + LogMsgPrefix: "Temp file removal", ExecutionHandler: func() (bool, error) { - return false, errorutils.CheckError(os.Remove(filePath)) + err := os.Remove(filePath) + if err == nil { + return false, nil + } + + // Retry on Windows file locking errors (antivirus, process lock) + if runtime.GOOS == "windows" { + if pathErr, ok := err.(*os.PathError); ok { + if errno, ok := pathErr.Err.(syscall.Errno); ok { + // ERROR_SHARING_VIOLATION=32, ERROR_LOCK_VIOLATION=33, ERROR_ACCESS_DENIED=5 + if errno == 32 || errno == 33 || errno == 5 { + return true, err + } + } + } + } + + return false, errorutils.CheckError(err) }, } + return executor.Execute() } diff --git a/utils/io/fileutils/temp.go b/utils/io/fileutils/temp.go index 8fa1d8fa1..1fd690ee0 100644 --- a/utils/io/fileutils/temp.go +++ b/utils/io/fileutils/temp.go @@ -4,8 +4,10 @@ import ( "fmt" "os" "path" + "runtime" "strconv" "strings" + "syscall" "time" "github.com/jfrog/jfrog-client-go/utils/errorutils" @@ -48,6 +50,42 @@ func GetTempDirBase() string { return tempDirBase } +func removeWithRetry(path string, removeFunc func(string) error) error { + maxRetries := 10 + delay := 100 * time.Millisecond + maxDelay := 1000 * time.Millisecond + + for attempt := 0; attempt <= maxRetries; attempt++ { + err := removeFunc(path) + if err == nil { + return nil + } + + // Retry on Windows file locking errors (antivirus, process lock) + shouldRetry := false + if runtime.GOOS == "windows" { + if pathErr, ok := err.(*os.PathError); ok { + if errno, ok := pathErr.Err.(syscall.Errno); ok { + // ERROR_SHARING_VIOLATION=32, ERROR_LOCK_VIOLATION=33, ERROR_ACCESS_DENIED=5 + shouldRetry = errno == 32 || errno == 33 || errno == 5 + } + } + } + + if !shouldRetry || attempt == maxRetries { + return err + } + + time.Sleep(delay) + delay *= 2 + if delay > maxDelay { + delay = maxDelay + } + } + + return fmt.Errorf("failed to remove after %d attempts", maxRetries) +} + func RemoveTempDir(dirPath string) error { exists, err := IsDirExists(dirPath, false) if err != nil { @@ -56,13 +94,15 @@ func RemoveTempDir(dirPath string) error { if !exists { return nil } - if err = os.RemoveAll(dirPath); err == nil { - return nil + + err = removeWithRetry(dirPath, os.RemoveAll) + if err != nil { + // On Windows, if locked by another process, remove contents only + // CleanOldDirs() will remove the directory itself later + return RemoveDirContents(dirPath) } - // Sometimes removing the directory fails (in Windows) because it's locked by another process. - // That's a known issue, but its cause is unknown (golang.org/issue/30789). - // In this case, we'll only remove the contents of the directory, and let CleanOldDirs() remove the directory itself at a later time. - return RemoveDirContents(dirPath) + + return nil } // Create a new temp file named "tempPrefix+timeStamp". diff --git a/utils/retryexecutor.go b/utils/retryexecutor.go index bad3dc16e..2d6f5e23f 100644 --- a/utils/retryexecutor.go +++ b/utils/retryexecutor.go @@ -4,9 +4,10 @@ import ( "context" "errors" "fmt" - "github.com/jfrog/jfrog-client-go/utils/errorutils" "time" + "github.com/jfrog/jfrog-client-go/utils/errorutils" + "github.com/jfrog/jfrog-client-go/utils/log" ) @@ -22,6 +23,12 @@ type RetryExecutor struct { // Number of milliseconds to sleep between retries. RetriesIntervalMilliSecs int + // Use exponential backoff (double delay each retry) + UseExponentialBackoff bool + + // Maximum backoff interval in milliseconds (0 = no limit) + MaxBackoffMilliSecs int + // Message to display when retrying. ErrorMessage string @@ -35,6 +42,8 @@ type RetryExecutor struct { func (runner *RetryExecutor) Execute() error { var err error var shouldRetry bool + currentInterval := runner.RetriesIntervalMilliSecs + for i := 0; i <= runner.MaxRetries; i++ { // Run ExecutionHandler shouldRetry, err = runner.ExecutionHandler() @@ -47,12 +56,17 @@ func (runner *RetryExecutor) Execute() error { return cancelledErr } - // Print retry log message runner.LogRetry(i, err) - // Going to sleep for RetryInterval milliseconds - if runner.RetriesIntervalMilliSecs > 0 && i < runner.MaxRetries { - time.Sleep(time.Millisecond * time.Duration(runner.RetriesIntervalMilliSecs)) + if currentInterval > 0 && i < runner.MaxRetries { + time.Sleep(time.Millisecond * time.Duration(currentInterval)) + + if runner.UseExponentialBackoff { + currentInterval *= 2 + if runner.MaxBackoffMilliSecs > 0 && currentInterval > runner.MaxBackoffMilliSecs { + currentInterval = runner.MaxBackoffMilliSecs + } + } } } // If the error is not nil, return it and log the timeout message. Otherwise, generate new error.