Re: [PATCH] index-pack: spawn threads atomically
- From
Jeff King <peff@peff.net>
- Date
- Jan 10, 2024, 11:44 UTC
- Message-ID
- <20240110114456.GF16674@coredump.intra.peff.net>
- In-Reply-To
- <ZZgvUyQK6X/MacDC@nand.local>
On Fri, Jan 05, 2024 at 11:33:23AM -0500, Taylor Blau wrote:
Show 7 quoted lines
> - test_must_fail git index-pack --fix-thin --stdin <recoverable.pack > + test_must_fail git index-pack --threads=1 --fix-thin --stdin <recoverable.pack > [...] > For what it's worth, I'm fine with either approach, mostly to avoid > tying up more of the list's time discussing the options. But I have a > vague preference towards `--threads=1` since it doesn't require us to > touch production code.
That's quite tempting, actually. The flip side, though, is that the test no longer reflects the production code as well. That is, in the real world we'd still call exit() from a thread. That obviously works OK now (modulo LSan), but if we ever had a regression where that left us in an inconsistent state, we'd be less likely to notice it. Feels kind of unlikely in practice, though.
I dunno. I guess the real least-bad thing is seeing if LSan can be fixed to handle this atomically. I haven't even reported it there.
If do go with "--threads=1", I suspect several tests in that file need it.
-Peff