{"thread":{"id":"63700","subject":"[PATCH] daemon: correctly handle soft accept() errors","startedAt":"2025-06-26T16:11:16Z","lastAt":"2025-07-01T19:33:44Z","messageCount":14,"participants":["Carlo Marcelo Arenas Belón","Kristoffer Haugsbakk","Phillip Wood","Junio C Hamano","Hridoy Ahmed"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"520769","messageId":"20250626161038.85966-1-carenas@gmail.com","threadId":"63700","inReplyTo":null,"subject":"[PATCH] daemon: correctly handle soft accept() errors","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2025-06-26T16:10:38Z","receivedAt":"2025-06-26T16:11:16Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"Since df076bdbcc ([PATCH] GIT: Listen on IPv6 as well, if available.,\n2005-07-23), the original error checking was included in an inner loop\nunchanged, where its effect was different.\n\nInstead of retrying, after a EINTR during accept() in the listening\nsocket, it will advance to the next one and try with that instead,\nleaving the client waiting for another round.\n\nMake sure that the loop doesn't advance and while at it, make sure\nthat any possible completed childs get reaped earlier. To avoid an\nunlilely busy loop, fallback to the old behaviour after a couple\nof attempts.\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\n daemon.c | 14 ++++++++++++--\n 1 file changed, 12 insertions(+), 2 deletions(-)\n\ndiff --git a/daemon.c b/daemon.c\nindex d1be61fd57..8d02099a4e 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -1153,11 +1153,21 @@ static int service_loop(struct socketlist *socklist)\n #endif\n \t\t\t\t} ss;\n \t\t\t\tsocklen_t sslen = sizeof(ss);\n-\t\t\t\tint incoming = accept(pfd[i].fd, &ss.sa, &sslen);\n+\t\t\t\tint incoming;\n+\n+redo:\n+\t\t\t\tincoming = accept(pfd[i].fd, &ss.sa, &sslen);\n \t\t\t\tif (incoming < 0) {\n+\t\t\t\t\tint retry = 2;\n+\n \t\t\t\t\tswitch (errno) {\n-\t\t\t\t\tcase EAGAIN:\n \t\t\t\t\tcase EINTR:\n+\t\t\t\t\t\tif (--retry) {\n+\t\t\t\t\t\t\tcheck_dead_children();\n+\t\t\t\t\t\t\tgoto redo;\n+\t\t\t\t\t\t}\n+\t\t\t\t\t\t/* fallthrough */\n+\t\t\t\t\tcase EAGAIN:\n \t\t\t\t\tcase ECONNABORTED:\n \t\t\t\t\t\tcontinue;\n \t\t\t\t\tdefault:\n-- \n2.50.0.132.gb7f585ac00\n\n"},{"id":"520770","messageId":"4e92bb11-1993-4cb2-be16-a972088f7cf3@app.fastmail.com","threadId":"63700","inReplyTo":"20250626161038.85966-1-carenas@gmail.com","subject":"Re: [PATCH] daemon: correctly handle soft accept() errors","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2025-06-26T16:15:13Z","receivedAt":"2025-06-26T16:15:35Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Thu, Jun 26, 2025, at 18:10, Carlo Marcelo Arenas Belón wrote:\n> Since df076bdbcc ([PATCH] GIT: Listen on IPv6 as well, if available.,\n> 2005-07-23), the original error checking was included in an inner loop\n> unchanged, where its effect was different.\n>\n> Instead of retrying, after a EINTR during accept() in the listening\n> socket, it will advance to the next one and try with that instead,\n> leaving the client waiting for another round.\n>\n> Make sure that the loop doesn't advance and while at it, make sure\n> that any possible completed childs get reaped earlier. To avoid an\n\ns/childs/children/\n\n> unlilely busy loop, fallback to the old behaviour after a couple\n\nunlilely?\n\n> of attempts.\n>\n> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n"},{"id":"520774","messageId":"20250626172159.87204-1-carenas@gmail.com","threadId":"63700","inReplyTo":"20250626161038.85966-1-carenas@gmail.com","subject":"[PATCH v2] daemon: correctly handle soft accept() errors in service_loop","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2025-06-26T17:21:59Z","receivedAt":"2025-06-26T17:22:32Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"Since df076bdbcc ([PATCH] GIT: Listen on IPv6 as well, if available.,\n2005-07-23), the original error checking was included in an inner loop\nunchanged, where its effect was different.\n\nInstead of retrying, after a EINTR during accept() in the listening\nsocket, it will advance to the next one and try with that instead,\nleaving the client waiting for another round.\n\nMake sure that the loop doesn't advance and while at it, make sure\nthat any possible completed children get reaped earlier. To avoid an\nunlikely busy loop, fallback to the old behaviour after a couple\nof attempts.\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\n daemon.c | 13 +++++++++++--\n 1 file changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/daemon.c b/daemon.c\nindex d1be61fd57..f113839781 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -1145,6 +1145,7 @@ static int service_loop(struct socketlist *socklist)\n \n \t\tfor (size_t i = 0; i < socklist->nr; i++) {\n \t\t\tif (pfd[i].revents & POLLIN) {\n+\t\t\t\tint incoming;\n \t\t\t\tunion {\n \t\t\t\t\tstruct sockaddr sa;\n \t\t\t\t\tstruct sockaddr_in sai;\n@@ -1153,11 +1154,19 @@ static int service_loop(struct socketlist *socklist)\n #endif\n \t\t\t\t} ss;\n \t\t\t\tsocklen_t sslen = sizeof(ss);\n-\t\t\t\tint incoming = accept(pfd[i].fd, &ss.sa, &sslen);\n+\t\t\t\tint retry = 3;\n+\n+\t\t\tredo:\n+\t\t\t\tincoming = accept(pfd[i].fd, &ss.sa, &sslen);\n \t\t\t\tif (incoming < 0) {\n \t\t\t\t\tswitch (errno) {\n-\t\t\t\t\tcase EAGAIN:\n \t\t\t\t\tcase EINTR:\n+\t\t\t\t\t\tif (--retry) {\n+\t\t\t\t\t\t\tcheck_dead_children();\n+\t\t\t\t\t\t\tgoto redo;\n+\t\t\t\t\t\t}\n+\t\t\t\t\t\t/* fallthrough */\n+\t\t\t\t\tcase EAGAIN:\n \t\t\t\t\tcase ECONNABORTED:\n \t\t\t\t\t\tcontinue;\n \t\t\t\t\tdefault:\n-- \n2.50.0.132.g195eec4876\n\n"},{"id":"520802","messageId":"08804dbe-56dd-4c0e-b36b-a82768b0aa29@gmail.com","threadId":"63700","inReplyTo":"20250626172159.87204-1-carenas@gmail.com","subject":"Re: [PATCH v2] daemon: correctly handle soft accept() errors in service_loop","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-06-27T08:38:47Z","receivedAt":"2025-06-27T08:38:50Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Carlo\n\nOn 26/06/2025 18:21, Carlo Marcelo Arenas Belón wrote:\n> Since df076bdbcc ([PATCH] GIT: Listen on IPv6 as well, if available.,\n> 2005-07-23), the original error checking was included in an inner loop\n> unchanged, where its effect was different.\n> \n> Instead of retrying, after a EINTR during accept() in the listening\n> socket, it will advance to the next one and try with that instead,\n> leaving the client waiting for another round.\n> \n> Make sure that the loop doesn't advance\n\nThat makes sense\n\n> and while at it, make sure\n> that any possible completed children get reaped earlier.\n\nWhat's the rationale for that? It means we end up calling \nreap_dead_children() twice which sounds inefficient. If it is important \nthen I'm also struggling to see how it fits in with the proposal to use \nSA_RESTART\n\n> To avoid an\n> unlikely busy loop, fallback to the old behaviour after a couple\n> of attempts.\n> \n> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n> ---\n>   daemon.c | 13 +++++++++++--\n>   1 file changed, 11 insertions(+), 2 deletions(-)\n> \n> diff --git a/daemon.c b/daemon.c\n> index d1be61fd57..f113839781 100644\n> --- a/daemon.c\n> +++ b/daemon.c\n> @@ -1145,6 +1145,7 @@ static int service_loop(struct socketlist *socklist)\n>   \n>   \t\tfor (size_t i = 0; i < socklist->nr; i++) {\n>   \t\t\tif (pfd[i].revents & POLLIN) {\n> +\t\t\t\tint incoming;\n>   \t\t\t\tunion {\n>   \t\t\t\t\tstruct sockaddr sa;\n>   \t\t\t\t\tstruct sockaddr_in sai;\n> @@ -1153,11 +1154,19 @@ static int service_loop(struct socketlist *socklist)\n>   #endif\n>   \t\t\t\t} ss;\n>   \t\t\t\tsocklen_t sslen = sizeof(ss);\n> -\t\t\t\tint incoming = accept(pfd[i].fd, &ss.sa, &sslen);\n\nWhy is the declaration of incoming moved but retry is declared here?\n\nThanks\n\nPhillip\n\n> +\t\t\t\tint retry = 3;\n> +\n> +\t\t\tredo:\n> +\t\t\t\tincoming = accept(pfd[i].fd, &ss.sa, &sslen);\n>   \t\t\t\tif (incoming < 0) {\n>   \t\t\t\t\tswitch (errno) {\n> -\t\t\t\t\tcase EAGAIN:\n>   \t\t\t\t\tcase EINTR:\n> +\t\t\t\t\t\tif (--retry) {\n> +\t\t\t\t\t\t\tcheck_dead_children();\n> +\t\t\t\t\t\t\tgoto redo;\n> +\t\t\t\t\t\t}\n> +\t\t\t\t\t\t/* fallthrough */\n> +\t\t\t\t\tcase EAGAIN:\n>   \t\t\t\t\tcase ECONNABORTED:\n>   \t\t\t\t\t\tcontinue;\n>   \t\t\t\t\tdefault:\n\n\n"},{"id":"520821","messageId":"u4scxaxivz62fsljo7okkfdvcle3zdby6h2sdzd6ed5n6wi5xb@5ekxdycixwxe","threadId":"63700","inReplyTo":"08804dbe-56dd-4c0e-b36b-a82768b0aa29@gmail.com","subject":"Re: [PATCH v2] daemon: correctly handle soft accept() errors in service_loop","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2025-06-27T19:05:15Z","receivedAt":"2025-06-27T19:05:18Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Fri, Jun 27, 2025 at 09:38:47AM -0800, Phillip Wood wrote:\n> \n> On 26/06/2025 18:21, Carlo Marcelo Arenas Belón wrote:\n> > \n> > diff --git a/daemon.c b/daemon.c\n> > index d1be61fd57..f113839781 100644\n> > --- a/daemon.c\n> > +++ b/daemon.c\n> > @@ -1145,6 +1145,7 @@ static int service_loop(struct socketlist *socklist)\n> >   \t\tfor (size_t i = 0; i < socklist->nr; i++) {\n> >   \t\t\tif (pfd[i].revents & POLLIN) {\n> > +\t\t\t\tint incoming;\n> >   \t\t\t\tunion {\n> >   \t\t\t\t\tstruct sockaddr sa;\n> >   \t\t\t\t\tstruct sockaddr_in sai;\n> > @@ -1153,11 +1154,19 @@ static int service_loop(struct socketlist *socklist)\n> >   #endif\n> >   \t\t\t\t} ss;\n> >   \t\t\t\tsocklen_t sslen = sizeof(ss);\n> > -\t\t\t\tint incoming = accept(pfd[i].fd, &ss.sa, &sslen);\n> \n> Why is the declaration of incoming moved but retry is declared here?\n\nSeparating the declaration and assignment for incoming is needed so we can\ninsert a label for goto; moving it up just removes distractions so the rest\nof the logic is clearly in view.\n\nObviously that includes the definition and assignment for retry.\n\nHow would you suggest to arrange this better?\n\nCarlo\n"},{"id":"520826","messageId":"xmqq34bl7xa1.fsf@gitster.g","threadId":"63700","inReplyTo":"u4scxaxivz62fsljo7okkfdvcle3zdby6h2sdzd6ed5n6wi5xb@5ekxdycixwxe","subject":"Re: [PATCH v2] daemon: correctly handle soft accept() errors in service_loop","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-27T20:19:18Z","receivedAt":"2025-06-27T20:19:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:\n\n> On Fri, Jun 27, 2025 at 09:38:47AM -0800, Phillip Wood wrote:\n>> \n>> On 26/06/2025 18:21, Carlo Marcelo Arenas Belón wrote:\n>> > \n>> > diff --git a/daemon.c b/daemon.c\n>> > index d1be61fd57..f113839781 100644\n>> > --- a/daemon.c\n>> > +++ b/daemon.c\n>> > @@ -1145,6 +1145,7 @@ static int service_loop(struct socketlist *socklist)\n>> >   \t\tfor (size_t i = 0; i < socklist->nr; i++) {\n>> >   \t\t\tif (pfd[i].revents & POLLIN) {\n>> > +\t\t\t\tint incoming;\n>> >   \t\t\t\tunion {\n>> >   \t\t\t\t\tstruct sockaddr sa;\n>> >   \t\t\t\t\tstruct sockaddr_in sai;\n>> > @@ -1153,11 +1154,19 @@ static int service_loop(struct socketlist *socklist)\n>> >   #endif\n>> >   \t\t\t\t} ss;\n>> >   \t\t\t\tsocklen_t sslen = sizeof(ss);\n>> > -\t\t\t\tint incoming = accept(pfd[i].fd, &ss.sa, &sslen);\n>> \n>> Why is the declaration of incoming moved but retry is declared here?\n>\n> Separating the declaration and assignment for incoming is needed so we can\n> insert a label for goto; moving it up just removes distractions so the rest\n> of the logic is clearly in view.\n>\n> Obviously that includes the definition and assignment for retry.\n>\n> How would you suggest to arrange this better?\n\nI think what Phillip meant was more like this, perhaps.\n\n\t\tsocklen_t sslen = sizeof(ss);\n-\t\tint incoming = accept(pfd[i].fd, &ss.sa, &sslen);\n+\t\tint incoming;\n+\t\tint retry = 3;\n+\n+\t\tincoming = accept(pfd[i].fd, &ss.sa, &sslen);\n\t\tif (incoming < 0) {\n\t\t\t...\n"},{"id":"520829","messageId":"vgailqqh3bcip3gxtdffoo4ey7xjso4xerewxncy22shrzn4k2@25hst4sfgxq4","threadId":"63700","inReplyTo":"xmqq34bl7xa1.fsf@gitster.g","subject":"Re: [PATCH v2] daemon: correctly handle soft accept() errors in service_loop","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2025-06-27T23:05:52Z","receivedAt":"2025-06-27T23:05:54Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Fri, Jun 27, 2025 at 01:19:18PM -0800, Junio C Hamano wrote:\n> Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:\n> \n> > On Fri, Jun 27, 2025 at 09:38:47AM -0800, Phillip Wood wrote:\n> >> \n> >> On 26/06/2025 18:21, Carlo Marcelo Arenas Belón wrote:\n> >> > \n> >> > diff --git a/daemon.c b/daemon.c\n> >> > index d1be61fd57..f113839781 100644\n> >> > --- a/daemon.c\n> >> > +++ b/daemon.c\n> >> > @@ -1145,6 +1145,7 @@ static int service_loop(struct socketlist *socklist)\n> >> >   \t\tfor (size_t i = 0; i < socklist->nr; i++) {\n> >> >   \t\t\tif (pfd[i].revents & POLLIN) {\n> >> > +\t\t\t\tint incoming;\n> >> >   \t\t\t\tunion {\n> >> >   \t\t\t\t\tstruct sockaddr sa;\n> >> >   \t\t\t\t\tstruct sockaddr_in sai;\n> >> > @@ -1153,11 +1154,19 @@ static int service_loop(struct socketlist *socklist)\n> >> >   #endif\n> >> >   \t\t\t\t} ss;\n> >> >   \t\t\t\tsocklen_t sslen = sizeof(ss);\n> >> > -\t\t\t\tint incoming = accept(pfd[i].fd, &ss.sa, &sslen);\n> >> \n> >> Why is the declaration of incoming moved but retry is declared here?\n> >\n> > Separating the declaration and assignment for incoming is needed so we can\n> > insert a label for goto; moving it up just removes distractions so the rest\n> > of the logic is clearly in view.\n> >\n> > Obviously that includes the definition and assignment for retry.\n> >\n> > How would you suggest to arrange this better?\n> \n> I think what Phillip meant was more like this, perhaps.\n> \n> \t\tsocklen_t sslen = sizeof(ss);\n> -\t\tint incoming = accept(pfd[i].fd, &ss.sa, &sslen);\n> +\t\tint incoming;\n> +\t\tint retry = 3;\n> +\n> +\t\tincoming = accept(pfd[i].fd, &ss.sa, &sslen);\n> \t\tif (incoming < 0) {\n> \t\t\t...\n\nThat seems unnecessarily restrictive just to minimize churn and leaves the\ndeflaration of incoming strangely sitting in between two assignments, which\nwhile it doesn't trigger -Wdeclaration-after-statement seems to go against\nits spirit.\n\nWill include in a v3 with all other suggestions, but frankly think that the\noriginal was overall cleaner.\n\nCarlo\n"},{"id":"520830","messageId":"20250627231404.27100-1-carenas@gmail.com","threadId":"63700","inReplyTo":"20250626172159.87204-1-carenas@gmail.com","subject":"[PATCH v3] daemon: correctly handle soft accept() errors in service_loop","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2025-06-27T23:14:04Z","receivedAt":"2025-06-27T23:14:27Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"Since df076bdbcc ([PATCH] GIT: Listen on IPv6 as well, if available.,\n2005-07-23), the original error checking was included in an inner loop\nunchanged, where its effect was different.\n\nInstead of retrying, after a EINTR during accept() in the listening\nsocket, it will advance to the next one and try with that instead,\nleaving the client waiting for another round.\n\nMake sure to retry with the same listener socket that failed originally.\n\nTo avoid an unlikely busy loop, fallback to the old behaviour after a\ncouple of attempts.\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\n daemon.c | 12 ++++++++++--\n 1 file changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/daemon.c b/daemon.c\nindex d1be61fd57..9ac9efa17c 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -1153,11 +1153,19 @@ static int service_loop(struct socketlist *socklist)\n #endif\n \t\t\t\t} ss;\n \t\t\t\tsocklen_t sslen = sizeof(ss);\n-\t\t\t\tint incoming = accept(pfd[i].fd, &ss.sa, &sslen);\n+\t\t\t\tint incoming;\n+\t\t\t\tint retry = 3;\n+\n+\t\t\tredo:\n+\t\t\t\tincoming = accept(pfd[i].fd, &ss.sa, &sslen);\n \t\t\t\tif (incoming < 0) {\n \t\t\t\t\tswitch (errno) {\n-\t\t\t\t\tcase EAGAIN:\n \t\t\t\t\tcase EINTR:\n+\t\t\t\t\t\tif (--retry)\n+\t\t\t\t\t\t\tgoto redo;\n+\n+\t\t\t\t\t\t/* fallthrough */\n+\t\t\t\t\tcase EAGAIN:\n \t\t\t\t\tcase ECONNABORTED:\n \t\t\t\t\t\tcontinue;\n \t\t\t\t\tdefault:\n-- \n2.50.0.132.g195eec4876.dirty\n\n"},{"id":"520831","messageId":"061ECE45-21BA-4216-8F3A-61D5A8314706@gmail.com","threadId":"63700","inReplyTo":"vgailqqh3bcip3gxtdffoo4ey7xjso4xerewxncy22shrzn4k2@25hst4sfgxq4","subject":"Re: [PATCH v2] daemon: correctly handle soft accept() errors in service_loop","fromName":"Hridoy Ahmed","fromEmail":"ariyanhridoy130@gmail.com","sentAt":"2025-06-27T23:25:26Z","receivedAt":"2025-06-27T23:25:30Z","isPatch":true,"sender":{"key":"ariyanhridoy130@gmail.com","avatar":null},"body":"\nHridoy Ahmed\n\n> On 28 Jun 2025, at 2:06 AM, Carlo Marcelo Arenas Belón <carenas@gmail.com> wrote:\n> \n> On Fri, Jun 27, 2025 at 01:19:18PM -0800, Junio C Hamano wrote:\n>> Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:\n>> \n>>> On Fri, Jun 27, 2025 at 09:38:47AM -0800, Phillip Wood wrote:\n>>>> \n>>>>> On 26/06/2025 18:21, Carlo Marcelo Arenas Belón wrote:\n>>>>>> \n>>>>>> diff --git a/daemon.c b/daemon.c\n>>>>>> index d1be61fd57..f113839781 100644\n>>>>>> --- a/daemon.c\n>>>>>> +++ b/daemon.c\n>>>>>> @@ -1145,6 +1145,7 @@ static int service_loop(struct socketlist *socklist)\n>>>>>>          for (size_t i = 0; i < socklist->nr; i++) {\n>>>>>>              if (pfd[i].revents & POLLIN) {\n>>>>>> +                int incoming;\n>>>>>>                  union {\n>>>>>>                      struct sockaddr sa;\n>>>>>>                      struct sockaddr_in sai;\n>>>>>> @@ -1153,11 +1154,19 @@ static int service_loop(struct socketlist *socklist)\n>>>>>>  #endif\n>>>>>>                  } ss;\n>>>>>>                  socklen_t sslen = sizeof(ss);\n>>>>>> -                int incoming = accept(pfd[i].fd, &ss.sa, &sslen);\n>>>>> \n>>>>> Why is the declaration of incoming moved but retry is declared here?\n>>> \n>>> Separating the declaration and assignment for incoming is needed so we can\n>>> insert a label for goto; moving it up just removes distractions so the rest\n>>> of the logic is clearly in view.\n>>> \n>>> Obviously that includes the definition and assignment for retry.\n>>> \n>>> How would you suggest to arrange this better?\n>> \n>> I think what Phillip meant was more like this, perhaps.\n>> \n>>        socklen_t sslen = sizeof(ss);\n>> -        int incoming = accept(pfd[i].fd, &ss.sa, &sslen);\n>> +        int incoming;\n>> +        int retry = 3;\n>> +\n>> +        incoming = accept(pfd[i].fd, &ss.sa, &sslen);\n>>        if (incoming < 0) {\n>>            ...\n> \n> That seems unnecessarily restrictive just to minimize churn and leaves the\n> deflaration of incoming strangely sitting in between two assignments, which\n> while it doesn't trigger -Wdeclaration-after-statement seems to go against\n> its spirit.\n> \n> Will include in a v3 with all other suggestions, but frankly think that the\n> original was overall cleaner.\n> \n> Carlo\n> \n"},{"id":"520833","messageId":"xmqqy0tc68s4.fsf@gitster.g","threadId":"63700","inReplyTo":"vgailqqh3bcip3gxtdffoo4ey7xjso4xerewxncy22shrzn4k2@25hst4sfgxq4","subject":"Re: [PATCH v2] daemon: correctly handle soft accept() errors in service_loop","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-27T23:53:47Z","receivedAt":"2025-06-27T23:53:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:\n\n>> \t\tsocklen_t sslen = sizeof(ss);\n>> -\t\tint incoming = accept(pfd[i].fd, &ss.sa, &sslen);\n>> +\t\tint incoming;\n>> +\t\tint retry = 3;\n>> +\n>> +\t\tincoming = accept(pfd[i].fd, &ss.sa, &sslen);\n>> \t\tif (incoming < 0) {\n>> \t\t\t...\n>\n> That seems unnecessarily restrictive just to minimize churn and leaves the\n> deflaration of incoming strangely sitting in between two assignments, which\n> while it doesn't trigger -Wdeclaration-after-statement seems to go against\n> its spirit.\n\nHmph, I am not Phillip, but my take on it is that incoming and retry\nare fairly closely related variables in this loop, and better\ngrouped together?\n\nI also find it a bit ugly to hardcode \"3\" here like this, but\nperhaps I am overthinking about it.\n"},{"id":"520908","messageId":"abf3aabb-8e46-4324-9e35-634cdaf110fb@gmail.com","threadId":"63700","inReplyTo":"xmqqy0tc68s4.fsf@gitster.g","subject":"Re: [PATCH v2] daemon: correctly handle soft accept() errors in service_loop","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-06-30T08:59:48Z","receivedAt":"2025-06-30T08:59:54Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 28/06/2025 00:53, Junio C Hamano wrote:\n> Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:\n> \n>> That seems unnecessarily restrictive just to minimize churn and leaves the\n>> deflaration of incoming strangely sitting in between two assignments, which\n>> while it doesn't trigger -Wdeclaration-after-statement seems to go against\n>> its spirit.\n> \n> Hmph, I am not Phillip, but my take on it is that incoming and retry\n> are fairly closely related variables in this loop, and better\n> grouped together?\n\nThat was my thinking too\n\nThanks\n\nPhillip\n"},{"id":"520910","messageId":"0d507273-8b8c-42d9-a14f-27a5da0dac27@gmail.com","threadId":"63700","inReplyTo":"20250627231404.27100-1-carenas@gmail.com","subject":"Re: [PATCH v3] daemon: correctly handle soft accept() errors in service_loop","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-06-30T09:00:17Z","receivedAt":"2025-06-30T09:00:23Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Carlo\n\nThis looks good\n\nThanks\n\nPhillip\n\nOn 28/06/2025 00:14, Carlo Marcelo Arenas Belón wrote:\n> Since df076bdbcc ([PATCH] GIT: Listen on IPv6 as well, if available.,\n> 2005-07-23), the original error checking was included in an inner loop\n> unchanged, where its effect was different.\n> \n> Instead of retrying, after a EINTR during accept() in the listening\n> socket, it will advance to the next one and try with that instead,\n> leaving the client waiting for another round.\n> \n> Make sure to retry with the same listener socket that failed originally.\n> \n> To avoid an unlikely busy loop, fallback to the old behaviour after a\n> couple of attempts.\n> \n> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n> ---\n>   daemon.c | 12 ++++++++++--\n>   1 file changed, 10 insertions(+), 2 deletions(-)\n> \n> diff --git a/daemon.c b/daemon.c\n> index d1be61fd57..9ac9efa17c 100644\n> --- a/daemon.c\n> +++ b/daemon.c\n> @@ -1153,11 +1153,19 @@ static int service_loop(struct socketlist *socklist)\n>   #endif\n>   \t\t\t\t} ss;\n>   \t\t\t\tsocklen_t sslen = sizeof(ss);\n> -\t\t\t\tint incoming = accept(pfd[i].fd, &ss.sa, &sslen);\n> +\t\t\t\tint incoming;\n> +\t\t\t\tint retry = 3;\n> +\n> +\t\t\tredo:\n> +\t\t\t\tincoming = accept(pfd[i].fd, &ss.sa, &sslen);\n>   \t\t\t\tif (incoming < 0) {\n>   \t\t\t\t\tswitch (errno) {\n> -\t\t\t\t\tcase EAGAIN:\n>   \t\t\t\t\tcase EINTR:\n> +\t\t\t\t\t\tif (--retry)\n> +\t\t\t\t\t\t\tgoto redo;\n> +\n> +\t\t\t\t\t\t/* fallthrough */\n> +\t\t\t\t\tcase EAGAIN:\n>   \t\t\t\t\tcase ECONNABORTED:\n>   \t\t\t\t\t\tcontinue;\n>   \t\t\t\t\tdefault:\n\n"},{"id":"520928","messageId":"xmqqv7od452s.fsf@gitster.g","threadId":"63700","inReplyTo":"0d507273-8b8c-42d9-a14f-27a5da0dac27@gmail.com","subject":"Re: [PATCH v3] daemon: correctly handle soft accept() errors in service_loop","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-30T15:33:31Z","receivedAt":"2025-06-30T15:33:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> Hi Carlo\n>\n> This looks good\n>\n> Thanks\n>\n> Phillip\n\nThanks, both of you.  Shall we mark the topic for 'next', then?\n\n\n> On 28/06/2025 00:14, Carlo Marcelo Arenas Belón wrote:\n>> Since df076bdbcc ([PATCH] GIT: Listen on IPv6 as well, if available.,\n>> 2005-07-23), the original error checking was included in an inner loop\n>> unchanged, where its effect was different.\n>> Instead of retrying, after a EINTR during accept() in the listening\n>> socket, it will advance to the next one and try with that instead,\n>> leaving the client waiting for another round.\n>> Make sure to retry with the same listener socket that failed\n>> originally.\n>> To avoid an unlikely busy loop, fallback to the old behaviour after\n>> a\n>> couple of attempts.\n>> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n>> ---\n>>   daemon.c | 12 ++++++++++--\n>>   1 file changed, 10 insertions(+), 2 deletions(-)\n>> diff --git a/daemon.c b/daemon.c\n>> index d1be61fd57..9ac9efa17c 100644\n>> --- a/daemon.c\n>> +++ b/daemon.c\n>> @@ -1153,11 +1153,19 @@ static int service_loop(struct socketlist *socklist)\n>>   #endif\n>>   \t\t\t\t} ss;\n>>   \t\t\t\tsocklen_t sslen = sizeof(ss);\n>> -\t\t\t\tint incoming = accept(pfd[i].fd, &ss.sa, &sslen);\n>> +\t\t\t\tint incoming;\n>> +\t\t\t\tint retry = 3;\n>> +\n>> +\t\t\tredo:\n>> +\t\t\t\tincoming = accept(pfd[i].fd, &ss.sa, &sslen);\n>>   \t\t\t\tif (incoming < 0) {\n>>   \t\t\t\t\tswitch (errno) {\n>> -\t\t\t\t\tcase EAGAIN:\n>>   \t\t\t\t\tcase EINTR:\n>> +\t\t\t\t\t\tif (--retry)\n>> +\t\t\t\t\t\t\tgoto redo;\n>> +\n>> +\t\t\t\t\t\t/* fallthrough */\n>> +\t\t\t\t\tcase EAGAIN:\n>>   \t\t\t\t\tcase ECONNABORTED:\n>>   \t\t\t\t\t\tcontinue;\n>>   \t\t\t\t\tdefault:\n"},{"id":"521098","messageId":"628e4050-912e-491d-b55b-a054623db32a@gmail.com","threadId":"63700","inReplyTo":"xmqqv7od452s.fsf@gitster.g","subject":"Re: [PATCH v3] daemon: correctly handle soft accept() errors in service_loop","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-07-01T19:33:41Z","receivedAt":"2025-07-01T19:33:44Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 30/06/2025 16:33, Junio C Hamano wrote:\n> \n> Thanks, both of you.  Shall we mark the topic for 'next', then?\n\nYes, I think so\n\nThanks\n\nPhillip\n"}]}