Repository navigation
Conversation
Replace the dedicated BaseIOStream.read_until_close code path with a loop that calls read_bytes with partial=True, as recommended for callers in the method's own docstring. The externally observable behavior is unchanged: connection-reset-as-normal-close, StreamClosedError with real_error for closes with exc_info, inline propagation of errors from _try_inline_read, and StreamBufferFullError on buffer overflow are all preserved and exercised by the existing tests. The internal _read_until_close flag and its special case in close() are removed, and the stale read_until_close reference in the _read_to_buffer_loop target-bytes comment is updated. A new test confirms the loop reassembles chunks that arrive one byte at a time. The method is now also marked deprecated in its docstring, encouraging new callers to open-code the same loop for finer flow control and clearer error reporting after a partial response. Fixes tornadoweb#3728
This branch has not been deployed
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.
Fixes #3728.
BaseIOStream.read_until_closehad its own dedicated code path throughBaseIOStream: a_read_until_closeflag was set when the method wascalled, special-cased in
close()to deliver the buffered data (or let_signal_closedfail withStreamClosedError(real_error=…)), andcoordinated with the internal
_read_bufferas the data accumulatedthere until the stream closed. The docstring already pointed callers
at the same loop a user can write themselves —
read_byteswithpartial=True, iterated until aStreamClosedErrorwithreal_error=None.This PR removes the special case and makes the method just that loop:
read_until_close()kicks off the firstread_bytes(..., partial=True)synchronously so an inline error (closed stream whose read buffer is
empty, or a test that patches
_try_inline_read) still raises fromthe call itself, matching the historical behavior
(
test_read_until_close_with_error)._read_until_close_loop()is a smallasync defthat awaits thepending read, accumulates the chunk, requests the next partial read,
and treats
StreamClosedErrorwithreal_error=Noneor aconnection reset as a normal close (returning the accumulated bytes)
— preserving
test_read_until_close_after_close,test_read_until_close_after_error_close, andtest_read_until_close_after_connection_reset.len(buf)passesmax_buffer_sizeandraised as
StreamClosedError(real_error=StreamBufferFullError)withthe same
"Reached maximum read buffer size"log line used by_read_to_buffer. The old"error on read"log line from_handle_readis no longer emitted for this specific case becausethe overflow is now detected at the user-buffer level rather than
inside the read handler;
test_read_until_close_with_buffer_overflowis updated accordingly.
_read_until_closeattribute and its branch inclose()are deleted; the staleread_until_closemention in the_read_to_buffer_looptarget-bytes comment is updated.A new regression test (
test_read_until_close_chunked) covers theloop's reassembly path by driving
max_buffer_sizebytes throughread_until_closeone byte at a time with intervening checkpoints,ensuring the result fills up to (and no further than) the limit.
The method's docstring is now also tagged with
.. deprecated:: 6.6and a
.. versionchanged:: 6.6note explaining both the deprecationreasoning (finer flow control, clearer error reporting after a partial
response) and the under-the-hood refactor.
Testing
python3 -m tornado.testpasses (1390 tests, 79 skipped).python3 -m unittest tornado.test.iostream_test.TestIOStream tornado.test.iostream_test.TestIOStreamSSL tornado.test.iostream_test.TestIOStreamSSLContextpasses 108/108including every
test_read_until_close_*case.black --checkandmypyare clean on the changed files;flake8has no new warnings.
The changelog entry has been added under
docs/releases/v6.6.0.rstunder both "Deprecation notices" and the new
tornado.iostreamsection.