From: Michael Montalbo <mmontalbo@gmail.com>
Date: Tue, 01 Sep 2026 14:28:38 GMT
Subject: Re: [PATCH v4 3/3] t/lib-httpd: document writing concurrency-safe CGI helpers
Message-ID: <CAC2Qwm+dOZedmzzui5TSKJ1FNEiDypwUwVqGBgttG_OvDWQBBg@mail.gmail.com>
In-Reply-To: <apa0N7VNNkcKurbi@pks.im>

On Tue, Sep 1, 2026 at 4:17 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Tue, Sep 01, 2026 at 12:27:56AM +0000, Michael Montalbo via GitGitGadget wrote:
> > diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
> > index a216e5376f..8ca09fe85b 100644
> > --- a/t/lib-httpd.sh
> > +++ b/t/lib-httpd.sh
> > @@ -159,6 +159,17 @@ prepare_httpd() {
> >       mkdir -p "$HTTPD_DOCUMENT_ROOT_PATH"
> >       cp "$TEST_PATH"/passwd "$HTTPD_ROOT_PATH"
> >       cp "$TEST_PATH"/proxy-passwd "$HTTPD_ROOT_PATH"
> > +     # Apache can run the following scripts concurrently per request. Make
> > +     # sure any state management logic is resilient to race conditions.
> > +     #
> > +     # For example:
> > +     #   - 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
>
> Nit: I would have written "with the logic that is guarded by the marker"
> instead of "claims".
>

That makes more sense, the current version is  circular (rm is the logic doing
the claiming). Will fix.

> > +     #     "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
> >       install_script incomplete-body-upload-pack-v2-http.sh
> >       install_script error-no-report.sh
>
> Other than that the whole series reads a lot better now, thanks.
>

Thank you for the call out and taking another look. I really appreciate your
feedback!