http-429.sh records "already returned 429 once" with a "test -f"
followed by a "touch" of a shared state file. That check-then-act is not
atomic: Apache can run this CGI for several requests at once, and two of
them can both pass the "test -f" before either "touch"es, so both treat
themselves as the first request. The retry flow that drives this
endpoint is mostly sequential, so this has not been seen to fail, but
the race is latent.
Decide whether this is the first request with a single atomic mkdir,
which fails if the directory already exists, so exactly one of any
concurrent requests is rate-limited and the rest are forwarded.
Skipping state for "permanent" is required for correctness, not just an
optimization. The marker tells a later or concurrent request that a 429
has already been served, so that it forwards to git-http-backend instead
of rate-limiting. Since "permanent" must return 429 to every request,
that marker must never become visible to another such request.
The original did not achieve this by staying stateless: its "touch" of
the marker ran unconditionally, and the "permanent" case removed it
afterward with "rm -f". That create-then-remove leaves a window in which
a concurrent "permanent" request sees the marker and is forwarded. It is
the same class of check-then-act race this patch removes from the
first-request check, latent for the same reason: the flow is mostly
sequential. This version fuses the check and the mark into one atomic
mkdir and, rather than recreate the pattern as mkdir-then-rmdir, skips
the mkdir for "permanent" with a "!= permanent" guard. No marker is ever
created, so there is no window and every "permanent" request
rate-limits.
There is no accompanying regression test. The check and the set are
adjacent commands with no external step in between to synchronize on, so
the overlap cannot be forced deterministically, only reproduced
probabilistically; the fix is preventive.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ffb323e5b7