daemon: explicitly allow EINTR during poll()
Get series with b4
b4 am [email protected]Versions newest first · linked by author and title
- v4 Latest2 patches 6 repliesIn review
- v34 patches 17 repliesSuperseded
- v23 patches 13 repliesSuperseded
Earlier versions · 1
| Version | Posted | Patches | Replies | Status |
|---|---|---|---|---|
| v1 | 3 patches | 10 replies | Superseded |
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 files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/2 | compat/mingw: allow sigaction(SIGCHLD)compat/ compat/ | +4 | 1 reply | |
| 2/2 | daemon: use sigaction() to install child_handler()daemon.c | +7 | 0 replies |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v4 yet.
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