Skip to content

pr-2171/mmontalbo/mm/lib-httpd-cgi-safe-proto-v4

tagged this 01 Sep 00:27
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 v3:

 * Rewrite all the prose in the series from scratch without AI to remove
   fluff.
 * Fix the lack of clarity around the actual fix applied to
   apply-one-time-script.sh, which ultimately has nothing to do with rm
   itself, but rather how rm is used in conjunction with the surrounding
   state management logic.
 * No logical behavior change.

[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                       | 11 ++++
 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, 142 insertions(+), 26 deletions(-)
 create mode 100755 t/t5567-one-time-script.sh

base-commit: 5b2471720c93ee30e5764a19f3d3b3ae9ec9712a

Submitted-As: https://lore.kernel.org/git/pull.2171.v4.git.1788222476.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.2171.git.1783479584.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.2171.v2.git.1783704657.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.2171.v3.git.1786583137.gitgitgadget@gmail.com
Assets 2
Loading