Re: [PATCH 1/3] t/lib-httpd: fix apply-one-time-script race under concurrent requests

On Wed, Jul 8, 2026 at 12:54 PM Junio C Hamano [off-list ref] wrote:

"Michael Montalbo via GitGitGadget" [off-list ref] writes:

quoted

+#
+# Apache can run this CGI for concurrent requests (for example a partial fetch
+# that lazily fetches a missing object while the first response is still in
+# flight), so the helper claims the marker atomically with a rename, and only
+# once it has decided to modify the response. A request that loses the race
+# finds the marker already gone and serves its response unchanged; no request
+# is left emitting an empty body, which the server would report as HTTP 500.
+# Scratch files are per-request ($$) so concurrent requests do not clobber each
+# other.
+
+test -f one-time-script || exec "$GIT_EXEC_PATH/git-http-backend"

-     "$GIT_EXEC_PATH/git-http-backend" >out
-     ./one-time-script out >out_modified
+LC_ALL=C
+export LC_ALL

The original was somehow inconsistent in that it forced C locale
only when one-time-script munged the output, and otherwise the
backend was run in the original locale.  I am not sure if that
matters very much.

I think it's still the same after the rewrite, though I could be
mistaken. If the
first `test -f` fails git-http-backend executes with inherited locale
(analogous to
the else branch execution in the original), and if `test -f` succeeds the locale
is forced to C and the one-time-script / git-http-backend run with the forced
locale. That being said, I think forcing the locale to C consistently would
make more sense. Depending on what you think, I can integrate that into the
series or leave for a future cleanup.

Ah, we assume running one-time-script itself multiple times is safe
and does not cause issues.  Our objective is to avoid returning
modified output twice.  So while the first instance of us
successfully renames one-time-script to one-time-script.$$ and emits
the modified result, even if the second instance raced and managed
to run the script again, it will fail to rename with "mv", and
discard the modified output, and instead show the unmodified output
generated by the backend.

OK.  It is a bit tricky.  It may help future readers if we said
something about this in the proposed log message (i.e., we consider
that it is perfectly fine to run one-time-script more than once; we
only want to avoid letting the second invocation's output used).

Yes that is a good call, I will add some detail about this subtlety in the
log message and helper comment.