Scoop icon indicating copy to clipboard operation
Scoop copied to clipboard

Add file lock to prevent concurrent executions

Open KoltesDigital opened this issue 1 month ago • 4 comments

Description

This patch adds a file lock to ensure only one process uses Scoop. Extra processes wait 1 second and retry, so dequeuing is not handled in order.

The diff shows a lot of modified lines, but it's actually just an indent increase due to the try-finally block.

Motivation and Context

I've created a scheduled task to automate scoop update * every morning or at every startup. Whether it's good or bad is another topic :) But I've noticed some conflicts. For instance, scoop list shows install failed while the app is being installed. I suspect that I got corrupted git-buckets due to parallel updates too.

This is also in line with my expectations of a service. IMHO Scoop acts like a session-wide service that several processes can access, so it must ensure it's safe to do so. The fact that Scoop exclusively works with PS1 scripts is an implementation detail.

Note that global installs made by different process owners could still conflicts. A better version would be to pinpoint which commands are subjects to lock, and in these cases whether the lock should be global. This would also have the benefit not to lock for things like displaying help. I'd love to have that, but there's more effort to put, so please tell me what you think first.

How Has This Been Tested?

Locally with multiple shells.

Checklist:

  • [ ] I have read the Contributing Guide.
  • [X] I have ensured that I am targeting the develop branch.
  • [ ] I have updated the documentation accordingly.
  • [ ] I have updated the tests accordingly.
  • [X] I have added an entry in the CHANGELOG.

Summary by CodeRabbit

  • New Features

    • Adds a file locking mechanism to prevent concurrent executions; callers see a one-time waiting message if a run is already active.
  • Bug Fixes

    • Ensures the lock is always released after execution.
    • Unknown or unsupported commands now emit a clear warning and exit with a non-zero status.
  • Chores

    • Updated ignore rules to exclude lock files.

KoltesDigital avatar Nov 17 '25 14:11 KoltesDigital

Walkthrough

Implements a file-based exclusive lock in bin/scoop.ps1, adds a .lock ignore to .gitignore, and documents the change in CHANGELOG.md. The script prints a single waiting message while retrying exclusive open, wraps subcommand dispatch in try/finally to always release the lock, and warns on unknown commands.

Changes

Cohort / File(s) Summary
Configuration
\.gitignore
Adds rule to ignore a .lock file.
Changelog
CHANGELOG.md
Adds Unreleased entry: core: Add file lock to prevent concurrent executions.
CLI Locking & Control Flow
bin/scoop.ps1
Implements exclusive file lock with a non-blocking retry loop that prints a single waiting message; wraps subcommand dispatch in try/finally to ensure lock disposal; restores alias re-aliasing inside the locked region; adds unknown-command warning and exit code 1; restructures command dispatch flow.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Script as scoop.ps1
    participant LockFile as Lock File
    participant Command as Subcommand

    User->>Script: invoke scoop command
    Script->>LockFile: attempt exclusive open (write)
    alt Lock unavailable
        Note over Script,LockFile `#f6f0d9`: print waiting message once\nretry until writable
        LockFile-->>Script: lock acquired
    else Lock acquired immediately
        LockFile-->>Script: lock acquired
    end
    Script->>Script: restore aliases / re-aliasing
    Script->>Command: dispatch selected subcommand
    Command-->>Script: returns result or error
    Script->>LockFile: close/dispose lock (finally)
    Script-->>User: exit (success or code 1 for unknown command)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Review focus:
    • bin/scoop.ps1 lock acquisition loop (single waiting message, retry/backoff behavior).
    • Guaranteed disposal of the lock stream across success, failure, and early exits.
    • Correct placement and effect of alias re-aliasing inside the locked region.
    • Unknown-command handling and appropriate exit code propagation.

Poem

🐰 I found a tiny padlock bright,
I waited once beneath the light,
No clashing paws, the path is clear,
One hop, one lock — no need to fear. 🔒

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'Add file lock to prevent concurrent executions' directly and clearly summarizes the main change: implementing a file-based lock mechanism in the scoop.ps1 script to prevent multiple concurrent Scoop processes from running simultaneously.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Nov 17 '25 14:11 coderabbitai[bot]

Question: is there a shared persistent state that Scoop reads/writes when updating or installing apps? As far as I know Scoop was one of the few apps that supported doing multiple installs/updates (of different apps of course) at the same time.

Running a scheduled task for update is fine, but what's the use case of running list or status while the apps are getting updated?

rashil2000 avatar Nov 18 '25 14:11 rashil2000

@rashil2000 more on the context, to be honest... and also to get feedback from you :)

Thought I'm a developer, I'm dealing with IT for a small company, so I'm trying to use my dev habits to manage PCs. Workstations run Windows, and we have a couple of in-house apps. I've chosen to install them and keep them updated through Scoop. Installation itself is automatized through Intune depending on groups the user or machine belong to, but it can't handle updates. So the first thing Intune does is install Scoop, and then Intune only install apps through Scoop. This allows user to install extra apps. Intune also installs a scheduled task as described, to get things up-to-date. We're not big enough to get fixed, verified versions of apps... so I prefer to always have the latest versions, with the tiny risk that an app may break.

Last detail: Intune can install its things as device or as user, and it's kind of a nightmare regarding permissions. When as device, install process belongs to SYSTEM account, and is elevated. When as user, install process belongs to the user, and is automatically elevated iff the user is admin. This last point is really painful: user context is not the same for all users. For instance, when Intune installs Scoop for admin users, buckets (Git repos) somehow get wrong permissions, and non-elevated shells cannot use Scoop anymore... so there are some tricks to get back on track.

Because Scoop takes care of updates, I don't have to deal with that on my apps, and that's great. Whenever CICD pushes a new version, scoop update ... does the job. However, my users won't run Scoop. That's why the scheduled task runs it every morning. However, we can have several updates per day, most of them are bugfixes to the previous one, so we can't wait the next morning. So in the main app, there's a regular check to its manifest (directly through https) to see whether there's a new version, and if so, offers to the user to run scoop update <app> under the hood. Well I know, it's not far away from reimplementing a whole update process... but at least Scoop does a part of that.

Many users reported many times that the app couldn't launch anymore. My assumption is that they launched the app right at startup, the app tells them to update it, so they do it, but the scheduled task is also doing it, and boom. Thankfully, the fix is simply uninstall and reinstall.

list or status were just examples, they show an incorrect state but it's harmless. Running concurrent updates is not. If you're willing to consider a dedicated lock for each app, that would be awesome. Please tell me and I can try to do it. BTW I've put the lock at the repo's root, but if we're getting a bunch of them, another location surely is better, please tell me an advice.

KoltesDigital avatar Nov 18 '25 15:11 KoltesDigital

This should ideally be done on a per-app basis. The SCOOP/apps/<app-name>/ is one location to place the lock in.

We can also show the status 'Updating' in the list or status command if the lock file exists for that app.

rashil2000 avatar Nov 23 '25 16:11 rashil2000