Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>> The question was not about the millisecond typo, but about why time-out
>> at all.
>
> Because I do not want to change the semantics!
>
> ATM, in those cases where it works (as opposed to hanging!), git-daemon
> --verbose reports in the syslog when a client disconnected, possibly with
> an error. It does so with a timestamp so that you can see how long the
> connection lasted. That is what logs are useful for.
>
> Now, syslog has timestamps at second-resolution (at least here it does),
> and I wanted to imitate that.
Yes, we do not want to change the semantics, but that is not a reason for
an unused daemon to wake up every second, isn't it?
> The alternative would be to deprive all users of an (mostly) accurate
> timestamp of the disconnect time.
>
>> Another way would be to set up a pipe to ourself that is included in the
>> poll() and write a byte to the pipe from the signal handler.
>
> It still would need to break out of the poll(), in which case the effect
> would be _exactly_ the same,...
I do not think so.
In the solution I suggested, you would set up a pipe to yourself, and give
the read end of the pipe and the accepting socket to poll() with infinite
timeout. And when you do reap in the signal handler, you write a byte to
the write end of the pipe (and that would be the only codepath that would
write to that pipe). That would make the pipe you are polling redable,
and allow you to break out of poll(). When poll() returns thusly, you
notice you are in that condition and read out that byte to keep the pipe
clean, and do the check_dead_children() thing.
That way, you would wake up only when you actually have something useful
to do. Otherwise you will stay dormant, waiting for something to happen,
either by socket becoming ready to accept, or child dying and raising
SIGCHLD.
I agree that it is a bit too elaborate change that we do not want to have
in 'maint'.
Even then, for 'maint', we probably would at least want something like
this on top of your patch, so that when we know we have absolutely nothing
to do, we do not have to spin.
daemon.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/daemon.c b/daemon.c
index 6dc96aa..ce3a6f5 100644
--- a/daemon.c
+++ b/daemon.c
@@ -944,6 +944,7 @@ static int service_loop(int socknum, int *socklist)
for (;;) {
int i;
+ int timeout;
/*
* This 1-sec timeout could lead to idly looping but it is
@@ -952,7 +953,8 @@ static int service_loop(int socknum, int *socklist)
* to ourselves that we poll, and write to the fd from child_handler()
* to wake us up (and consume it when the poll() returns...
*/
- i = poll(pfd, socknum, 1000);
+ timeout = (children_spawned != children_deleted) ? 1000 : -1;
+ i = poll(pfd, socknum, timeout);
if (i < 0) {
if (errno != EINTR) {
error("poll failed, resuming: %s",
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
