TS-4732: Changing the do_io_read API so it can be called with NULL and #857
Conversation
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
0 byte values. Allowing do_io_read and do_io_write to not warn on closed connection when we are only trying to disable the reads and writes for it. Removed some macros that were used in a few places (not all the time) and a couple that were not used at all.
Member
|
I've been thinking about this and wonder if we should avoid this and have an explicit |
|
FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/523/ for details. |
|
Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/420/ for details. |
| VIO * | ||
| UnixNetVConnection::do_io_read(Continuation *c, int64_t nbytes, MIOBuffer *buf) | ||
| { | ||
| ink_assert(c || 0 == nbytes); |
There was a problem hiding this comment.
Maybe move this assert after the new check and add an equivalent one to do_io_write?
Member
|
Looks good. Having an explicit disable_io_read/write might be nice. But using NULL's to clear out the current io's does't seem too surprising. |
Contributor
Author
|
Yeah, I started down the path of disable_read() and disable_write(), then ran into having to implement the methods in multiple locations (pluginvc) when declaring them pure virtual. I might switch back to disable_read() and disable_write()... |
Contributor
Author
|
I am going to go with this right now and open a bug for disable_read() and disable_write() |
bneradt
added a commit
to bneradt/trafficserver
that referenced
this pull request
Jul 12, 2024
Revert "Give a chance to send a response before receiving next request on H2 (apache#9997)" This reverts commit 62abe06. I'm reverting this commit because it seems to introduce the crash seen in YTSATS-4273.
masaori335
pushed a commit
to masaori335/trafficserver
that referenced
this pull request
May 29, 2025
* Fix pipelined request for chunked bodies This addresses a variety of bugs concerning pipelined requests. In particular, the HttpTunnel logic had fundamentally assumed that it could consume all bytes available in the producer's reader. If a request was pipelined after a previous request that had a chunked body, this would result in the second request being unparsed and either sent along to the origin or dropped on the floor, depening on configuration. This adds an explicit autest for pipelined requests and addresses these issues. This patch largely does the following: 1. Updates the copy_partial_post_data data to take the number of bytes it consumes rather than consuming all bytes in the reader. It also now returns the number of bytes it consumes, which the tunnel needs to keep track of the number of bytes it processes. 2. Previous to this patch, the HttpTunnel assumed that it could consume all bytes in the reader originally passed to it (all bytes in init_bytes_done). This simply will not work for pipelined requests. This addresses this issue by adding a new variable to the tunnel: bytes_consumed. This way the tunnel can keep track of how many bytes it consumed while processing the request body, which allows the HttpSM to later process just the right number of bytes from its reader rather than eating into any pipelined requests that follow it. 3. The HttpSM must not consume bytes from its client reader that are pipelined requests. It now uses the tunnel's processing bytes_consumed to process bytes from its reader rather than simply consuming all read_available() bytes from it. * Fix bytes consumed chunk computation Fix a possible miscalculation of bytes consumed while parsing chunked content. * Test verification updates. --------- Co-authored-by: Brian Neradt <brian.neradt@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
0 byte values. Allowing do_io_read and do_io_write to not warn on closed
connection when we are only trying to disable the reads and writes for it.
Removed some macros that were used in a few places (not all the time) and
a couple that were not used at all.