[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    Re: [PATCH] config: use a static lock_file struct
From:       Jeff King <peff () peff ! net>
Date:       2017-08-31 4:09:07
Message-ID: 20170831040907.a4qpydb3w2aghatm () sigill ! intra ! peff ! net
[Download RAW message or body]

On Wed, Aug 30, 2017 at 02:06:02PM -0700, Brandon Williams wrote:

> > We could extend that protection by having sigchain_push_common() set
> > sa_mask to cover all of the related signals. On Linux and BSD the
> > current code using signal() also implies SA_RESTART. We could add that
> > to our flags, though I suspect in practice it doesn't matter. Whenever
> > we establish a handler like this our intent is to never return from it.
> > 
> > That just protects us from calling the _same_ handler from itself. But
> > that's probably enough in practice, and means handlers don't have to
> > worry about "critical sections". The other alternative is sigprocmask()
> > to block signals entirely during a section. I'm not sure if there are
> > portability questions there (it looks like we have a mingw wrapper
> > there, but it's a complete noop).
> 
> Yeah there's a lot about signals that I'm not very clear on.  I do know
> that Eric helped me out on the fork-exec series I worked on earlier in
> the year and I believe it was to turn on/off signals during process
> launches in 45afb1ca9 (run-command: block signals between fork and
> execve, 2017-04-19).  Though that bit of code is strictly for unix so I
> wouldn't know how that would work on windows machines.  Portability does
> seem to always be a challenging problem.

Based on the sketch I wrote above, I figured it would be pretty easy to
convert sigchain to sigaction. But after taking a look at
compat/mingw.c, I don't think Windows would be on board. sigaction()
there is is a stub implementation that _only_ handles SIGALRM and
nothing else.

So I think the best we could do is put big #ifdefs around it to use
sigaction on other platforms, and fall back to signal() on Windows.
That's do-able, but my enthusiasm is waning as the complexity increases.
Getting two SIGINTs in a row seems plausible, but we already handle that
well. Getting SIGINT and then a _different_ signal while we're in the
handler seems less likely in practice. The only combination I can think
that would be common is TERM+KILL, but of course we're not catching KILL
in the first place.

So this seems like adding complexity for very little benefit.

-Peff
[prev in list] [next in list] [prev in thread] [next in thread]