Fix SIGABRT when destroying OdCDS handles on worker threads #42253
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
wdauchy
force-pushed
the
odcds
branch
5 times, most recently
from
15f79be to
b782257
Compare
Member
|
@adisuissa as our xDS expert |
wdauchy
force-pushed
the
odcds
branch
3 times, most recently
from
058d2f6 to
95ede76
Compare
The OdCdsApiHandleImpl destructor can be invoked on worker threads when filters are removed (e.g., during VHDS filter updates). When the last reference to an OdCdsApiSharedPtr is destroyed on a worker thread, the subscription cleanup code attempts to run on the wrong thread, causing an assertion failure or SIGABRT since subscription operations must execute on the main thread. This fix adds a destructor to OdCdsApiHandleImpl that checks if destruction is happening on a worker thread. If so, it dispatches the OdCdsApiSharedPtr destruction to the main thread dispatcher via a lambda, ensuring the subscription is properly cleaned up on the thread where it was created. The fix follows the same pattern used elsewhere in Envoy (e.g., ThreadLocal::SlotImpl destructor) for cross-thread cleanup operations. Note: A longer-term optimization could cache and reuse OdCDS handles in ClusterManager with reference counting and deferred cleanup, avoiding unnecessary subscription teardown/recreation during filter rotations. This is tracked by the existing TODO in allocateOdCdsApi(). Signed-off-by: William Dauchy <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Commit Message:
The OdCdsApiHandleImpl destructor can be invoked on worker threads when filters are removed (e.g., during VHDS filter updates). When the last reference to an OdCdsApiSharedPtr is destroyed on a worker thread, the subscription cleanup code attempts to run on the wrong thread, causing an assertion failure or SIGABRT since subscription operations must execute on the main thread.
This fix adds a destructor to OdCdsApiHandleImpl that checks if destruction is happening on a worker thread. If so, it dispatches the OdCdsApiSharedPtr destruction to the main thread dispatcher via a lambda, ensuring the subscription is properly cleaned up on the thread where it was created.
The fix follows the same pattern used elsewhere in Envoy (e.g., ThreadLocal::SlotImpl destructor) for cross-thread cleanup operations.
Additional Description:
Note: A longer-term optimization could cache and reuse OdCDS handles in ClusterManager with reference counting and deferred cleanup, avoiding unnecessary subscription teardown/recreation during filter rotations. This is tracked by the existing TODO in
allocateOdCdsApi().Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]