On Mon, Jan 22, 2024 at 11:32:14AM +0100, Patrick Steinhardt wrote: > > I didn't think too hard about the details, but does this mean that > > every user of stat_validity_check() has the same issue? The other big > > one is packed-refs (for which the code was originally written). Should > > this fix go into that API? > > In theory, the issue is the same for the `packed-refs` file. But in > practice it's much less likely to be an issue: Thanks for laying this all out. It does concern me a little that there's still a possible race, because they can be so hard to catch and debug in practice. But I think you make a compelling argument that it's probably not happening a lot in practice, and especially... > Also, applying the same fix for the packed-refs would essentially mean > that the caching mechanism is now ineffective on Windows systems where > we do not have proper `st_dev` and `st_ino` values available. I think > this is a no-go in the context of packed-refs because we don't have a > second-level caching mechanism like we do in the reftable backend. It's > not great that we have to reread the "tables.list" file on Windows > systems for now, but at least it's better than having to reread the > complete "packed-refs" file like we'd have to do with the files backend. ...here that the performance profile is so different. If the "fix" means re-reading the whole packed-refs file constantly, that's going to be quite noticeable. Given that this race has been here forever-ish, I agree with you that we should leave it be. -Peff
