On Thu, Mar 27, 2014 at 9:20 AM, Marc Herbert <[email protected]> wrote: > [ Karl, I think the discussion is worth moving back to the list at > this stage - hope you don't mind]
Oh, not at all---I just didn't want to spam everyone. > 2014-03-27 9:35 GMT+00:00 Karl Wiberg <[email protected]>: > > > Hmm, OK. I see---close isn't as synchronous as I thought when it's > > a socket or pipe that we're closing. But I still don't see what > > problems could be caused by closing before killing. Sure, it's a > > race---in the process we're killing. And that process was just > > reading things (it was writing to the pipe, but we just closed > > that), so the race can't affect anything. > > OK: the fact that this sub-process has no side-effect is definitely > a Good Thing. > > However, assuming the worst you could still have a problem where the > race condition in git cat-file triggers something and makes > it... hang - just like the initial problem! > > Shutting down "distributed systems" correctly is never easy and > typically requires time-outs somewhere. And it's extremely hard to > test fully. > > After looking a tiny bit more at the code I think the (fairly rich!) > subprocess wrapper class "run.py:Run" is surprisingly missing a > "kill()" method. This is where the solution to such problems should > be centralized and a time-out *could* be implemented. I certainly wouldn't object if anyone attempted to make the Run class more generally robust. Historically, very little robustness has been needed because the git commands are very well-behaved, but as this recent bug shows, even usually well-behaved programs can turn into delinquents in the wrong environment. -- Karl Wiberg, [email protected] subrabbit.wordpress.com www.treskal.com/kalle _______________________________________________ stgit-users mailing list [email protected] https://mail.gna.org/listinfo/stgit-users
