stream info: prefer reference rather than shared pointer for route() by wbpcode · Pull Request #43938 · envoyproxy/envoy

wbpcode · 2026-03-13T02:23:33Z

Commit Message: stream info: return reference rather than copy of shared pointer
Additional Description:

This pr update previous route() as routeSharedPtr() and add a new route() which will return OptRef<const Route>.

Risk Level: low.
Testing: n/a.
Docs Changes: n/a.
Release Notes: n/a.
Platform Specific Features: n/a.

wbpcode · 2026-03-13T02:26:26Z

This bring minor change to interface of StreamInfo, so @ravenblackx @yanavlasov to avoid this affect your forks. (Although I think this won't bring any change to existing code, but in case)

nezdolik · 2026-03-13T10:29:37Z

We also have a fork (cilium-proxy), but this change does not seem to affect us.

agrawroh

LGTM, Thanks!

wbpcode · 2026-03-13T14:46:47Z

We also have a fork (cilium-proxy), but this change does not seem to affect us.

Yeah. It should won't affect any source code but may affect the test mock of route() method because now it will return a reference to Router::RouteConstSharedPtr. That's say the previous way to return a shared pointer of mock route will be affected.

ravenblackx

Thanks for the heads-up. Looks fine to me.

ravenblackx · 2026-03-13T15:09:11Z

/retest

tyxia

Thanks!

yanavlasov

LGTM from me. If others feel like this is a serious issue, please comment.

/wait-any

tyxia

Thanks！and benchmark

wbpcode · 2026-03-19T10:03:49Z

After offline discussion. We will migrate to two methods style. :)

wbpcode · 2026-03-20T17:11:39Z

/retest

nezdolik

lgtm, thanks!

ravenblackx · 2026-03-23T14:22:16Z

    FilterManager& manager_;
    absl::string_view filter_config_name_;
-    Router::RouteConstSharedPtr route_;
+    OptRef<const Router::Route> route_;


I'm a bit concerned about this one - seems like a place where the shared ownership might be important. Do we know where else owns this route, that definitely has a lifetime exceeding that of a FilterManager?

Even if it's okay, it's probably worth a comment here explaining who owns it and why we know their lifetime makes it okay to have just a ref here.

It's not easy to see why it's fine to use ref here on the github diff. But if you read the whole file, you will find the route_ here is a field of FilterChainFactoryCallbacksImpl which is only be used as temporarily object when creating new filter chain.

wbpcode · 2026-03-24T02:19:49Z

/retest

Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>

wbpcode · 2026-03-25T13:27:01Z

This latest commit change nothing but merge main and resolve the conflicts.

wbpcode · 2026-03-25T14:39:38Z

/retest

…nvoyproxy#43938) Commit Message: stream info: return reference rather than copy of shared pointer Additional Description: See envoyproxy#44025 This pr update previous `route()` as `routeSharedPtr()` and add a new `route()` which will return `OptRef<const Route>`. Risk Level: low. Testing: n/a. Docs Changes: n/a. Release Notes: n/a. Platform Specific Features: n/a. --------- Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com> Signed-off-by: Xuyang Tao <taoxuy@google.com>

…nvoyproxy#43938) Commit Message: stream info: return reference rather than copy of shared pointer Additional Description: See envoyproxy#44025 This pr update previous `route()` as `routeSharedPtr()` and add a new `route()` which will return `OptRef<const Route>`. Risk Level: low. Testing: n/a. Docs Changes: n/a. Release Notes: n/a. Platform Specific Features: n/a. --------- Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com> Signed-off-by: Jonathan Wu <jtwu@google.com>

…nvoyproxy#43938) Commit Message: stream info: return reference rather than copy of shared pointer Additional Description: See envoyproxy#44025 This pr update previous `route()` as `routeSharedPtr()` and add a new `route()` which will return `OptRef<const Route>`. Risk Level: low. Testing: n/a. Docs Changes: n/a. Release Notes: n/a. Platform Specific Features: n/a. --------- Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com> Signed-off-by: Nick Shipilov <nick.shipilov.n@gmail.com>

…nvoyproxy#43938) Commit Message: stream info: return reference rather than copy of shared pointer Additional Description: See envoyproxy#44025 This pr update previous `route()` as `routeSharedPtr()` and add a new `route()` which will return `OptRef<const Route>`. Risk Level: low. Testing: n/a. Docs Changes: n/a. Release Notes: n/a. Platform Specific Features: n/a. --------- Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>

wbpcode assigned ravenblackx and yanavlasov Mar 13, 2026

wbpcode requested review from agrawroh, kyessenov, mathetake, mattklein123, nezdolik and ravenblackx as code owners March 13, 2026 02:23

wbpcode marked this pull request as draft March 13, 2026 03:10

wbpcode marked this pull request as ready for review March 13, 2026 14:41

wbpcode requested review from tyxia and yanavlasov as code owners March 13, 2026 14:41

agrawroh previously approved these changes Mar 13, 2026

View reviewed changes

wbpcode assigned nezdolik Mar 13, 2026

ravenblackx previously approved these changes Mar 13, 2026

View reviewed changes

nezdolik previously approved these changes Mar 13, 2026

View reviewed changes

tyxia reviewed Mar 15, 2026

View reviewed changes

Comment thread envoy/stream_info/stream_info.h Outdated

yanavlasov reviewed Mar 17, 2026

View reviewed changes

repokitteh-read-only Bot added the waiting:any label Mar 17, 2026

tyxia previously approved these changes Mar 17, 2026

View reviewed changes

repokitteh-read-only Bot removed the waiting:any label Mar 17, 2026

wbpcode marked this pull request as draft March 19, 2026 10:03

wbpcode dismissed stale reviews from tyxia, nezdolik, and ravenblackx via 5667cfd March 20, 2026 07:53

wbpcode marked this pull request as ready for review March 20, 2026 07:56

wbpcode requested review from RyanTheOptimist and abeyad as code owners March 20, 2026 10:29

wbpcode force-pushed the dev-make-filter-stream-simple branch 2 times, most recently from 5888f0a to a19b99b Compare March 20, 2026 12:44

wbpcode changed the title ~~stream info: return reference rather than copy of shared pointer~~ stream info: prefer reference rather than shared pointer for route() Mar 20, 2026

wbpcode mentioned this pull request Mar 23, 2026

http: prefer reference rather than shared pointer for route() #44078

Merged

nezdolik reviewed Mar 23, 2026

View reviewed changes

nezdolik previously approved these changes Mar 23, 2026

View reviewed changes

ravenblackx reviewed Mar 23, 2026

View reviewed changes

wbpcode dismissed nezdolik’s stale review via 48a5c87 March 23, 2026 15:17

ravenblackx previously approved these changes Mar 23, 2026

View reviewed changes

wbpcode added 5 commits March 25, 2026 12:34

stream info: prefer reference rather than shared pointer for route()

c5e9cf5

Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>

fix test

61772b4

Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>

fix test

b85387f

Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>

make test a little more clear

7bf5d71

Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>

add a comment

e49d4ca

Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>

wbpcode dismissed ravenblackx’s stale review via e49d4ca March 25, 2026 13:19

wbpcode force-pushed the dev-make-filter-stream-simple branch from 48a5c87 to e49d4ca Compare March 25, 2026 13:19

nezdolik approved these changes Mar 25, 2026

View reviewed changes

wbpcode merged commit 5d8deb7 into envoyproxy:main Mar 26, 2026
29 checks passed

wbpcode deleted the dev-make-filter-stream-simple branch March 26, 2026 01:24

Conversation

wbpcode commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wbpcode commented Mar 13, 2026

Uh oh!

nezdolik commented Mar 13, 2026

Uh oh!

agrawroh left a comment

Choose a reason for hiding this comment

Uh oh!

wbpcode commented Mar 13, 2026

Uh oh!

ravenblackx left a comment

Choose a reason for hiding this comment

Uh oh!

ravenblackx commented Mar 13, 2026

Uh oh!

tyxia left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yanavlasov left a comment

Choose a reason for hiding this comment

Uh oh!

tyxia left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wbpcode commented Mar 19, 2026

Uh oh!

wbpcode commented Mar 20, 2026

Uh oh!

nezdolik left a comment

Choose a reason for hiding this comment

Uh oh!

ravenblackx Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

ravenblackx Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

wbpcode Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

wbpcode commented Mar 24, 2026

Uh oh!

wbpcode commented Mar 25, 2026

Uh oh!

wbpcode commented Mar 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

wbpcode commented Mar 13, 2026 •

edited

Loading

tyxia left a comment •

edited

Loading