Re: [PATCH v3] mingw: workaround for hangs when sending STDIN
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Feb 27, 2020, 21:15 UTC
- Message-ID
- <nycvar.QRO.7.76.6.2002272215210.46@tvgsbejvaqbjf.bet>
- In-Reply-To
- <xmqqy2szkfxr.fsf@gitster-ct.c.googlers.com>
Hi Junio & Alex,
On Tue, 18 Feb 2020, Junio C Hamano wrote:
Show 13 quoted lines
> "Alexandr Miloslavskiy via GitGitGadget" <gitgitgadget@gmail.com> > writes: > > > From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com> > > > > Explanation > > ----------- > > The problem here is flawed `poll()` implementation. When it tries to > > see if pipe can be written without blocking, it eventually calls > > `NtQueryInformationFile()` and tests `WriteQuotaAvailable`. However, > > the meaning of quota was misunderstood. The value of quota is reduced > > when either some data was written to a pipe, *or* there is a pending > > read on the pipe. Therefore, if there is a pending read of size >= then
I usually try to refrain from grammar policing, but in this case, the typo "then" (instead of "than") threw me.
Other than that, I think the patch is fine. At least it works as advertised in my hands.
Thanks, Dscho
Show 32 quoted lines
> > the pipe's buffer size, poll() will think that pipe is not writable and > > will hang forever, usually that means deadlocking both pipe users. > > ... > > Chosen solution > > --------------- > > Make `poll()` always reply "writable" for write end of the pipe. > > Hopefully one day someone will find a way to implement it properly. > > > > Reproduction > > ------------ > > printf "%8388608s" X >large_file.txt > > git stash push --include-untracked -- large_file.txt > > > > I have decided not to include this as test to avoid slowing down the > > test suite. I don't expect the specific problem to come back, and > > chances are that `git stash push` will be reworked to avoid sending the > > entire patch via STDIN. > > > > Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com> > > --- > > Thanks for a detailed description. > > I notice that we saw no comments from Windows experts for these > three rounds. Can somebody give an Ack (or nack) on it at least? > > I picked obvious "experts" in the output from > > $ git shortlog --since=1.year --no-merges master compat/ming\* compat/win\* > > Thanks. >