Re: [PATCH v2] daemon: correctly handle soft accept() errors in service_loop

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Carlo

On 26/06/2025 18:21, Carlo Marcelo Arenas Belón wrote:
Since df076bdbcc ([PATCH] GIT: Listen on IPv6 as well, if available.,
2005-07-23), the original error checking was included in an inner loop
unchanged, where its effect was different.

Instead of retrying, after a EINTR during accept() in the listening
socket, it will advance to the next one and try with that instead,
leaving the client waiting for another round.

Make sure that the loop doesn't advance

That makes sense

and while at it, make sure
that any possible completed children get reaped earlier.

What's the rationale for that? It means we end up calling reap_dead_children() twice which sounds inefficient. If it is important then I'm also struggling to see how it fits in with the proposal to use SA_RESTART

To avoid an
unlikely busy loop, fallback to the old behaviour after a couple
of attempts.

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@xxxxxxxxx>
---
  daemon.c | 13 +++++++++++--
  1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/daemon.c b/daemon.c
index d1be61fd57..f113839781 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1145,6 +1145,7 @@ static int service_loop(struct socketlist *socklist)
for (size_t i = 0; i < socklist->nr; i++) {
  			if (pfd[i].revents & POLLIN) {
+				int incoming;
  				union {
  					struct sockaddr sa;
  					struct sockaddr_in sai;
@@ -1153,11 +1154,19 @@ static int service_loop(struct socketlist *socklist)
  #endif
  				} ss;
  				socklen_t sslen = sizeof(ss);
-				int incoming = accept(pfd[i].fd, &ss.sa, &sslen);

Why is the declaration of incoming moved but retry is declared here?

Thanks

Phillip

+				int retry = 3;
+
+			redo:
+				incoming = accept(pfd[i].fd, &ss.sa, &sslen);
  				if (incoming < 0) {
  					switch (errno) {
-					case EAGAIN:
  					case EINTR:
+						if (--retry) {
+							check_dead_children();
+							goto redo;
+						}
+						/* fallthrough */
+					case EAGAIN:
  					case ECONNABORTED:
  						continue;
  					default:






[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux