[PATCH v2 0/2] MR11272: server: Report a stream socket writable while its send buffer has room.
Previously floated as an RFC on wine-devel (https://list.winehq.org/hyperkitty/list/wine-devel@list.winehq.org/thread/HV...); positive response, no objections, opening as an MR now. Problem. Windows select() reports a connected stream socket writable whenever send() would still accept data. Wine follows the host poll(), and on Linux POLLOUT is only raised once the send queue drains below ~2/3 of SO_SNDBUF. An app-limited sender that does a select() writability check before arming FD_WRITE (libcurl's multi loop, and anything built on it) sees "not writable" while its sends keep succeeding, and waits out its full poll timeout (~1s) between bursts, throttling single-stream uploads to ~140 KB/s. Found with a Backblaze B2 client. Fix. In poll_socket(), report a connected stream socket writable when TIOCOUTQ < SO_SNDBUF. Guarded by #ifdef TIOCOUTQ so behaviour is unchanged where it is unavailable. Why this is correct for Wine. It reproduces what the application observes on real Windows rather than fixing a Windows-side defect: native Windows reports the socket writable and runs the same client at full speed, while the Linux POLLOUT threshold is a host artefact invisible to the Windows application. Validated against real Windows and against Wine on a Linux guest. Test. dlls/ws2_32/tests adds a conformance test for the writability invariant marked todo_wine; the fix commit removes the todo. It counts invariant violations and asserts zero rather than asserting timing or throughput, to stay robust across CI configurations. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=59893 -- v2: server: Report a stream socket writable whenever a send would be accepted. ws2_32/tests: Test send writability and FD_WRITE after select. https://gitlab.winehq.org/wine/wine/-/merge_requests/11272
From: Martyn Forryan <winehq-gitlab@forryan.co.uk> Windows Server 2022 confirms that observing a socket as not writable through select() does not re-arm FD_WRITE. The 2.67-million-assertion run leaves the event unsignaled and the wait times out. --- dlls/ws2_32/tests/sock.c | 268 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 268 insertions(+) diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 698ae0fe0b9..44a6977f66c 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -7151,6 +7151,272 @@ static void test_write_events(struct event_test_ctx *ctx) free(buffer); } +static int socket_select_writable(SOCKET socket) +{ + struct timeval timeout = {0}; + fd_set writefds; + int ret; + + FD_ZERO(&writefds); + FD_SET(socket, &writefds); + ret = select(0, NULL, &writefds, NULL, &timeout); + if (ret == SOCKET_ERROR) + return SOCKET_ERROR; + return ret && FD_ISSET(socket, &writefds); +} + +static void test_send_writability(void) +{ + static const int max_sends = 65536; + unsigned int select_error = 0, send_error = 0, recv_error = 0; + unsigned int writable_wouldblock = 0; + unsigned int total_sent = 0, total_received = 0; + SOCKET client, server; + char buffer[4096]; + int zero_rounds; + int recv_size; + int writable; + int value; + int ret; + int i; + + memset(buffer, 'a', sizeof(buffer)); + + tcp_socketpair(&client, &server); + set_blocking(server, FALSE); + + value = 65536; + ret = setsockopt(server, SOL_SOCKET, SO_SNDBUF, (char *)&value, sizeof(value)); + ok(!ret, "got %d, error %u\n", ret, WSAGetLastError()); + ret = setsockopt(client, SOL_SOCKET, SO_RCVBUF, (char *)&value, sizeof(value)); + ok(!ret, "got %d, error %u\n", ret, WSAGetLastError()); + + for (i = 0; i < max_sends; ++i) + { + writable = socket_select_writable(server); + if (writable == SOCKET_ERROR) + { + select_error = WSAGetLastError(); + break; + } + + ret = send(server, buffer, sizeof(buffer), 0); + if (writable && ret == SOCKET_ERROR && WSAGetLastError() == WSAEWOULDBLOCK) + ++writable_wouldblock; + + if (ret == SOCKET_ERROR) + { + send_error = WSAGetLastError(); + break; + } + total_sent += ret; + } + + ok(!select_error, "got select error %u\n", select_error); + ok(send_error == WSAEWOULDBLOCK, "got send error %u after %d sends\n", send_error, i); + ok(!writable_wouldblock, "got %u would-block errors while writable\n", + writable_wouldblock); + + /* The socket is hard-full and cannot accept a send. */ + /* The first WSAEWOULDBLOCK can be transient: the peer's kernel keeps + * absorbing in-flight data without the application reading. The state is + * only static once a settle-and-refill round accepts nothing. */ + zero_rounds = 0; + for (i = 0; i < 64 && zero_rounds < 2; ++i) + { + int added = 0; + + Sleep(50); + while ((ret = send(server, buffer, sizeof(buffer), 0)) > 0) + { + added += ret; + total_sent += ret; + } + zero_rounds = added ? 0 : zero_rounds + 1; + } + ok(zero_rounds == 2, "connection did not become quiescent after %d rounds\n", i); + writable = socket_select_writable(server); + ok(!writable, "got writable %d\n", writable); + ret = send(server, buffer, sizeof(buffer), 0); + send_error = ret == SOCKET_ERROR ? WSAGetLastError() : 0; + ok(ret == SOCKET_ERROR && send_error == WSAEWOULDBLOCK, + "got %d, error %u\n", ret, send_error); + if (ret > 0) + total_sent += ret; + + /* The queued amount at the wedge depends on kernel timing, so no fixed + * buffer size reliably lands between it and the poll low-water mark. + * Find the boundary behaviourally instead: grow the send buffer until the + * kernel accepts a byte, then step a little further in. The socket is then + * writable by Windows rules, while the headroom stays far below the + * low-water mark (half the queued amount), so Linux withholds POLLOUT. */ + for (value = 65536 + 8192; value <= 1048576; value += 8192) + { + int actual = 0; + socklen_t len = sizeof(actual); + + ret = setsockopt(server, SOL_SOCKET, SO_SNDBUF, (char *)&value, sizeof(value)); + ok(!ret, "got %d, error %u\n", ret, WSAGetLastError()); + ret = send(server, buffer, 1, 0); + if (ret == 1) + { + ++total_sent; + break; + } + /* The host may cap SO_SNDBUF (net.core.wmem_max on Linux). Once the + * buffer stops growing there is no headroom to find. */ + if (!getsockopt(server, SOL_SOCKET, SO_SNDBUF, (char *)&actual, &len) && actual < value) + break; + } + if (ret != 1) + { + skip("send buffer cannot grow past the queued data on this host\n"); + goto done; + } + value += 32768; + ret = setsockopt(server, SOL_SOCKET, SO_SNDBUF, (char *)&value, sizeof(value)); + ok(!ret, "got %d, error %u\n", ret, WSAGetLastError()); + writable = socket_select_writable(server); + todo_wine ok(writable == 1, "got writable %d\n", writable); + ret = send(server, buffer, sizeof(buffer), 0); + send_error = ret == SOCKET_ERROR ? WSAGetLastError() : 0; + ok(ret == sizeof(buffer), "got %d, error %u\n", ret, send_error); + if (ret > 0) + total_sent += ret; + + /* Drain every accepted byte and verify the empty socket is writable. */ + recv_error = 0; + while (total_received < total_sent) + { + recv_size = total_sent - total_received; + if (recv_size > sizeof(buffer)) recv_size = sizeof(buffer); + ret = recv(client, buffer, recv_size, 0); + if (ret <= 0) + { + recv_error = ret == SOCKET_ERROR ? WSAGetLastError() : 0; + break; + } + total_received += ret; + } + ok(total_received == total_sent, "received %u of %u bytes, error %u\n", + total_received, total_sent, recv_error); + + Sleep(100); + writable = socket_select_writable(server); + ok(writable == 1, "got writable %d\n", writable); + ret = send(server, buffer, sizeof(buffer), 0); + send_error = ret == SOCKET_ERROR ? WSAGetLastError() : 0; + ok(ret == sizeof(buffer), "got %d, error %u\n", ret, send_error); + +done: + closesocket(server); + closesocket(client); +} + +static void test_write_event_no_rearm_after_select(void) +{ + static const int buffer_size = 1024; + static const int max_sends = 65536; + WSANETWORKEVENTS events; + SOCKET client, server; + unsigned int error = 0; + int send_blocked = 0; + int select_blocked = 0; + int short_send = 0; + char *buffer; + HANDLE event; + DWORD wait; + int value; + int ret; + int i; + + buffer = malloc(buffer_size); + memset(buffer, 'a', buffer_size); + + tcp_socketpair(&client, &server); + set_blocking(client, FALSE); + + value = 4096; + ret = setsockopt(server, SOL_SOCKET, SO_SNDBUF, (char *)&value, sizeof(value)); + ok(!ret, "got %d, error %u\n", ret, WSAGetLastError()); + ret = setsockopt(client, SOL_SOCKET, SO_RCVBUF, (char *)&value, sizeof(value)); + ok(!ret, "got %d, error %u\n", ret, WSAGetLastError()); + + event = CreateEventW(NULL, TRUE, FALSE, NULL); + ok(!!event, "got error %lu\n", GetLastError()); + + /* WSAEventSelect() makes the server socket non-blocking. */ + ret = WSAEventSelect(server, event, FD_WRITE); + ok(!ret, "got %d, error %u\n", ret, WSAGetLastError()); + + wait = WSAWaitForMultipleEvents(1, &event, FALSE, 1000, FALSE); + ok(wait == WSA_WAIT_EVENT_0, "got wait %#lx\n", wait); + + memset(&events, 0xcc, sizeof(events)); + ret = WSAEnumNetworkEvents(server, event, &events); + ok(!ret, "got %d, error %u\n", ret, WSAGetLastError()); + ok(events.lNetworkEvents == FD_WRITE, "got events %#lx\n", events.lNetworkEvents); + ok(!events.iErrorCode[FD_WRITE_BIT], "got error %d\n", events.iErrorCode[FD_WRITE_BIT]); + + for (i = 0; i < max_sends; ++i) + { + ret = socket_select_writable(server); + if (ret == SOCKET_ERROR) + { + error = WSAGetLastError(); + break; + } + if (!ret) + { + select_blocked = 1; + break; + } + + ret = send(server, buffer, buffer_size, 0); + if (ret == SOCKET_ERROR) + { + error = WSAGetLastError(); + if (error == WSAEWOULDBLOCK) + send_blocked = 1; + break; + } + if (ret != buffer_size) + { + short_send = ret; + break; + } + } + + ok(!error || send_blocked, "got error %u\n", error); + ok(!short_send, "got short send %d\n", short_send); + + if (send_blocked) + skip("send returned WSAEWOULDBLOCK before select observed not-writable\n"); + else if (!select_blocked) + skip("select remained writable after %d sends\n", max_sends); + else + { + ret = socket_select_writable(server); + ok(!ret, "got %d\n", ret); + + while ((ret = recv(client, buffer, buffer_size, 0)) > 0) + ; + ok(ret == SOCKET_ERROR && WSAGetLastError() == WSAEWOULDBLOCK, + "got %d, error %u\n", ret, WSAGetLastError()); + + /* FD_WRITE re-arms only after send() fails with WSAEWOULDBLOCK; observing + * not-writable through poll does not re-arm it, as verified on Windows Server 2022. */ + wait = WSAWaitForMultipleEvents(1, &event, FALSE, 1000, FALSE); + ok(wait == WSA_WAIT_TIMEOUT, "got wait %#lx\n", wait); + } + + WSAEventSelect(server, NULL, 0); + CloseHandle(event); + closesocket(server); + closesocket(client); + free(buffer); +} + static void test_read_events(struct event_test_ctx *ctx) { OVERLAPPED overlapped = {0}; @@ -15102,6 +15368,8 @@ START_TEST( sock ) test_write_watch(); test_events(); + test_send_writability(); + test_write_event_no_rearm_after_select(); test_select_after_WSAEventSelect(); test_ipv6only(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11272
From: Martyn Forryan <winehq-gitlab@forryan.co.uk> Linux withholds POLLOUT from stream sockets until the send queue reaches its low-water mark, while sendmsg() accepts data whenever sk_wmem_queued is below sk_sndbuf. This leaves sockets reported as not writable even though a send would succeed. Use SO_MEMINFO to report writability from the kernel's own send-accept condition. Apply the same condition in the send_socket blocking gate; otherwise Wine parks a blocking send on raw POLLOUT even when the kernel would accept it. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=59893 --- dlls/ws2_32/tests/sock.c | 2 +- server/sock.c | 39 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 44a6977f66c..3009d529e1b 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -7277,7 +7277,7 @@ static void test_send_writability(void) ret = setsockopt(server, SOL_SOCKET, SO_SNDBUF, (char *)&value, sizeof(value)); ok(!ret, "got %d, error %u\n", ret, WSAGetLastError()); writable = socket_select_writable(server); - todo_wine ok(writable == 1, "got writable %d\n", writable); + ok(writable == 1, "got writable %d\n", writable); ret = send(server, buffer, sizeof(buffer), 0); send_error = ret == SOCKET_ERROR ? WSAGetLastError() : 0; ok(ret == sizeof(buffer), "got %d, error %u\n", ret, send_error); diff --git a/server/sock.c b/server/sock.c index 0716b3f71f4..403a2d9aff1 100644 --- a/server/sock.c +++ b/server/sock.c @@ -62,6 +62,9 @@ #ifdef HAVE_LINUX_RTNETLINK_H # include <linux/rtnetlink.h> #endif +#ifdef SO_MEMINFO +# include <linux/sock_diag.h> +#endif #ifdef HAVE_NETIPX_IPX_H # include <netipx/ipx.h> @@ -3577,6 +3580,29 @@ static void handle_exclusive_poll(struct poll_req *req) } } +static int sock_stream_send_ready( struct sock *sock ) +{ +#ifdef SO_MEMINFO + unsigned int meminfo[SK_MEMINFO_VARS]; + socklen_t len = sizeof(meminfo); + int unix_fd; + + if (sock->type != WS_SOCK_STREAM || sock->state != SOCK_CONNECTED || sock->wr_shutdown) + return 0; + + if ((unix_fd = get_unix_fd( sock->fd )) < 0) + return 0; + + if (getsockopt( unix_fd, SOL_SOCKET, SO_MEMINFO, meminfo, &len )) return 0; + + return len >= (SK_MEMINFO_WMEM_QUEUED + 1) * sizeof(*meminfo) && + meminfo[SK_MEMINFO_WMEM_QUEUED] < meminfo[SK_MEMINFO_SNDBUF]; +#else + (void)sock; + return 0; +#endif +} + static void poll_socket( struct sock *poll_sock, struct async *async, int exclusive, timeout_t timeout, unsigned int count, const struct afd_poll_socket_64 *sockets ) { @@ -3638,7 +3664,17 @@ static void poll_socket( struct sock *poll_sock, struct async *async, int exclus pollfd.fd = get_unix_fd( sock->fd ); pollfd.events = poll_flags_from_afd( sock, mask ); if (pollfd.events >= 0 && poll( &pollfd, 1, 0 ) >= 0) + { + if ((mask & AFD_POLL_WRITE) && + !(pollfd.revents & (POLLOUT | POLLERR | POLLHUP)) && + sock->type == WS_SOCK_STREAM && sock->state == SOCK_CONNECTED && + !sock->wr_shutdown) + { + if (sock_stream_send_ready( sock )) + pollfd.revents |= POLLOUT; + } sock_poll_event( sock->fd, pollfd.revents ); + } /* FIXME: do other error conditions deserve a similar treatment? */ if (sock->state != SOCK_CONNECTING && sock->errors[AFD_POLL_BIT_CONNECT_ERR] && (mask & AFD_POLL_CONNECT_ERR)) @@ -4052,7 +4088,8 @@ DECL_HANDLER(send_socket) * asyncs will not consume all available space; if there's no space * available, the current request won't be immediately satiable. */ - if ((!force_async && sock->nonblocking) || check_fd_events( sock->fd, POLLOUT )) + if ((!force_async && sock->nonblocking) || check_fd_events( sock->fd, POLLOUT ) || + sock_stream_send_ready( sock )) { /* Give the client opportunity to complete synchronously. * If it turns out that the I/O request is not actually immediately satiable, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11272
Force-pushed a reworked version; the description is updated to match. The original version could deadlock. TIOCOUTQ reports queued payload bytes, but the kernel blocks a send on truesize accounting (sk_wmem_queued against sk_sndbuf), so near the blocking boundary it could report a socket writable when a send would in fact block. The full ws2_32:sock unit reproduces this deterministically here: test_select walks a blocking send into a full socket the old predicate had just called writable, and it never returns. That was present in the version you were reviewing. Chasing it exposed the same divergence a second time, inside send_socket(), which the description covers. The new version uses one predicate at both sites, taken from the kernel rather than approximated. @zfigura, on your point about waking a wait that is already blocked without kernel changes: I built the FD_WRITE re-arm and tested it against real Windows before submitting anything. Windows does not re-arm FD_WRITE when a poll observes the socket as not writable; the wait times out exactly as it does on Wine. Only a failed send re-arms. So the re-arm is not the answer, and the series pins that behaviour in a test instead. @gofman, your remark about Windows never short-writing shaped the test design. The fill loops stop at the first WSAEWOULDBLOCK and skip rather than assert when the state cannot be reached, which is what keeps them meaningful on the TestBot Windows images. TestBot for the new series, no new failures across XP through Windows 11 and the Wine VMs: https://testbot.winehq.org/JobDetails.pl?Key=163922 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11272#note_148071
I still don't understand which is the practical advantage of making this Linux specific, depending on gore kernel implementation details (it does IMO, if that is not could you please link some documentation or socket specification describing these details?). While that can be easily avoided. The downsides are clear: - it is not fully correct, already waiting select won't be woken in the same logic; - it is complicated and depends on details of Linux kernal implementation (unless I am wrong and that is specified / documented somewhere? in that case we can remove this part about internal details but it is still complicated); - it will work this way on Linux only. If I am missing some important disadvantages of alternative solution (which will also fix that short write thing on the way), maybe those can be named? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11272#note_148074
On Wed Aug 5 22:58:37 2026 +0000, Paul Gofman wrote:
I still don't understand which is the practical advantage of making this Linux specific, depending on gore kernel implementation details (it does IMO, if that is not could you please link some documentation or socket specification describing these details?). While that can be easily avoided. The downsides are clear: - it is not fully correct, already waiting select won't be woken in the same logic; - it is complicated and depends on details of Linux kernal implementation (unless I am wrong and that is specified / documented somewhere? in that case we can remove this part about internal details but it is still complicated); - it will work this way on Linux only. If I am missing some important disadvantages of alternative solution (which will also fix that short write thing on the way), maybe those can be named? Thanks Paul, that's a fair challenge and I'd rather get this right than get it merged.
You're right that it's Linux-only, and I don't have a specification to point you at. `wmem_queued < sndbuf` is the condition `sendmsg()` itself applies, but as kernel internals rather than documented API, so that objection stands as you put it. The already-waiting select is a gap I conceded earlier and then didn't fix. On complexity, I'd like to come back to you properly. I've started reading through what your alternative would actually involve, and I have some early notes. Two things, still provisional: the change looks wider than the three steps suggest, because `select()`/`WSAPoll` don't reach POLLOUT through the event-select mask in `sock_get_poll_events()` but through the `poll_list` loop and `get_poll_flags()`, so that funnel is the surface that matters. And there are lifetime questions around data already reported as sent: half-close, `SO_LINGER` against the dup'd fd, and close with a queue outstanding. I'd want answers to those rather than guesses. Give me a day or two, and I'll come back with the detail on both, plus the disadvantages you asked for, since there are some worth naming on each side. My starting point was to keep the change as small and as contained as I could, partly because it only bites a fairly narrow set of senders, and most users never see it, so a large rework felt disproportionate to the fault. That's a judgement about scope rather than about the design, and if you'd rather it went the other way, I'm happy to do the work. If the queue-based design is where you and Elizabeth want this to go, I'd rather spend the time on that than defend what I've already written. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11272#note_148114
On Wed Aug 5 22:58:37 2026 +0000, Martyn Forryan wrote:
Thanks Paul, that's a fair challenge and I'd rather get this right than get it merged. You're right that it's Linux-only, and I don't have a specification to point you at. `wmem_queued < sndbuf` is the condition `sendmsg()` itself applies, but as kernel internals rather than documented API, so that objection stands as you put it. The already-waiting select is a gap I conceded earlier and then didn't fix. On complexity, I'd like to come back to you properly. I've started reading through what your alternative would actually involve, and I have some early notes. Two things, still provisional: the change looks wider than the three steps suggest, because `select()`/`WSAPoll` don't reach POLLOUT through the event-select mask in `sock_get_poll_events()` but through the `poll_list` loop and `get_poll_flags()`, so that funnel is the surface that matters. And there are lifetime questions around data already reported as sent: half-close, `SO_LINGER` against the dup'd fd, and close with a queue outstanding. I'd want answers to those rather than guesses. Give me a day or two, and I'll come back with the detail on both, plus the disadvantages you asked for, since there are some worth naming on each side. My starting point was to keep the change as small and as contained as I could, partly because it only bites a fairly narrow set of senders, and most users never see it, so a large rework felt disproportionate to the fault. That's a judgement about scope rather than about the design, and if you'd rather it went the other way, I'm happy to do the work. If the queue-based design is where you and Elizabeth want this to go, I'd rather spend the time on that than defend what I've already written. Thanks for your patience on this. I wanted to come back with something more useful than a defence of what I had already written, which took a while. Taking your questions in order.
On the practical advantage of doing it this way, there is not one worth having. The appeal was that it is a guarded read I could revert in a line, and it fixed the case I could reproduce. Set against your three objections that is not much of a case, and I would rather spend the time on the alternative than defend it. On documentation, I am afraid not, and I should be straighter about this than I was earlier in the thread. When I argued the check was not really Linux-specific I was defending the `TIOCOUTQ` version, where both quantities are documented byte counts. The current revision uses `SO_MEMINFO` and tests `wmem_queued < sndbuf`, which is `sk_stream_memory_free()` and carries no stability contract at all. The reason I moved to it is that `TIOCOUTQ` reports queued payload while the kernel blocks on `sk_wmem_queued`, which counts per-skb overhead, so the two disagree exactly at the boundary the patch cares about. That was necessary for correctness, and it made your objection stronger rather than weaker. The already-waiting select is a real gap. I conceded it earlier and then did not fix it, and I do not think it can be fixed from the poll answer alone. So to the part you asked about. The disadvantages of the queue approach, as far as I can see them. The blast radius is the core send path for every socket application, where the current patch is a guarded read on one path. It also changes behaviour on platforms that have no bug, since `poll()` already reports at the low-water mark on the BSDs and macOS. The `rem_async` allocation and copy move from the rare partial-write case into every would-block, so a large send becomes a large copy in the hot path. Error reporting after acceptance has nowhere good to go once the application has been told its send succeeded. Lifetime is the sharp end of that: a peer FIN wakes the write queue with success and the remainder is dropped, so a half-close silently discards bytes the application was told were sent. `SO_LINGER` has no server-side representation and the dup'd fd means kernel linger applies to the wrong close. And `sock_close_handle()` leaves `write_q` alone, so a stalled peer pins the socket object and the copied buffer until the process exits. `test_select`'s fill loop will need revisiting: it runs on a blocking socket and exits only when `select()` reports not-writable. The one I would most like your view on is the bound, and it goes back to Elizabeth's point about Windows having a buffer limit. Bounding at one async in flight means a small send after a large queued one fails, where Windows would accept it, because Windows bounds by `SO_SNDBUF` in bytes rather than by outstanding operations. I wondered whether a byte bound against `SO_SNDBUF` would be closer, and whether it would also make the FD_WRITE rule fall out rather than need defining: a send past the bound fails with `WSAEWOULDBLOCK`, which is the existing trigger, and the clear in `send_socket_completion_callback()` already fires on a failed send. `SO_SNDBUF = 0` would need handling separately, as the case where Windows does no buffering at all. You will know if there is a reason none of that works. Elizabeth's objection is that the queue approach does not fix the discrepancy, because draining is still throttled by POLLOUT. I read that as a point about the drain rather than about the application's wait, and I cannot tell whether the distinction matters in practice. The application would no longer be the thing waiting, but its next send would still be admitted only when Wine's queue clears, and that clearing is governed by when Linux raises POLLOUT. Whether half-buffer granularity keeps the pipe full is the part I would settle before anyone writes code, and I would rather have your and Elizabeth's read on that than my own. One thing still open from earlier in the thread: Elizabeth asked why the check guards `SOCK_STREAM` at all. I offered to drop the type check, cover connected datagram sockets, and extend the test. That is still on the table whichever direction this goes. One other thing, unrelated to the design question but found while chasing this. Wine answers `SIO_IDEAL_SEND_BACKLOG_QUERY` with a hard-coded 64 KB, and the application in the bug report sizes its send buffer directly from that answer, so on a long path it ends up with a buffer far smaller than the connection wants. I want to measure what Windows actually returns before I claim anything, so I will raise it separately with the numbers rather than fold it into this. What I would like is for you to tell me how you want this built, and then I will build it that way. If it would help, I will write up a plan against the actual call sites first so there is something concrete to correct before any code exists. I am not looking for the quick version. This touches a good deal more of the send path than the current patch does, and I would rather spend the time getting it right than land something fast that introduces problems elsewhere. I have the time for it either way. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11272#note_148351
On Fri Aug 7 23:46:49 2026 +0000, Martyn Forryan wrote:
Thanks for your patience on this. I wanted to come back with something more useful than a defence of what I had already written, which took a while. Taking your questions in order. On the practical advantage of doing it this way, there is not one worth having. The appeal was that it is a guarded read I could revert in a line, and it fixed the case I could reproduce. Set against your three objections that is not much of a case, and I would rather spend the time on the alternative than defend it. On documentation, I am afraid not, and I should be straighter about this than I was earlier in the thread. When I argued the check was not really Linux-specific I was defending the `TIOCOUTQ` version, where both quantities are documented byte counts. The current revision uses `SO_MEMINFO` and tests `wmem_queued < sndbuf`, which is `sk_stream_memory_free()` and carries no stability contract at all. The reason I moved to it is that `TIOCOUTQ` reports queued payload while the kernel blocks on `sk_wmem_queued`, which counts per-skb overhead, so the two disagree exactly at the boundary the patch cares about. That was necessary for correctness, and it made your objection stronger rather than weaker. The already-waiting select is a real gap. I conceded it earlier and then did not fix it, and I do not think it can be fixed from the poll answer alone. So to the part you asked about. The disadvantages of the queue approach, as far as I can see them. The blast radius is the core send path for every socket application, where the current patch is a guarded read on one path. It also changes behaviour on platforms that have no bug, since `poll()` already reports at the low-water mark on the BSDs and macOS. The `rem_async` allocation and copy move from the rare partial-write case into every would-block, so a large send becomes a large copy in the hot path. Error reporting after acceptance has nowhere good to go once the application has been told its send succeeded. Lifetime is the sharp end of that: a peer FIN wakes the write queue with success and the remainder is dropped, so a half-close silently discards bytes the application was told were sent. `SO_LINGER` has no server-side representation and the dup'd fd means kernel linger applies to the wrong close. And `sock_close_handle()` leaves `write_q` alone, so a stalled peer pins the socket object and the copied buffer until the process exits. `test_select`'s fill loop will need revisiting: it runs on a blocking socket and exits only when `select()` reports not-writable. The one I would most like your view on is the bound, and it goes back to Elizabeth's point about Windows having a buffer limit. Bounding at one async in flight means a small send after a large queued one fails, where Windows would accept it, because Windows bounds by `SO_SNDBUF` in bytes rather than by outstanding operations. I wondered whether a byte bound against `SO_SNDBUF` would be closer, and whether it would also make the FD_WRITE rule fall out rather than need defining: a send past the bound fails with `WSAEWOULDBLOCK`, which is the existing trigger, and the clear in `send_socket_completion_callback()` already fires on a failed send. `SO_SNDBUF = 0` would need handling separately, as the case where Windows does no buffering at all. You will know if there is a reason none of that works. Elizabeth's objection is that the queue approach does not fix the discrepancy, because draining is still throttled by POLLOUT. I read that as a point about the drain rather than about the application's wait, and I cannot tell whether the distinction matters in practice. The application would no longer be the thing waiting, but its next send would still be admitted only when Wine's queue clears, and that clearing is governed by when Linux raises POLLOUT. Whether half-buffer granularity keeps the pipe full is the part I would settle before anyone writes code, and I would rather have your and Elizabeth's read on that than my own. One thing still open from earlier in the thread: Elizabeth asked why the check guards `SOCK_STREAM` at all. I offered to drop the type check, cover connected datagram sockets, and extend the test. That is still on the table whichever direction this goes. One other thing, unrelated to the design question but found while chasing this. Wine answers `SIO_IDEAL_SEND_BACKLOG_QUERY` with a hard-coded 64 KB, and the application in the bug report sizes its send buffer directly from that answer, so on a long path it ends up with a buffer far smaller than the connection wants. I want to measure what Windows actually returns before I claim anything, so I will raise it separately with the numbers rather than fold it into this. What I would like is for you to tell me how you want this built, and then I will build it that way. If it would help, I will write up a plan against the actual call sites first so there is something concrete to correct before any code exists. I am not looking for the quick version. This touches a good deal more of the send path than the current patch does, and I would rather spend the time getting it right than land something fast that introduces problems elsewhere. I have the time for it either way. I'm sorry, I'm having trouble reading through all of this. If you used AI to write or help write it, please don't; it invariably makes things harder to read (and be aware that using AI to write or help write patches themselves is not allowed for Wine). Otherwise can you please find a way to respond more succinctly?
As far as I'm aware the main question is if there's any approach submitted or proposed which actually helps libcurl; if so we just need an explanation of what the call structure looks like such that the POLLOUT problem doesn't occur; if not we can look into putting something in the kernel, or maybe faking SNDBUF values. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11272#note_148488
On Mon Aug 10 15:43:55 2026 +0000, Elizabeth Figura wrote:
I'm sorry, I'm having trouble reading through all of this. If you used AI to write or help write it, please don't; it invariably makes things harder to read (and be aware that using AI to write or help write patches themselves is not allowed for Wine). Otherwise can you please find a way to respond more succinctly? As far as I'm aware the main question is if there's any approach submitted or proposed which actually helps libcurl; if so we just need an explanation of what the call structure looks like such that the POLLOUT problem doesn't occur; if not we can look into putting something in the kernel, or maybe faking SNDBUF values. Apologies; I've been known for excess verbosity since school. I like to try and ensure I've covered everything possible, especially with a complicated or technical issue, but of course it runs the risk of turning into word soup.
Let me try again to answer your question with brevity: yes, the submitted patch. curl is the case it was written for. curl's loop, one iteration per burst: ``` if (select(writefds) says writable) send(~64K, non-blocking) /* succeeds, produce next chunk, loop */ else WSAWaitForMultipleEvents(FD_WRITE, 1000ms) ``` FD_WRITE only arms after a send fails with WSAEWOULDBLOCK. curl is application-limited and never fills the buffer, so no send ever fails, so the event is never armed. On stock Wine select() reports not-writable while the queue sits above Linux's POLLOUT threshold, curl takes the else branch into a wait nothing can signal, and burns the full second. One burst per second is the \~140 KB/s. With select() reporting writable whenever a send would be accepted, curl stays on the send path and only reaches that wait after send() returns WSAEWOULDBLOCK, which is the only time it can wake promptly. Measured: \~1 Mbit/s to \~79 Mbit/s on a 110 Mbit/s uplink, and the conformance test goes from \~230 violations in \~660 sends to zero. What it does not fix is a select() already blocked when space frees later. I conceded that to Paul and I still think it cannot be closed from the poll answer, so that gap is where a kernel change would be needed. Faking SNDBUF would not help this case. The stall is the POLLOUT threshold, not the buffer size. One thing to settle between you; Paul's objections to this approach stand and I had conceded them, and offered to build the write-queue alternative instead. You have since said that alternative does not fix the discrepancy, because sending from the queue is still throttled by POLLOUT. So the choice is this patch, Linux-specific and with the already-waiting gap, or the kernel route. I am happy to build whichever you both choose. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11272#note_148550
Apologies; I've been known for excess verbosity since school. I like to try and ensure I've covered everything possible, especially with a complicated or technical issue, but of course it runs the risk of turning into word soup.
Thanks. Honestly it would probably help a lot to quote-reply when responding to multiple different things at once.
curl is application-limited and never fills the buffer,
Well, not quite, the thing is Windows never does a partial send, so if we're below SNDBUF the buffer is effectively unlimited. If that makes sense.
so no send ever fails, so the event is never armed.
Which is how it works on Windows as far as we've tested. Even if you fill up the sndbuf such that the next send would fail, it doesn't reset FD_WRITE. Which means that this libcurl loop is actually just broken on Windows? Or is there some discrepancy there? So this patch only helps libcurl because libcurl is broken and waits 1 second every time it fills the buffer, so filling the buffer a little more increases throughput??? If that's really what's happening we should just fix curl instead. Like, Windows compatibility and all, but this is an obvious bug and there's no preexisting application we need to worry about.
Faking SNDBUF would not help this case. The stall is the POLLOUT threshold, not the buffer size.
I mean on the Linux side, inflating the SNDBUF value we pass to Linux so that it's closer to the 2/3 mark, so POLLOUT is triggered when we hit the win32 SNDBUF value. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11272#note_148553
So this patch only helps libcurl because libcurl is broken and waits 1 second every time it fills the buffer, so filling the buffer a little more increases throughput???
If that's really what's happening we should just fix curl instead. Like, Windows compatibility and all, but this is an obvious bug and there's no preexisting application we need to worry about.
Yeah, no, it's more complicated than that; they did run into that problem: https://github.com/curl/curl/pull/6245 and then fixed it by sending 0 bytes to every socket before polling it. So if it fails because the sndbuf is full, that resets it. And if it succeeds and doesn't reset FD_WRITE, then a following poll should also succeed because the sndbuf isn't full. Wine, or perhaps we should say Linux, breaks this assumption by always succeeding the send (because the true sndbuf isn't full) but then failing the poll (because we're over Linux's threshold). So this solution ends up working by checking if the sndbuf is *really* full. It still has the problem with POLLOUT not triggering until the threshold, but that's fine, because obviously you're not going to be actually limited by the size of the sndbuf as long as you keep it from running empty. It's still very awkward. Paul's suggestion of basically virtualizing the queue entirely is, meh, maybe better in the long run but still an awfully big hammer. Depending on Linux internals are ugly. I'm inclined to propose a simpler solution, which is to reset AFD_POLL_WRITE like we currently do in send_socket_completion_callback if the iosb fails OR the socket isn't writable according to POLLOUT. It's a bit odd in terms of compatibility but I think it avoids any problems like this, no matter where the kernel actually chooses to signal POLLOUT, while also being very simple. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11272#note_148556
Honestly it would probably help a lot to quote-reply
Noted, doing that from here.
Windows never does a partial send, so if we're below SNDBUF the buffer is effectively unlimited
Understood, and that is a better framing than mine.
they did run into that problem: [curl/curl#6245](https://github.com/curl/curl/issues/6245) and then fixed it by sending 0 bytes to every socket before polling it
That explains the piece I had wrong, thank you. So the assumption curl relies on is that a send which is accepted implies writable, and Linux breaks it by accepting the send while failing the poll.
reset AFD_POLL_WRITE like we currently do in send_socket_completion_callback if the iosb fails OR the socket isn't writable according to POLLOUT
[wine-fdwrite-rearm.patch](/uploads/d231e9f8fb6d128d5d278d1e104fcce4/wine-fdwrite-rearm.patch) I have this built already and have attached it. Mine sits in `poll_socket()` rather than at send completion, clearing `reported_events & AFD_POLL_WRITE` when a poll sees a connected stream that cannot accept a send. It applies on top of the writability patch in this MR rather than stock master, since its context needs `sock_stream_send_ready()`. I did not submit it because it contradicts the FD_WRITE re-arm test in this MR. I checked real Windows and it does not re-arm on a poll-observed not-writable. That test would need to become todo_wine, or the divergence accepted on purpose. Measured on a live upload, it took aggregate throughput from 13.0-13.9 Mbit/s to 40.9. That was on top of the writability patch rather than instead of it, so whether the re-arm alone is enough is untested. I can test that. Your version also answers Paul's objection in a way mine does not, since it needs nothing from the kernel beyond POLLOUT. If you want it at send completion as you describe, I will build it that way rather than push mine.
inflating the SNDBUF value we pass to Linux so that it's closer to the 2/3 mark
The multiplier depends on the exact condition the kernel tests, so I would rather derive it than assume 2/3. It also carries the same kernel-internal dependency Paul objected to, moved rather than removed. [wine-fdwrite-rearm.patch](/uploads/d32153d4c5b48e1bc6ee53414b7c42ad/wine-fdwrite-rearm.patch) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11272#note_148673
I have this built already and have attached it. Mine sits in `poll_socket()` rather than at send completion, clearing `reported_events & AFD_POLL_WRITE` when a poll sees a connected stream that cannot accept a send. It applies on top of the writability patch in this MR rather than stock master, since its context needs `sock_stream_send_ready()`.
I did not submit it because it contradicts the FD_WRITE re-arm test in this MR. I checked real Windows and it does not re-arm on a poll-observed not-writable. That test would need to become todo_wine, or the divergence accepted on purpose.
Yeah, I don't think we want to do that if it's incorrect. Why this instead of in send_socket_completion_callback()? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11272#note_148678
Yeah, I don't think we want to do that if it's incorrect. Why this instead of in send_socket_completion_callback()?
It went in `poll_socket()` simply because the writability block was already there, so it was a few lines on top of something I had built anyway. I should be clear about what it is. It is a stopgap in a container I maintain, carried while a proper fix is worked out here, and dropped the moment one is merged. It also presumes one: it will not apply to stock master at all, since its context needs `sock_stream_send_ready()` from this MR. I attached it because you described the same idea, not because I think the placement is right. The two patches are doing different jobs, which is why I run both. The writability patch keeps a curl-shaped sender out of a wait it cannot be woken from. The re-arm helps the ones that end up in it anyway. On a transmission of data with the writability patch already applied, aggregate throughput went from 13-14 Mbit/s to 41, at an unchanged per-connection ceiling, so that second gain is recovered idle time rather than a faster pipe. I have never measured the re-arm on its own, so I cannot tell you what it is worth without the first. Yours is the better place of the two. Windows re-arms off the outcome of a send, so hooking send completion is at least the right shape. A poll is something Windows never re-arms on at all. I do not think the placement is what makes it incorrect, though. You pointed out earlier that filling the sndbuf so the next send would fail does not reset FD_WRITE on Windows. Resetting AFD_POLL_WRITE when the iosb succeeded but POLLOUT is clear is that same case: the send went through, the buffer is now full, and Windows would leave the event alone. It is the same divergence as mine, moved to a better place. On that basis it would rule out both. I raise it because I have been trying throughout to keep my changes matching what real Windows does, which is why I keep testing this work against Windows VMs rather than reasoning about what it ought to do. That leaves the SO_MEMINFO version with Paul's objections standing against it, or the kernel route. I'm happy to go in any direction, and I have no attachment to what I have already written. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11272#note_148686
participants (4)
-
Elizabeth Figura (＠zfigura) -
Martyn Forryan
-
Martyn Forryan (＠foz)
-
Paul Gofman (＠gofman)