[BUG] DockerHub:MaxParallelRequests limits each client instance instead of the whole process #265

Summary

DockerUpdateGuard:DockerHub:MaxParallelRequests (default 4, range 1–32) is meant to cap how many Docker Hub requests DockerUpdateGuard has in flight. It is implemented as a SemaphoreSlim inside each DockerHubClient instance, and the client is a transient typed HttpClient that is created at least twice per DI scope, so every scan, every UI operation and both instances of one scope each get the full allowance. The real parallelism against Docker Hub is therefore unbounded by the setting, which increases the risk of HTTP 429 rate limiting, and a configuration reload has no effect on instances already alive.

Area

• docs/areas/registries.md (added in [Docs] Describe the current behavior by area #263): Docker Hub request budget and parallelism, Configuration reload
• Security area: None

Current behavior

• The throttle is a per-instance field: src/DockerUpdateGuard/DockerHub/DockerHubClient.cs:84 (private readonly SemaphoreSlim _requestThrottle;), sized once in the constructor from the current options (:114-115):

  var maxParallelRequests = Math.Max(1, optionsMonitor.CurrentValue.DockerHub.MaxParallelRequests);
  _requestThrottle = new SemaphoreSlim(maxParallelRequests, maxParallelRequests);

  SendThrottledAsync (:850-865) waits on it for web API, login, token service and registry requests.
• Registration: src/DockerUpdateGuard/ServiceCollectionExtensions.cs:73-79 registers AddHttpClient<DockerHubClient>(…), which makes DockerHubClient transient. :112 (IDockerHubClient) and :113 (IRegistryMetadataClient) are two separate scoped factories that each call GetRequiredService<DockerHubClient>(), so one scope holds two instances with two semaphores (see F-045 (P6) — Two DockerHubClient instances per scope → duplicated token cache (amplifies F-009) #6).
• Every background scan, the account discovery and every Blazor circuit work in their own scope, so N concurrent operations can have up to 2 × N × MaxParallelRequests Docker Hub requests in flight.
• A changed MaxParallelRequests only affects instances created after the reload; long-lived scopes (a Blazor circuit) keep the old value.
• OciRegistryClient (src/DockerUpdateGuard/Images/OciRegistryClient.cs, SendCoreAsync :1087-1106, GetBearerTokenAsync :1114) has no limit at all.

Failure scenario: set MaxParallelRequests to 1; start the scheduled observed-image scan and, at the same time, a manual scan from the UI and the Docker Hub account discovery. Each operation (and each of the two client instances inside it) sends its own request concurrently, so three or more Docker Hub requests are in flight at once although the setting says 1.

Expected behavior

Decision (product manager): MaxParallelRequests is a process-wide limit for Docker Hub, applied live when the configuration is reloaded.

• At any moment, the number of in-flight Docker Hub requests of the whole process (web API at ApiBaseUrl, login, auth.docker.io token service, registry-1.docker.io) does not exceed the current DockerHub:MaxParallelRequests, independent of how many scopes, scans, UI circuits or DockerHubClient instances exist.
• A reloaded value takes effect for the next request that waits for a slot, without a restart: raising it lets waiting requests proceed at once; lowering it lets in-flight requests finish and admits new ones only once the count is below the new limit.
• Waiting for a slot honors cancellation (a cancelled wait throws OperationCanceledException, as today).
• Requests to other registries (OciRegistryClient) are not covered by this decision; they stay unlimited and the area document says so explicitly.
• The Docker Hub request budget keys (Scanning:DockerHubRequestLimit*) stay a scheduling hint only (decided separately; no change).

Acceptance criteria

• Two DockerHubClient instances (or two DI scopes) sharing the process-wide limiter never have more than MaxParallelRequests requests in flight together (test with a blocking stub handler that counts concurrent calls).
• Raising MaxParallelRequests through a test IOptionsMonitor while requests wait admits more requests without recreating the client; lowering it admits no new request until the in-flight count drops below the new value.
• A cancelled wait for a slot throws OperationCanceledException and does not leak a slot.
• A request that fails (exception or non-success status) releases its slot.
• The limiter is registered as a singleton; DockerHubClient no longer owns a semaphore for parallelism (its Dispose no longer disposes one).
• Unit tests (MSTest, names per docs/UNIT_TESTS.md), for example DockerHubRequestLimiterConcurrentRequestsNeverExceedLimit, DockerHubRequestLimiterLimitRaisedOnReloadAdmitsWaitingRequests, DockerHubRequestLimiterLimitLoweredOnReloadAdmitsNoNewRequestUntilBelowLimit, DockerHubRequestLimiterCancelledWaitReleasesNoSlot, DockerHubClientTwoInstancesShareProcessWideLimit, plus a ServiceCollectionExtensionsTests case asserting the singleton registration.
• README configuration reference for MaxParallelRequests says "Maximum number of Docker Hub requests in flight at once for the whole process (1–32); applied on configuration reload".
• Format, build, tests, Analyzer gate and Coverage gate from .squad/stack.md pass.
• The area document docs/areas/registries.md describes the new behavior and its Known gaps entry for this issue is removed.

Implementation notes

• Affected: src/DockerUpdateGuard/DockerHub/DockerHubClient.cs (_requestThrottle, constructor, SendThrottledAsync, Dispose), src/DockerUpdateGuard/ServiceCollectionExtensions.cs, a new limiter type (one top-level type per file, #region blocks, XML docs), README.md, docs/ARCHITECTURE.md (Integration clients / configuration model if it mentions the limit).
• Options (recommendation first):
  1. Singleton limiter service (for example DockerHubRequestLimiter behind an interface) injected into DockerHubClient, which reads IOptionsMonitor<DockerUpdateGuardOptions>.CurrentValue.DockerHub.MaxParallelRequests on every acquire. A SemaphoreSlim cannot shrink, so implement the gate as a counter guarded by a lock plus a queue of waiters (TaskCompletionSource with RunContinuationsAsynchronously), or re-evaluate the limit on each release. Keep it small and fully unit-testable with no real clock.
  2. A DelegatingHandler on the DockerHubClient pipeline that uses the same singleton gate. Works too, but note handler ordering with TransientHttpRetryHandler: placed inside the retry handler each attempt takes a slot and retry waits do not hold one; placed outside, a request keeps its slot during Retry-After waits. Either is acceptable; document the choice in the area document.
• Pitfall: the Docker Hub login (RequestAccessTokenAsync) runs while _tokenRefreshLock is held and also takes a slot. With a process-wide limit of 1 this must not deadlock (the lock holder only waits for a slot, never the reverse); add a test with limit 1 and concurrent authenticated requests.
• This issue does not need to fix F-045 (P6) — Two DockerHubClient instances per scope → duplicated token cache (amplifies F-009) #6 (two instances per scope) or F-009 (P1) — DockerHub access token cache tied to scope lifetime #25 (token cache per scope), but the limiter must be correct regardless of instance count, so either fix can land independently.
• Configuration: no new key, no migration. MaxParallelRequests moves from "read when an instance is created" to "read live" in the area document's Configuration reload section; validation (1–32) stays.
• Must not change: TransientHttpRetryHandler semantics, RequestTimeoutSeconds as the total time per request including retries, and the outcome model of ExternalOperationResult<T> (.squad/project.md, Guarantees).

Related

• F-045 (P6) — Two DockerHubClient instances per scope → duplicated token cache (amplifies F-009) #6 (two DockerHubClient instances per scope)
• F-009 (P1) — DockerHub access token cache tied to scope lifetime #25 (Docker Hub token cache tied to scope lifetime)
• PR [Docs] Describe the current behavior by area #263 (area documents)

Generated by Claude Code