router: fixing inline write fails for upstream connections#19221
router: fixing inline write fails for upstream connections#19221alyssawilk merged 3 commits intoenvoyproxy:mainfrom
Conversation
|
/wait |
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
c40a931 to
c5bbecd
Compare
4637fea to
618d683
Compare
eeab262 to
8e6a9fc
Compare
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
8e6a9fc to
2208965
Compare
|
Hey Snow, I'm throwing this your way as Matt's out. |
snowp
left a comment
There was a problem hiding this comment.
Just so I understand, what is the motivation for this change? The runtime flag talks about inline writes so I'm guessing it's related to that?
The code seems right to be if we're trying to eagerly call the clean up logic behind the upstream requests, just curious why we want this :)
|
Ah, so when TCP does writes, it puts data in the connection buffer, waits for an event, and then writes on the next epoll loop. for UDP we just write directly to the socket. If the write fails, we close the connection under the stack of the write, and here we delete the upstream request while doing work in the upstream request, resulting in a use-after-free. This adds a (really synthetic) test of writing fail due to an injected error, and fixes up the code so we don't have a use-after-free. |
snowp
left a comment
There was a problem hiding this comment.
LGTM, thanks for the explanation!
…y#19221) Changing the router code such that we do deferred delete of the upstream request any time we might be under the stack of the upstream request. the upstream request is still deleted inline for timeouts and other alarms. Note only HTTP/3 currently does inline writes, so this fixes a currently-http3-only issue Risk Level: high (changes lifetime of upstream requests) Testing: new integration test Docs Changes: n/a Release Notes: n/a Runtime guard: envoy.reloadable_features.allow_upstream_inline_write Signed-off-by: Alyssa Wilk <alyssar@chromium.org> Signed-off-by: Josh Perry <josh.perry@mx.com>
Changing the router code such that we do deferred delete of the upstream request any time we might be under the stack of the upstream request. the upstream request is still deleted inline for timeouts and other alarms.
Note only HTTP/3 currently does inline writes, so this fixes a currently-http3-only issue
Risk Level: high (changes lifetime of upstream requests)
Testing: new integration test
Docs Changes: n/a
Release Notes: n/a
Runtime guard: envoy.reloadable_features.allow_upstream_inline_write