On Fri, May 28 2021, Bagas Sanjaya wrote: > On 28/05/21 19.11, Ævar Arnfjörð Bjarmason wrote: >> Let's also change this for the push-to-checkout hook. Now instead of >> checking if the hook exists and either doing a push to checkout or a >> push to deploy we'll always attempt a push to checkout. If the hook >> doesn't exist we'll fall back on push to deploy. The same behavior as >> before, without the TOCTOU race. See 0855331941b (receive-pack: >> support push-to-checkout hook, 2014-12-01) for the introduction of the >> previous behavior. > > What is TOCTOU? I never hear this term before. A common race condition[1], time of use, time of check. I.e. in this case we use the hook first, and then later we check if there's a hook. Thus we've got a logic error where we're assuming we'll get the same answer[2] in both instances. In this case what we plainly want is to just save away whether we did X, not do X, and then later check if we can do X to see if we did X earlier. I think this whole thing is still suspect even with my fix, there's an inherent assumption here of a single-thread view of the world. I.e. that you have a user running a git command at a time, and that while our hook run didn't change the index, a background process might have. But this probably handles the most common case, i.e. a hook explicitly changing the index and wanting its invoker to notice, and in any case we have the single-thread assumption already, so we might as well make it less buggy/racy. If you dig through the history we used to do this without the check for "earlier X?" before for at least one of the hooks, but it got changed to the current state as an optimization. 1. https://en.wikipedia.org/wiki/TOCTOU 2. I mean, I don't think any programmer assumed it in this case, it was just intentional lazyness or a legitimate trade-off, but now that it's easy to eliminate the race...
