Junio C Hamano <gitster@xxxxxxxxx> wrote: > Johannes Sixt <j6t@xxxxxxxx> writes: > > > Quite frankly, I don't quite know what to do with this series. On the > > one hand, it is a clean-up, but in practice it is not relevant whether > > die() kills only the async thread or the whole process because all > > callers of async die() themselves anyway when the async procedure died. > > On the other hand, it does enable threaded async procedures on POSIX... ... > . It must not change the program's state that the caller of the > facility also uses. > > And calling die() from async is obviously "change the program's state that > the caller of the facility also uses". We didn't uncover this as a bug > because the above "serious restrictions" go both ways. I agree with you Junio. I think that any async helper that is invoking die() in its code path is wrong. Just like its also wrong for an async helper to try and use the sha1_file.c family of functions. The helper should return failure and let its caller handle the process termination. Hell, its even wrong for an async helper to use xmalloc(), because in a low-memory situation that xmalloc may try to remove pack windows *without locking*. _DOUBLE_PLUS_UNGOOD_ > If we make threaded-async the default on any platform that is thread > capable, we would increase the likelihood of catching bugs that violate > the latter condition. I'm in favor of that. If we have threaded delta search enabled, we probably can also run these async procedures in a POSIX thread rather than forking off a child. Though on our primary target platform of Linux, the performance difference either way probably cannot be measured. -- Shawn. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html