[PATCH v5 0/3] t/lib-httpd: make CGI test helpers concurrency-safe

t/lib-httpd.sh provides several helpers that can be invoked concurrently by Apache while exercising tests. Currently, two of these helpers use state management logic that fails under certain race conditions.

apply-one-time-script.sh is one of those test helpers. It executes a "one-time-script" responsible for modifying the response normally returned by git-http-backend. Sometimes a race between multiple concurrent requests causes apply-one-time-script.sh to misbehave and return multiple modified responses or an empty response that results in:

fatal: ... The requested URL returned error: 500 fatal: could not fetch from promisor remote

This can be seen in the flaky failure of t5616.47 on the macOS CI runners[1].

Fix this by chaining (&&) the logic for executing "one-time-script" with its removal, rather than running them as separate actions. Add t/t5567-one-time-script.sh to verify this fix is effective.

http-429.sh is the other helper whose state management logic can fail under certain race conditions. However, these failures do not manifest themselves currently since http-429.sh is invoked sequentially.

As a preventive measure, fix http-429.sh's state management logic so it relies on an atomic mkdir operation to mark that a 429 was returned rather than separate "test -f marker", "touch marker", and "rm -f marker" actions to manage state. http-429.sh is not as straightforward to test as apply-one-time-script.sh, which is why no regression test was added for the change.

Finally, document these patterns and anti-patterns in t/lib-httpd.sh for future developers.

Changes since v4:

 * Reword advice about chaining (&&) atomic operations like rm so it refers
   to chaining with "the logic guarded by the marker" instead of "the logic
   that claims the marker" since the latter is circular and inaccurate
   (atomic operations like rm are the logic that claims markers).

[1] https://github.com/gitgitgadget/git/actions/runs/28756172690/job/85263916762?pr=2169

Michael Montalbo (3):
  t/lib-httpd: fix apply-one-time-script race under concurrent requests
  t/lib-httpd: make http-429 first-request check atomic
  t/lib-httpd: document writing concurrency-safe CGI helpers

 t/lib-httpd.sh                       | 12 ++++
 t/lib-httpd/apply-one-time-script.sh | 38 +++++++----
 t/lib-httpd/http-429.sh              | 22 +++----
 t/meson.build                        |  1 +
 t/t5567-one-time-script.sh           | 96 ++++++++++++++++++++++++++++
 5 files changed, 143 insertions(+), 26 deletions(-)
 create mode 100755 t/t5567-one-time-script.sh

base-commit: 5b2471720c93ee30e5764a19f3d3b3ae9ec9712a
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2171%2Fmmontalbo%2Fmm%2Flib-httpd-cgi-safe-proto-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2171/mmontalbo/mm/lib-httpd-cgi-safe-proto-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/2171

Range-diff vs v4:

 1:  e202142f19 = 1:  e202142f19 t/lib-httpd: fix apply-one-time-script race under concurrent requests
 2:  79396d491f = 2:  79396d491f t/lib-httpd: make http-429 first-request check atomic
 3:  d8d11ad246 ! 3:  75a184ca09 t/lib-httpd: document writing concurrency-safe CGI helpers
     @@ t/lib-httpd.sh: prepare_httpd() {
      +	#   - use "mkdir dir" to ensure only one request "succeeds" under some
      +	#     condition (see http-429.sh).
      +	#   - chain (&&) atomic operations like "rm marker" (no -f) with the
     -+	#     logic that "claims" the marker instead of relying on a separate
     -+	#     "test -f" and "rm marker" check (see apply-one-time-script.sh).
     ++	#     logic that is guarded by the marker instead of relying on a
     ++	#     separate "test -f" and "rm marker" check
     ++	#     (see apply-one-time-script.sh).
      +	#   - use scratch file names that include the process ID ($$), so
      +	#     concurrent requests do not overwrite each other's state.
       	install_script incomplete-length-upload-pack-v2-http.sh