{"thread":{"id":"60819","subject":"[PATCH] win32: special-case `ENOSPC` when writing to a pipe","startedAt":"2024-01-30T21:37:03Z","lastAt":"2024-01-30T21:37:03Z","messageCount":1,"participants":["Johannes Schindelin via GitGitGadget"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"487612","messageId":"pull.1648.git.1706650619950.gitgitgadget@gmail.com","threadId":"60819","inReplyTo":null,"subject":"[PATCH] win32: special-case `ENOSPC` when writing to a pipe","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-01-30T21:36:59Z","receivedAt":"2024-01-30T21:37:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nSince c6d3cce6f3c4 (pipe_command(): handle ENOSPC when writing to a\npipe, 2022-08-17), one `write()` call that results in an `errno` value\n`ENOSPC` (which typically indicates out of disk space, which makes\nlittle sense in the context of a pipe) is treated the same as `EAGAIN`.\n\nHowever, contrary to expectations, as diagnosed in\nhttps://github.com/python/cpython/issues/101881#issuecomment-1428667015,\nwhen writing to a non-blocking pipe on Windows, an `errno` value of\n`ENOSPC` means something else: the write _fails_. Completely. Because\nmore data was provided than the internal pipe buffer can handle.\nSomewhat surprising, considering that `write()` is allowed to write less\nthan the specified amount, e.g. by writing only as much as fits in that\nbuffer. But it doesn't, it writes no byte at all in that instance.\n\nLet's handle this by manually detecting when an `ENOSPC` indicates that\na pipe's buffer is smaller than what needs to be written, and re-try\nusing the pipe's buffer size as `size` parameter.\n\nIt would be plausible to try writing the entire buffer in a loop,\nfeeding pipe buffer-sized chunks, but experiments show that trying to\nwrite more than one buffer-sized chunk right after that will immediately\nfail because the buffer is unlikely to be drained as fast as `write()`\ncould write again. And the whole point of a non-blocking pipe is to be\nnon-blocking.\n\nWhich means that the logic that determines the pipe's buffer size\nunfortunately has to be run potentially many times when writing large\namounts of data to a non-blocking pipe, as there is no elegant way to\ncache that information between `write()` calls. It's the best we can do,\nthough, so it has to be good enough.\n\nThis fix is required to let t3701.60 (handle very large filtered diff)\npass with the MSYS2 runtime provided by the MSYS2 project: Without this\npatch, the failed write would result in an infinite loop. This patch is\nnot required with Git for Windows' variant of the MSYS2 runtime only\nbecause Git for Windows added an ugly work-around specifically to avoid\na hang in that test case.\n\nThe diff is slightly chatty because it extends an already-existing\nconditional that special-cases a _different_ `errno` value for pipes,\nand because this patch needs to account for the fact that\n`_get_osfhandle()` potentially overwrites `errno`.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n    win32: special-case ENOSPC when writing to a pipe\n    \n    This is part of my (slow, after-hours) effort to chip away at the\n    deviations between Git for Windows' variant of the MSYS2 runtime and\n    MSYS2's variant.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1648%2Fdscho%2Fwin32-pipe-enospc-fix-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1648/dscho/win32-pipe-enospc-fix-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1648\n\n compat/mingw.c | 19 +++++++++++++++----\n 1 file changed, 15 insertions(+), 4 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 238a84ddbaa..320fb99a90e 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -707,13 +707,24 @@ ssize_t mingw_write(int fd, const void *buf, size_t len)\n {\n \tssize_t result = write(fd, buf, len);\n \n-\tif (result < 0 && errno == EINVAL && buf) {\n+\tif (result < 0 && (errno == EINVAL || errno == ENOSPC) && buf) {\n+\t\tint orig = errno;\n+\n \t\t/* check if fd is a pipe */\n \t\tHANDLE h = (HANDLE) _get_osfhandle(fd);\n-\t\tif (GetFileType(h) == FILE_TYPE_PIPE)\n+\t\tif (GetFileType(h) != FILE_TYPE_PIPE)\n+\t\t\terrno = orig;\n+\t\telse if (orig == EINVAL)\n \t\t\terrno = EPIPE;\n-\t\telse\n-\t\t\terrno = EINVAL;\n+\t\telse {\n+\t\t\tDWORD buf_size;\n+\n+\t\t\tif (!GetNamedPipeInfo(h, NULL, NULL, &buf_size, NULL))\n+\t\t\t\tbuf_size = 4096;\n+\t\t\tif (len > buf_size)\n+\t\t\t\treturn write(fd, buf, buf_size);\n+\t\t\terrno = orig;\n+\t\t}\n \t}\n \n \treturn result;\n\nbase-commit: c5b454771e6b086f60c7f1f139025f174bcedac9\n-- \ngitgitgadget\n"}]}