tornadoweb / tornadoweb/tornado
Clear IOStream read buffer on connection close
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 22.2k
- Forks
- 5.6k
- Avg merge
- 3h 42m
- Merged PRs (30d)
- 16
Description
Commit c43996288e52a8f68728164f9284cf286d38543a clears the IOStream write buffer when the stream is closed. We should clear the read buffer as well, but there are currently tests that fail when that is done.
The issue is that we allow reads from buffered data after the underlying connection is closed. As long as one read leads directly (and synchronously) to another, the close callback is delayed. When there is a gap between the two reads, the close callback is run at that point, but we have some tests that rely on the availability of buffered data at this point. (We could perhaps declare these tests to be incorrect, but would need to offer some way for applications to indicate an intent to read when they need to preserve the buffered data).
There is additional discussion in #747, but note that the solution I mention there (acting as if there is a pending read when the buffer is non-empty but no read_callback is set) won't work because there is no guarantee that a read will come later (consider HTTPServer while a request is in flight)
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reviewing commit c43996288e52a8f68728164f9284cf286d38543a and the related discussion in #747, then identify the tests that fail when the read buffer is cleared. Determine behavior for buffered reads after connection close, including the HTTPServer case, and ensure the final tests cover the chosen lifecycle semantics.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100