Repository navigation
SSLSocket#write_nonblock: hand over up to 1 MB, convert only that (flaky NetHttp_Tls1) - #33
Merged
Merged
Conversation
NetHttp_Tls1's write_timeout check failed now and then on ubuntu ([:write_timeout, false]: Net::WriteTimeout came, but after more than 10 s). net/protocol's write0 slices off what write_nonblock wrote and passes the rest back, a copy of the rest each time, and the transport converted all of the data to bytes on every call to send its first 16 KB. For the test's 64 MB body that was two copies of ~60 MB per 16 KB record until Linux's loopback buffers (several MB) were full: gigabytes of copying before the 1 s timeout could start. write_nonblock now converts only what it may send, and goes on with the next 16 KB record while the last one went out at once, up to 1 MB a call; what it reports written is still what the kernel took plus the record in flight.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 5 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
NetHttp_Tls1failed now and then on ubuntu (master's run for #28 on net10.0, #32's on net8.0) with[:write_timeout, false]:Net::WriteTimeoutwas raised, but more than 10 s after the start, for awrite_timeoutof 1 s.Cause
write0slices off whatwrite_nonblockwrote and passes the rest back:str = str.byteslice(len, -len), a copy of the rest every call (CRuby shares the buffer there).Transport#write_nonblockhanded over at most 16 KB a call, and ranConvertToBytes()on all of the data to do so.For the test's 64 MB body that was two copies of ~60 MB per 16 KB record until the kernel buffers were full. Loopback buffers on Linux hold several MB, so that came to hundreds of records and gigabytes of copying before the 1 s timeout could even start. On Windows the buffers are small, which is why only ubuntu ever failed.
Fix
write_nonblockconverts only the bytes it may send (GetBinarySlice).ssl.rbandDocs/net-http-over-tls.mdare updated.Testing
Built locally for net10.0, and NetHttp_Tls1, NetHttp_TlsVerifyCallback1, NetHttp_PlainNoOpenSsl1, NetHttp_GemNetTls1, Ssl_NonBlocking1 and Puma_Tls1 pass on Windows. The flake only shows on ubuntu, where loopback buffers are large, so the ubuntu jobs are the ones that count.