Skip to content

Add file lock to prevent concurrent executions #6557

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
KoltesDigital wants to merge 1 commit into
base: develop
Choose a base branch
from
+63 −35
Open

Add file lock to prevent concurrent executions #6557

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
*.log
.DS_Store
._.DS_Store
.lock
scoop.sublime-workspace
test/installer/tmp/*
test/tmp/*
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- **install:** Add separator at the end of notes, highlight suggestions ([#6418](https://github.com/ScoopInstaller/Scoop/issues/6418))
- **download|scoop-download:** Add GitHub issue prompt when the default downloader fails ([#6539](https://github.com/ScoopInstaller/Scoop/issues/6539))
- **download|scoop-config:** Allow disabling automatic fallback to the default downloader when Aria2c download fails ([#6538](https://github.com/ScoopInstaller/Scoop/issues/6538))
- **core:** Add file lock to prevent concurrent executions ([#6557](https://github.com/ScoopInstaller/Scoop/issues/6557))

### Bug Fixes

Expand Down
96 changes: 61 additions & 35 deletions bin/scoop.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,48 +6,74 @@ Set-StrictMode -Off
. "$PSScriptRoot\..\lib\commands.ps1"
. "$PSScriptRoot\..\lib\help.ps1"

$subCommand = $Args[0]
$lock_file_path = "$PSScriptRoot\..\.lock"
$lock_stream = $null
$lock_show_waiting_message = $true

# for aliases where there's a local function, re-alias so the function takes precedence
$aliases = Get-Alias | Where-Object { $_.Options -notmatch 'ReadOnly|AllScope' } | ForEach-Object { $_.Name }
Get-ChildItem Function: | Where-Object -Property Name -In -Value $aliases | ForEach-Object {
Set-Alias -Name $_.Name -Value Local:$($_.Name) -Scope Script
while (-not $lock_stream) {
try {
$lock_stream = [System.IO.File]::Open($lock_file_path, [System.IO.FileMode]::Create, [System.IO.FileAccess]::Write)
} catch [System.IO.IOException] {
# Only deal with ERROR_SHARING_VIOLATION or ERROR_LOCK_VIOLATION.
$error_code = $_.Exception.HResult -band 0xFFFF
if ($error_code -notin 32, 33) {
throw
}

if ($lock_show_waiting_message) {
Write-Host 'Waiting for exclusive access...'
$lock_show_waiting_message = $false
}
Start-Sleep -Seconds 1
}
}
Comment on lines +9 to 29
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Tighten lock acquisition error handling to avoid masking non-contention failures

The lock loop works correctly for normal contention, but the generic catch means any failure opening the lock file (e.g. permission error, invalid path) will cause an infinite retry with only a single "Waiting for exclusive access..." line:

$lock_file_path = "$PSScriptRoot\..\.lock"
$lock_show_waiting_message = $true

while (-not $lock_stream.CanWrite) {
    try {
        $lock_stream = [System.IO.File]::Open($lock_file_path, [System.IO.FileMode]::Create, [System.IO.FileAccess]::Write)
    } catch {
        if ($lock_show_waiting_message) {
            'Waiting for exclusive access...'
            $lock_show_waiting_message = $false
        }
        Start-Sleep -Seconds 1
    }
}

That makes genuine configuration/IO problems look like normal queueing and leaves the user with a hung scoop process and no actionable error.

It would be safer to:

  • Only retry for contention-style errors (e.g. System.IO.IOException from a sharing violation), and
  • Let other exceptions bubble to the caller so the failure is visible.
  • Emit the waiting message via warn (or similar) so it doesn’t pollute the success pipeline.

For example:

-$lock_file_path = "$PSScriptRoot\..\.lock"
-$lock_show_waiting_message = $true
-
-while (-not $lock_stream.CanWrite) {
-    try {
-        $lock_stream = [System.IO.File]::Open($lock_file_path, [System.IO.FileMode]::Create, [System.IO.FileAccess]::Write)
-    } catch {
-        if ($lock_show_waiting_message) {
-            'Waiting for exclusive access...'
-            $lock_show_waiting_message = $false
-        }
-        Start-Sleep -Seconds 1
-    }
-}
+$lock_file_path = "$PSScriptRoot\..\.lock"
+$lock_show_waiting_message = $true
+$lock_stream = $null
+
+while ($true) {
+    try {
+        $lock_stream = [System.IO.File]::Open(
+            $lock_file_path,
+            [System.IO.FileMode]::Create,
+            [System.IO.FileAccess]::Write,
+            [System.IO.FileShare]::None
+        )
+        break
+    } catch [System.IO.IOException] {
+        if ($lock_show_waiting_message) {
+            warn 'Waiting for exclusive access...'
+            $lock_show_waiting_message = $false
+        }
+        Start-Sleep -Seconds 1
+    }
+}

This preserves the 1‑second retry behavior for true contention, but fails fast (with a clear error) on misconfiguration or other unexpected IO problems, and keeps the waiting message off the success pipeline.

🤖 Prompt for AI Agents 
In bin/scoop.ps1 around lines 9 to 22, the catch block currently swallows all
exceptions and prints a single plain string, causing infinite retries on
non-contention errors and polluting success output; change the logic to only
retry for contention-style IO errors (i.e. when the exception is a
System.IO.IOException indicating a sharing violation), rethrow any other
exceptions so they surface to the caller, and emit the waiting message with
Write-Warning (or similar) instead of writing to stdout; keep the 1-second retry
loop for true contention but fail fast for other IO/configuration errors.


switch ($subCommand) {
({ $subCommand -in @($null, '-h', '--help', '/?') }) {
exec 'help'
try {
$subCommand = $Args[0]

# for aliases where there's a local function, re-alias so the function takes precedence
$aliases = Get-Alias | Where-Object { $_.Options -notmatch 'ReadOnly|AllScope' } | ForEach-Object { $_.Name }
Get-ChildItem Function: | Where-Object -Property Name -In -Value $aliases | ForEach-Object {
Set-Alias -Name $_.Name -Value Local:$($_.Name) -Scope Script
}
({ $subCommand -in @('-v', '--version') }) {
Write-Host 'Current Scoop version:'
if ((Test-GitAvailable) -and (Test-Path "$PSScriptRoot\..\.git") -and ((get_config SCOOP_BRANCH 'master') -ne 'master')) {
Invoke-Git -Path "$PSScriptRoot\.." -ArgumentList @('--no-pager', 'log', 'HEAD', '-1', '--oneline')
} else {
$version = Select-String -Pattern '^## \[(v[\d.]+)\].*?([\d-]+)$' -Path "$PSScriptRoot\..\CHANGELOG.md"
Write-Host $version.Matches.Groups[1].Value -ForegroundColor Cyan -NoNewline
Write-Host " - Released at $($version.Matches.Groups[2].Value)"

switch ($subCommand) {
({ $subCommand -in @($null, '-h', '--help', '/?') }) {
exec 'help'
}
Write-Host ''

Get-LocalBucket | ForEach-Object {
$bucketLoc = Find-BucketDirectory $_ -Root
if ((Test-GitAvailable) -and (Test-Path "$bucketLoc\.git")) {
Write-Host "'$_' bucket:"
Invoke-Git -Path $bucketLoc -ArgumentList @('--no-pager', 'log', 'HEAD', '-1', '--oneline')
Write-Host ''
({ $subCommand -in @('-v', '--version') }) {
Write-Host 'Current Scoop version:'
if ((Test-GitAvailable) -and (Test-Path "$PSScriptRoot\..\.git") -and ((get_config SCOOP_BRANCH 'master') -ne 'master')) {
Invoke-Git -Path "$PSScriptRoot\.." -ArgumentList @('--no-pager', 'log', 'HEAD', '-1', '--oneline')
} else {
$version = Select-String -Pattern '^## \[(v[\d.]+)\].*?([\d-]+)$' -Path "$PSScriptRoot\..\CHANGELOG.md"
Write-Host $version.Matches.Groups[1].Value -ForegroundColor Cyan -NoNewline
Write-Host " - Released at $($version.Matches.Groups[2].Value)"
}
Write-Host ''

Get-LocalBucket | ForEach-Object {
$bucketLoc = Find-BucketDirectory $_ -Root
if ((Test-GitAvailable) -and (Test-Path "$bucketLoc\.git")) {
Write-Host "'$_' bucket:"
Invoke-Git -Path $bucketLoc -ArgumentList @('--no-pager', 'log', 'HEAD', '-1', '--oneline')
Write-Host ''
}
}
}
}
({ $subCommand -in (commands) }) {
[string[]]$arguments = $Args | Select-Object -Skip 1
if ($null -ne $arguments -and $arguments[0] -in @('-h', '--help', '/?')) {
exec 'help' @($subCommand)
} else {
exec $subCommand $arguments
({ $subCommand -in (commands) }) {
[string[]]$arguments = $Args | Select-Object -Skip 1
if ($null -ne $arguments -and $arguments[0] -in @('-h', '--help', '/?')) {
exec 'help' @($subCommand)
} else {
exec $subCommand $arguments
}
}
default {
warn "scoop: '$subCommand' isn't a scoop command. See 'scoop help'."
exit 1
}
}
default {
warn "scoop: '$subCommand' isn't a scoop command. See 'scoop help'."
exit 1
}
} finally {
$lock_stream.Dispose()
}
Comment on lines +77 to 79
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add null check before disposing the lock stream.

If an unexpected exception occurs during lock acquisition (before $lock_stream is successfully assigned), the finally block will attempt to call Dispose() on a null object, which will throw a NullReferenceException and mask the original exception.

Apply this diff to add a defensive null check:

 } finally {
-    $lock_stream.Dispose()
+    if ($null -ne $lock_stream) {
+        $lock_stream.Dispose()
+    }
 }
🤖 Prompt for AI Agents 
In bin/scoop.ps1 around lines 77 to 79, the finally block disposes $lock_stream
without checking for null which can throw if assignment failed; update the
finally block to check if $lock_stream is not $null before calling Dispose()
(i.e., wrap Dispose() call in an if ($lock_stream -ne $null) {
$lock_stream.Dispose() } or use conditional invocation) to avoid masking the
original exception.