Repository navigation
Conversation
Signed-off-by: wbpcode <wbphub@gmail.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain to block approval.
No open issues 🎉
What changed in this PR
Hardens the IntrusiveList introduced in #43967 and migrates selected non-core extensions to reduce per-item allocations. Core users and filter chains remain unchanged.
Changes:
- Prevents node copying and moving, asserts safe destruction, handles null insertion and invalid removal, and documents removal during iteration.
- Migrates generic proxy, Dubbo, RocketMQ, and SIP TRA lists; removes unused list inheritance from generic proxy upstream requests.
- Adds hardening tests and adapts existing tests to the intrusive-list API.
| File | Description |
|---|---|
test/extensions/filters/network/generic_proxy/proxy_test.cc |
Updates active-stream access to use front(). |
test/common/common/linked_object_test.cc |
Tests node ownership constraints and defensive behavior. |
test/common/common/BUILD |
Adds support for Envoy bug assertions in tests. |
source/extensions/filters/network/generic_proxy/router/router.h |
Removes unused LinkedObject inheritance and include. |
source/extensions/filters/network/generic_proxy/router/BUILD |
Removes the unused linked-object dependency. |
source/extensions/filters/network/generic_proxy/proxy.h |
Converts active streams to intrusive nodes and storage. |
source/extensions/filters/network/generic_proxy/proxy.cc |
Adapts stream insertion, removal, and access. |
source/extensions/filters/network/dubbo_proxy/conn_manager.h |
Converts active-message storage and its test accessor. |
source/extensions/filters/network/dubbo_proxy/conn_manager.cc |
Adapts message insertion and deferred removal. |
source/extensions/filters/network/dubbo_proxy/active_message.h |
Makes active messages intrusive-list nodes. |
source/common/common/linked_object.h |
Hardens node lifecycle, insertion, and removal. |
contrib/sip_proxy/filters/network/source/tra/tra_impl.h |
Converts callback nodes, storage, and cleanup. |
contrib/sip_proxy/filters/network/source/tra/tra_impl.cc |
Uses intrusive insertion for TRA callbacks. |
contrib/rocketmq_proxy/filters/network/test/router_test.cc |
Adapts message setup and access to the new API. |
contrib/rocketmq_proxy/filters/network/source/conn_manager.h |
Converts active-message storage and accessor. |
contrib/rocketmq_proxy/filters/network/source/conn_manager.cc |
Adapts message insertion, access, and removal. |
contrib/rocketmq_proxy/filters/network/source/active_message.h |
Makes active messages intrusive-list nodes. |
Review effort: Balanced
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.
Commit Message: common: harden IntrusiveList and migrate some extensions to it
Additional Description:
This hardens the
IntrusiveListadded in #43967 and moves a first batch of non-core extensions to it. Core users (router, connection pools, codecs, HCM) stay onLinkedObject/std::listfor now and will be migrated in follow-ups once this has been exercised.IntrusiveListhardening:IntrusiveListNodeis now non-copyable and non-movable. Nodes are owned throughunique_ptrand linked by address, so a copy or assignment of an inserted element previously aliased the list links and corrupted the owning list.push()/pushBack()ignore a nullunique_ptr.removeFromList()on a node that is not in a list is now anENVOY_BUGthat returnsnullptr, instead of a debug-only assert followed by a null dereference in release builds.next()must be saved before removing while iterating.Migrated to
IntrusiveList:ActiveStreamlist. Also drops the unusedLinkedObjectbase from the generic_proxy router'sUpstreamRequest, which is never placed in a list.ActiveMessagelist.ActiveMessagelist.The filter chains in these extensions keep
std::listiterators to resume iteration and are left unchanged.Risk Level: Low
Testing: Unit tests added for the new
IntrusiveListbehavior; existing tests of the migrated extensions pass.Docs Changes: N/A
Release Notes: N/A