Re: [PATCH v2] pipe_command(): mark stdin descriptor as non-blocking

From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH v2] pipe_command(): mark stdin descriptor as non-blocking
Date: Mon, 8 Aug 2022 14:59:49 +0200 (CEST)	[thread overview]
Message-ID: <q341oso8-1ps6-65n6-s394-n8q433q79nr2@tzk.qr> (raw)
In-Reply-To: <YunxHOa2sJeEpJxd@coredump.intra.peff.net>

Hi Peff,

On Tue, 2 Aug 2022, Jeff King wrote:

> diff --git a/run-command.c b/run-command.c
> index 14f17830f5..ed99503b22 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -1438,6 +1439,15 @@ int pipe_command(struct child_process *cmd,
>  		return -1;
>
>  	if (in) {
> +		if (enable_nonblock(cmd->in) < 0) {
> +			error_errno("unable to make pipe non-blocking");

It might be a bit heavy-handed to error out in this case, as it usually
does not cause problems. At least that's what the fact suggests to me that
I personally never encountered the dead-lock myself, and neither do I
recall anybody piping more than two megabytes through `git checkout -p`.

Could we turn this into `warning_errno()` and avoid reporting an error
here? We could add "; This might hang the process if large amounts of data
are piped to/from <cmd>" to improve the user experience.

Ciao,
Dscho

> +			close(cmd->in);
> +			if (out)
> +				close(cmd->out);
> +			if (err)
> +				close(cmd->err);
> +			return -1;
> +		}
>  		io[nr].fd = cmd->in;
>  		io[nr].type = POLLOUT;
>  		io[nr].u.out.buf = in;