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