Junio C Hamano, Jan 28, 2022 at 18:52:

If they have already exited but the fact hasn't reached us over the
network, the write() will succeed to deposit the packet in the send
buffer.  So I am not sure how much this would actually help, but it
should be safe to send an unsolicited keepalive as long as the other
side is expecting to hear from us.  When either report_status or
report_status_v2 capabilities is in effect, we will make a report()
or report_v2() call later, so we should be safe.

This is not perfect but I think this is the best we can do without
adding a new capability so that the client sends a reply to the
keepalive packet.

I suspect that any keepalive, unless it expects an active "yes, I am
still alive" response from the other side, is too weak to "ensure".

I guess "to notice a client that has disconnected (e.g. killed with
^C)" is more appropriate.

OK, I will change that.

quoted

+	if (use_sideband) {
+		static const char buf[] = "0005\2";
+		write_or_die(1, buf, sizeof(buf) - 1);
+	}

Observing how execute_commands() and helper functions report an
error to the callers higher in the call chain, and ask them to abort
the remainder of the operation, I am not sure if write_or_die() is
appropriate.

    Side note: inside copy_to_sideband(), which runs in async, it is
    a different matter (i.e. the main process on our side is not
    what gets killed by that _or_die() part of the call), but this
    one kills the main process.

The convention around this code path seems to be to fill explanation
of error in cmd->error_string and return to the caller.  In this
case, the error_strings may not reach the pusher via report() or
report_v2() as they may have disconnected, but calling the report()
functions is not the only thing the caller will want to do after
calling us, so giving it a chance to clean up may be a better
design, e.g.

	if (write_in_full(...) < 0) {
		for (cmd = commands; cmd; cmd = cmd->next)
	        	cmd->error_string = "pusher went away";
		return;
	}

Yes, the current code will not actually use the error string in any
useful way in this particular case, since report() or report_v2()
will have nobody listening to them.  But being consistent will help
maintaining the caller, as it can later be extended to use it
locally (e.g. log the request and its outcome, check which cmd has
succeeded and failed using the NULL-ness of cmd->error_string, etc.)

The main receive-pack process will be killed by SIGPIPE anyway but I can
fill the error_string fields and return for code consistency.

I'll send a v4, thanks for the review.