The account quota is check-then-act, so two concurrent runs can both pass a ceiling only one fits under

[lex00]

Found reviewing internal/vms for #54. Structurally present, not demonstrated, and not dismissible.

The shape

internal/vms/handlers.go, the RunMicrovm path:

allocated, live := h.svc.Allocated(region)      // reads under s.mu, releases it
if !h.quota.AllowMemory(allocated, want) || !h.quota.AllowMicrovm(live) {
    limits.WriteQuotaExceeded(w, "")
    return
}
vm := h.svc.Run(region, arn, version, …)        // takes the lock again to Put

Allocated walks the collection under s.mu and releases it before returning. The decision is made on a value that is already historical, and Run publishes without rechecking. Two concurrent runs that both read allocated = 2048 against a 4096 ceiling, each wanting 2048, both pass — and 6144 is allocated.

The default ceiling makes this tighter than it sounds: DefaultAccountMemoryMiB is 4096, which at the 2048 default tier is exactly two VMs. A consumer creating a replica pool is issuing precisely the concurrent runs that would race.

What I could not show

64 requests released from a sync.WaitGroup barrier over pre-warmed connections, against a server started with -max-microvms 1:

status tally: map[200:1 402:63]
non-terminal VMs on the server: 1

Exactly one succeeded, every time. The window between Allocated releasing the lock and Run's Put acquiring the store lock is a handful of instructions with no allocation or syscall in it, so two goroutines have to be preempted inside it. A slower machine, a GC pause or a scheduler decision would widen it; nothing here guarantees it stays shut.

So this is not "a bug that bites today". It is a check whose correctness rests on a window being small rather than on anything holding it closed.

Why it is not a small patch

Closing it means the quota decision and the create happen under one acquisition of the service mutex — which means Run has to know about the quota, or the service has to grow a RunIfWithinQuota. That is an API change and a layering decision (limits currently sits above vms, and this would invert part of that), not a lock moved two lines.

Options, roughly:

1. vms.Service grows a quota interface and checks inside the same lock as the Put. Correct, and puts a policy concern inside the resource package.
2. limits owns a reservation the handler takes before Run and releases on failure. Keeps the layering, adds a second thing to keep in step with terminate.
3. Leave it, and say so in docs/scope.md — an emulator whose quota is advisory under concurrency is a defensible thing to be, as long as a consumer testing quota behaviour knows not to trust it under load.

Option 3 is not obviously wrong. It is only wrong silently.

Related

The same shape may apply to MaxConcurrentSnapshotCreates on the image build path — not checked.