git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 2/3] daemon: use sigaction() to install child_handler()

From
Junio C Hamano <gitster@pobox.com>
Date
Jun 25, 2025, 16:06 UTC
Message-ID
<xmqqfrfnyff1.fsf@gitster.g>
In-Reply-To
<05d945aa1e546bcc028e215c8e4d174b5e0c32ad.1750836928.git.gitgitgadget@gmail.com>

"Carlo Marcelo Arenas Belón via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 49 quoted lines
> From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= <carenas@gmail.com>
>
> In a future change, the flags used for processing SIGCHLD will need to
> be updated, which is only possible by using sigaction().
>
> Factor out the call to set the signal handler and use sigaction instead
> of signal for the systems that allow that, which has the added benefit
> of using BSD semantics reliably and therefore not needing the rearming
> call.
>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>  daemon.c | 35 +++++++++++++++++++++++++++++------
>  1 file changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/daemon.c b/daemon.c
> index d1be61fd5789..8133bd902157 100644
> --- a/daemon.c
> +++ b/daemon.c
> @@ -912,14 +912,19 @@ static void handle(int incoming, struct sockaddr *addr, socklen_t addrlen)
>  		add_child(&cld, addr, addrlen);
>  }
>  
> -static void child_handler(int signo UNUSED)
> +static void child_handler(int signo MAYBE_UNUSED)
>  {
>  	/*
> -	 * Otherwise empty handler because systemcalls will get interrupted
> -	 * upon signal receipt
> -	 * SysV needs the handler to be rearmed
> +	 * Otherwise empty handler because systemcalls should get interrupted
> +	 * upon signal receipt.
>  	 */
> -	signal(SIGCHLD, child_handler);
> +#ifdef USE_NON_POSIX_SIGNAL
> +	/*
> +	 * SysV needs the handler to be rearmed, but this is known
> +	 * to trigger infinite recursion crashes at least in AIX.
> +	 */
> +	signal(signo, child_handler);
> +#endif
>  }
>  
>  static int set_reuse_addr(int sockfd)
> @@ -1118,8 +1123,26 @@ static void socksetup(struct string_list *listen_addr, int listen_port, struct s
>  	}
>  }
>  
> +#ifndef USE_NON_POSIX_SIGNAL

Does this #ifndef block with #else lack its #endif probably before the service_loop() is defined ...

Show 14 quoted lines
> +static void set_signal_handler(struct sigaction *psa)
> +{
> +	sigemptyset(&psa->sa_mask);
> +	psa->sa_flags = SA_NOCLDSTOP | SA_RESTART;
> +	psa->sa_handler = child_handler;
> +	sigaction(SIGCHLD, psa, NULL);
> +}
> +
> +#else
> +
> +static void set_signal_handler(struct sigaction *psa UNUSED)
> +{
> +	signal(SIGCHLD, child_handler);
> +}
... around here?

I am wondering if it would make it even cleaner to also define rearm_signal_handler() in this "if sigaction() can be used, do this, otherwise do that" block, and move the whole thing a bit earlier in this file. That way, the primary code paths do not have to see much of the #ifdef conditionals. With IPV6 related #ifdef noise already contaminating the file, it may not be a huge deal, but these crufts tend to build up unless we tightly control them with discipline.

Show 15 quoted lines
>  static int service_loop(struct socketlist *socklist)
>  {
> +	struct sigaction sa;
>  	struct pollfd *pfd;
>  
>  	CALLOC_ARRAY(pfd, socklist->nr);
> @@ -1129,7 +1152,7 @@ static int service_loop(struct socketlist *socklist)
>  		pfd[i].events = POLLIN;
>  	}
>  
> -	signal(SIGCHLD, child_handler);
> +	set_signal_handler(&sa);
>  
>  	for (;;) {
>  		check_dead_children();
Previous: Carlo Marcelo Arenas Belón via GitGitGadgetNext: Junio C Hamano
Message 23 of 70 in “daemon: explicitly allow EINTR during poll()”
  1. 0/3 daemon: explicitly allow EINTR during poll()Carlo Marcelo Arenas Belón via GitGitGadget, Jun 24, 2025
  2. 1/3 compat/posix.h: track SA_RESTART fallbackCarlo Marcelo Arenas Belón via GitGitGadget, Jun 24, 2025
  3. Junio C HamanoJun 24, 2025
  4. Junio C HamanoJun 24, 2025
  5. 2/3 daemon: use sigaction() to install child_handler()Carlo Marcelo Arenas Belón via GitGitGadget, Jun 24, 2025
  6. Junio C HamanoJun 24, 2025
  7. Carlo Marcelo Arenas BelónJun 24, 2025
  8. Chris TorekJun 24, 2025
  9. Junio C HamanoJun 24, 2025
  10. Carlo Marcelo Arenas BelónJun 24, 2025
  11. Junio C HamanoJun 24, 2025
  12. 3/3 daemon: explicitly allow EINTR during poll()Carlo Marcelo Arenas Belón via GitGitGadget, Jun 24, 2025
  13. Carlo Marcelo Arenas BelónJun 24, 2025
  14. Junio C HamanoJun 24, 2025
  15. 0/3 daemon: explicitly allow EINTR during poll()Carlo Marcelo Arenas Belón via GitGitGadget, Jun 25, 2025
  16. 1/3 compat/posix.h: track SA_RESTART fallbackCarlo Marcelo Arenas Belón via GitGitGadget, Jun 25, 2025
  17. Junio C HamanoJun 25, 2025
  18. Carlo Marcelo Arenas BelónJun 25, 2025
  19. Junio C HamanoJun 26, 2025
  20. Carlo Marcelo Arenas BelónJun 26, 2025
  21. Junio C HamanoJun 26, 2025
  22. 2/3 daemon: use sigaction() to install child_handler()Carlo Marcelo Arenas Belón via GitGitGadget, Jun 25, 2025
  23. Junio C HamanoJun 25, 2025
  24. Junio C HamanoJun 25, 2025
  25. 3/3 daemon: explicitly allow EINTR during poll()Carlo Marcelo Arenas Belón via GitGitGadget, Jun 25, 2025
  26. Junio C HamanoJun 25, 2025
  27. Phillip WoodJun 25, 2025
  28. Junio C HamanoJun 25, 2025
  29. Phillip WoodJun 25, 2025
  30. daemon: add a self pipe to trigger reaping of childrenCarlo Marcelo Arenas Belón, Jun 26, 2025
  31. 0/2 daemon: tracking childs without signalsCarlo Marcelo Arenas Belón, Jun 26, 2025
  32. 1/2 run-command: add a pipe() write end to childsCarlo Marcelo Arenas Belón, Jun 26, 2025
  33. 2/2 daemon: poor man's pidfd like POCCarlo Marcelo Arenas Belón, Jun 26, 2025
  34. Phillip WoodJun 27, 2025
  35. Carlo Marcelo Arenas BelónJun 28, 2025
  36. Phillip WoodJul 1, 2025
  37. Junio C HamanoJun 30, 2025
  38. Junio C HamanoJun 25, 2025
  39. Carlo Marcelo Arenas BelónJun 26, 2025
  40. 0/4 daemon: explicitly allow EINTR during poll()Carlo Marcelo Arenas Belón via GitGitGadget, Jun 26, 2025
  41. 1/4 compat/posix.h: track SA_RESTART fallbackCarlo Marcelo Arenas Belón via GitGitGadget, Jun 26, 2025
  42. Junio C HamanoJun 27, 2025
  43. 2/4 compat/mingw: allow sigaction(SIGCHLD)Carlo Marcelo Arenas Belón via GitGitGadget, Jun 26, 2025
  44. Phillip WoodJun 26, 2025
  45. Carlo Marcelo Arenas BelónJun 26, 2025
  46. Phillip WoodJun 26, 2025
  47. Carlo Marcelo Arenas BelónJun 26, 2025
  48. phillip.wood123@gmail.comJun 26, 2025
  49. Carlo Marcelo Arenas BelónJun 26, 2025
  50. Phillip WoodJul 9, 2025
  51. Carlo Marcelo Arenas BelónJul 9, 2025
  52. 3/4 daemon: use sigaction() to install child_handler()Carlo Marcelo Arenas Belón via GitGitGadget, Jun 26, 2025
  53. Phillip WoodJun 26, 2025
  54. Junio C HamanoJun 26, 2025
  55. Carlo Marcelo Arenas BelónJun 26, 2025
  56. Phillip WoodJun 26, 2025
  57. Junio C HamanoJul 7, 2025
  58. 4/4 daemon: explicitly allow EINTR during poll()Carlo Marcelo Arenas Belón via GitGitGadget, Jun 26, 2025
  59. Phillip WoodJun 26, 2025
  60. Phillip WoodJul 9, 2025
  61. Carlo Marcelo Arenas BelónJul 9, 2025
  62. 0/2 daemon: explicitly allow EINTR during poll()Carlo Marcelo Arenas Belón via GitGitGadget, Jul 10, 2025
  63. 1/2 compat/mingw: allow sigaction(SIGCHLD)Carlo Marcelo Arenas Belón via GitGitGadget, Jul 10, 2025
  64. Eric SunshineJul 10, 2025
  65. 2/2 daemon: use sigaction() to install child_handler()Carlo Marcelo Arenas Belón via GitGitGadget, Jul 10, 2025
  66. Junio C HamanoJul 10, 2025
  67. Carlo ArenasJul 10, 2025
  68. Phillip WoodJul 11, 2025
  69. Junio C HamanoJul 14, 2025
  70. Phillip WoodJul 15, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.