Skip to content

daemon: explicitly allow EINTR during poll()

Carlo Marcelo Arenas Belón via GitGitGadget git 4 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v4 Latest2 patches 6 repliesIn review
  2. v34 patches 17 repliesSuperseded
  3. v23 patches 13 repliesSuperseded
Earlier versions · 1
VersionPostedPatchesRepliesStatus
v13 patches10 repliesSuperseded

Changes in v4 from the cover letter

* Remove patches 1 and 4 as suggested by Phillip, and setup the signal without SA_RESTART instead.

Patches in v4 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/2compat/mingw: allow sigaction(SIGCHLD)compat/mingw-posix.h compat/mingw.c+4 -11 reply
2/2daemon: use sigaction() to install child_handler()daemon.c+7 -50 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v4 yet.

Read the v4 thread

Cover letter [PATCH v4 0/2] daemon: explicitly allow EINTR during poll()

This series addresses and ambiguity that is at least visible in OpenBSD, where zombie proceses would only be cleared after a new connection is received.

The underlying problem is that when this code was originally introduced, SA_RESTART was not widely implemented, and the signal() call usually implemented SysV like semantics, at least until it started being reimplemented by calling sigaction() internally.

Changes since v3

* Remove patches 1 and 4 as suggested by Phillip, and setup the signal without SA_RESTART instead.

Changes since v2

* Add a new patch 2 that modifies windows' sigaction so there is no more need for a fallback
* Hopefully no more silly mistakes and a variable that finally makes sense

Changes since v1

* Almost all references to siginterrupt has been removed and a better named variable used instead
* Changes had been abstracted to minimize ifdefs and their introduction staged more naturally

Carlo Marcelo Arenas Belón (2):
compat/mingw: allow sigaction(SIGCHLD)
daemon: use sigaction() to install child_handler()

 compat/mingw-posix.h |  1 +
 compat/mingw.c       |  4 +++-
 daemon.c             | 12 +++++++-----
 3 files changed, 11 insertions(+), 6 deletions(-)

base-commit: cb3b40381e1d5ee32dde96521ad7cfd68eb308a6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2002%2Fcarenas%2Fsiginterrupt-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2002/carenas/siginterrupt-v4
Pull-Request: https://github.com/git/git/pull/2002

Range-diff vs v3:

 1:  ae1ca6bb2b2 < -:  ----------- compat/posix.h: track SA_RESTART fallback
 2:  3f63479119f ! 1:  f21e8ff5c9d compat/mingw: allow sigaction(SIGCHLD)

@@ Commit message
A future change will start using sigaction to setup a SIGCHLD signal handler.

- The current code uses signal() which returns SIG_ERR (but doesn't
+ The current code uses signal(), which returns SIG_ERR (but doesn't
seem to set errno) so instruct sigaction() to do the same.

+ A new SA flag will be needed, so copy the one from Cygwinr; note that
+ the sigacgtion() implementation that is provided won't use it, so
+ its value is otherwise irrelevant.
+

## compat/mingw-posix.h ##
@@ compat/mingw-posix.h: struct sigaction {
- sig_handler_t sa_handler;
unsigned sa_flags;
};
+ #define SA_RESTART 0
+#define SA_NOCLDSTOP 1

struct itimerval {
3: c66bda461f4 ! 2: 81c41b43e60 daemon: use sigaction() to install child_handler()
@@ Metadata
## Commit message ##
daemon: use sigaction() to install child_handler()

- In a future change, the flags used for processing SIGCHLD will need to
- be updated, which is only possible by using sigaction().
+ Replace signal() with an equivalent invocation of sigaction(), but
+ make sure to NOT set SA_RESTART so the original code that expects
+ to be interrupted when children complete still works as designed.

- Replace signal() with an equivalent invocation of sigaction(), which
- has the added benefit of using BSD semantics reliably and therefore
- not needing the rearming call in the signal handler.
+ This change has the added benefit of using BSD signal semantics reliably
+ and therefore not needing the rearming call in the signal handler.

@@ daemon.c: static int service_loop(struct socketlist *socklist)

- signal(SIGCHLD, child_handler);
+ sigemptyset(&sa.sa_mask);
-+ sa.sa_flags = SA_NOCLDSTOP | SA_RESTART;
++ sa.sa_flags = SA_NOCLDSTOP;
+ sa.sa_handler = child_handler;
+ sigaction(SIGCHLD, &sa, NULL);

4: 851d663be0b < -: ----------- daemon: explicitly allow EINTR during poll()

Signature · 1 line
gitgitgadget