Re: [PATCH v2 3/3] t/README: document writing concurrency-safe helpers

From: Michael Montalbo <mmontalbo@gmail.com>
Date: Aug 7, 2026, 16:51 UTC
Message-ID: <CAC2Qwm+Jni+xU=gaef1AWCMj9+GUQhMrCWX9DFpS3y757pxv=Q@mail.gmail.com>
In-Reply-To: <anGcx4lRyy3jyS1D@pks.im>

On Tue, Aug 4, 2026 at 1:03 AM Patrick Steinhardt <ps@pks.im> wrote:

Show 10 quoted lines

>
> >
> > +Writing concurrency-safe helpers
> > +--------------------------------
>
> Nit: this paragraph is quite specific to lib-httpd, so it would make
> sense to mention it in the header here. E.g.
>
>     Writing concurrency-safe lib-httpd helpers
>

Originally, I did just have this as a blurb in t/lib-httpd.sh. I ended up moving it here and trying to make the advice apply more generally, though the only other existing example I could find in another domain was the make_symlink() reference. My intention was to make sure someone working on a test helper with concurrency didn't skip over the section just because they saw "http" and thought the advice didn't apply to their use case.

I'm inclined to make the language in the section more http-agnostic rather than changing the title to be specific to http, but I do not feel very strongly about it. If we were to frame this as http-specific advice maybe it should go back to t/lib-httpd.sh instead of t/README?

>
> A simple "rm" (without "-f") should work as well, right?
>

Yes, definitely. I think I over-corrected in excising "rm" from the test helpers and the advice given here since I associated it with the flawed patterns that allowed for the race issues. I will redo the treatment of "rm" in the series including reverting where "mv" replaced "rm" unnecessarily in the helpers.

Show 9 quoted lines

> > +A "$$" suffix on per-request scratch files keeps concurrent invocations
> > +from clobbering each other's fixed-name files.
>
> Nit: it might be a bit easier to read if we explicitly mention PIDs
> instead of assuming that every reader immediately knows that "$$" will
> expand to the PID. E.g.:
>
>     Appending a PID to the per-request scratch filenames keeps...
>

Agreed, will fix.

> Thanks!
>

Thank you for taking a look and your feedback!