git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] mingw: handle writes to non-blocking pipe

From
René Scharfe <l.s.r@web.de>
Date
Aug 10, 2022, 22:34 UTC
Message-ID
<77244ffe-41c1-65bd-8984-8ed6909ffe07@web.de>
In-Reply-To
<YvQO4ZYI8/fAk0Gj@coredump.intra.peff.net>
Am 10.08.2022 um 22:02 schrieb Jeff King:
Show 41 quoted lines
> On Wed, Aug 10, 2022 at 07:39:40AM +0200, René Scharfe wrote:
>
>> write() on Windows reports ENOSPC when writing to a non-blocking pipe
>> whose buffer is full and rejects writes bigger than the buffer outright.
>> Change the error code to EAGAIN and try a buffer-sized partial write to
>> comply with POSIX and the expections of our Git-internal callers.
>
> Bearing in mind that I have no qualifications for reviewing
> Windows-specific patches, this seems like the right thing to be doing
> from the behavior you saw.
>
> One question:
>
>> +	if (result < 0 && errno == ENOSPC) {
>> +		/* check if fd is a non-blocking pipe */
>> +		HANDLE h = (HANDLE) _get_osfhandle(fd);
>> +		DWORD s;
>> +		if (GetFileType(h) == FILE_TYPE_PIPE &&
>> +		    GetNamedPipeHandleState(h, &s, NULL, NULL, NULL, NULL, 0) &&
>> +		    (s & PIPE_NOWAIT)) {
>> +			DWORD obuflen;
>> +			if (!GetNamedPipeInfo(h, NULL, &obuflen, NULL, NULL))
>> +				obuflen = PIPE_BUFFER_SIZE;
>> +			if (len > obuflen)
>> +				return mingw_write(fd, buf, obuflen);
>> +			errno = EAGAIN;
>> +		}
>
> OK, so we call GetNamedPipeInfo() to find the size of the pipe buffer.
> It's unclear to me from Microsoft's docs if that is the _total_ size, or
> if it's the remaining available size. Hopefully the latter, since none
> of this works otherwise. ;)
>
> But two corner cases:
>
>   - If we fail to get the size, we guess that it's the maximum. Is this
>     dangerous? I'm not sure why the call would fail, but if for some
>     reason it did fail and we can't make forward progress, would we
>     enter an infinite recursion of mingw_write()? Would it be safer to
>     bail with EAGAIN in such a case (through granted, that probably just
>     puts us into an infinite loop in xwrite())?

AFAIU it's the total size, not the available space. I think I confused it with PIPE_BUF, which we should use instead.

Alternatively we could retry with ever smaller sizes, down to one byte, to avoid EAGAIN as much as possible. Sounds costly, though.

Show 8 quoted lines
>
>   - According to the docs:
>
>       If the buffer size is zero, the buffer is allocated as needed.
>
>     If we see this case, we'd then call mingw_write() with 0 bytes,
>     which I imagine also makes no forward progress (though maybe we
>     eventually return a successful 0-byte write?).
Ah, yes, forgot that case.
René
Previous: Jeff KingNext: Jeff King
Message 16 of 44 in “pipe_command(): mark stdin descriptor as non-blocking”
  1. pipe_command(): mark stdin descriptor as non-blockingJeff King, Aug 2, 2022
  2. Junio C HamanoAug 2, 2022
  3. Jeff KingAug 2, 2022
  4. Junio C HamanoAug 2, 2022
  5. pipe_command(): mark stdin descriptor as non-blockingJeff King, Aug 3, 2022
  6. René ScharfeAug 3, 2022
  7. Jeff KingAug 3, 2022
  8. René ScharfeAug 3, 2022
  9. Jeff KingAug 5, 2022
  10. René ScharfeAug 5, 2022
  11. René ScharfeAug 7, 2022
  12. Jeff KingAug 7, 2022
  13. mingw: handle writes to non-blocking pipeRené Scharfe, Aug 10, 2022
  14. Johannes SchindelinAug 10, 2022
  15. Jeff KingAug 10, 2022
  16. René ScharfeAug 10, 2022
  17. Jeff KingAug 11, 2022
  18. René ScharfeAug 11, 2022
  19. Jeff KingAug 11, 2022
  20. René ScharfeAug 14, 2022
  21. Jeff KingAug 17, 2022
  22. 0/6 fix pipe_command() deadlockJeff King, Aug 17, 2022
  23. 1/6 compat: add function to enable nonblocking pipesJeff King, Aug 17, 2022
  24. Junio C HamanoAug 17, 2022
  25. Jeff KingAug 18, 2022
  26. 2/6 nonblock: support WindowsJeff King, Aug 17, 2022
  27. 3/6 git-compat-util: make MAX_IO_SIZE define globally availableJeff King, Aug 17, 2022
  28. 4/6 pipe_command(): avoid xwrite() for writing to pipeJeff King, Aug 17, 2022
  29. 5/6 pipe_command(): handle ENOSPC when writing to a pipeJeff King, Aug 17, 2022
  30. Junio C HamanoAug 17, 2022
  31. Jeff KingAug 18, 2022
  32. 6/6 pipe_command(): mark stdin descriptor as non-blockingJeff King, Aug 17, 2022
  33. Jeff KingAug 17, 2022
  34. René ScharfeAug 19, 2022
  35. Jeff KingAug 20, 2022
  36. René ScharfeAug 10, 2022
  37. Jeff KingAug 10, 2022
  38. René ScharfeAug 10, 2022
  39. Jeff KingAug 11, 2022
  40. René ScharfeAug 7, 2022
  41. Johannes SchindelinAug 8, 2022
  42. Johannes SchindelinAug 8, 2022
  43. Jeff KingAug 9, 2022
  44. Johannes SchindelinAug 9, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.