Taylor Blau <me@xxxxxxxxxxxx> writes: > On Fri, May 13, 2022 at 04:06:39PM -0700, Jonathan Tan wrote: > > (An alternative to the change in this patch may be to reset *found_pack > > to NULL when it is found that the pack is invalid, but I haven't > > investigated all the callers to see if they can tolerate *found_pack > > moving changing non-NULL to NULL, so the change in this patch is > > probably more practical.) > > I haven't either, but I think that this points out a flaw in the patch I > originally posted. > > Consider this: > > - `want_object_in_pack()` calls `want_found_object()` with a pack that > has gone away and has zero open fds, and `want_found_object()` > returns -1 > - `want_object_in_pack()` continues and calls > `want_object_in_pack_one()` later on, with some pack that is the > same as `*found_pack` > - `want_object_in_pack_one()` then _doesn't_ call `is_pack_valid()` > (since `p == *found_pack`), leaving us in the same situation as > before. > > I think that would be sufficient to hit this race even after this patch. Ah, yes, indeed this would be a problem. > I'll take a look to see if `want_object_in_pack()` callers can handle > `*found_pack` being set back to NULL. They should be able to, but I want > to do a little more careful analysis to be sure. Sounds good. > Thanks for pointing this out, I am so glad for your review! :-) Thanks for your kind words! Thanks for your explanations too.
