test: fix null async tcp client dereference in tcp_async_client_integration_test#46109
Open
davidvo-lyft wants to merge 1 commit into
Open
test: fix null async tcp client dereference in tcp_async_client_integration_test#46109davidvo-lyft wants to merge 1 commit into
davidvo-lyft wants to merge 1 commit into
Conversation
…ration_test With half close enabled, the downstream payload and FIN can arrive at the test network filter in two separate read dispatches. The first dispatch destroys the async client via the kill_after_on_data path, and the second dispatch dereferenced the destroyed client and segfaulted, which is the CI flake reported in issue envoyproxy#43233. Guard the filter onData and downstream onEvent paths against a null client, and add a ClientTearDownSplitFin regression test that delivers the FIN as a separate read event to reproduce the crash deterministically. Signed-off-by: David Vo <davidvo@lyft.com>
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
1 participant
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.
test_network_async_tcp_filterdestroys itsAsyncTcpClientin the firstonData()whenkill_after_on_datais set. With half close enabled, the payload and FIN can arrive as two separate read dispatches, and the second dispatch dereferenced the destroyed client (segfault, the CI flake in #43233). The coalescing is nondeterministic, which is why the flake was hard to reproduce. This guards the filteronData()and downstreamonEvent()paths against a null client and adds aClientTearDownSplitFinregression test that always delivers the FIN as a separate read event.Commit Message: test: fix null async tcp client dereference in tcp_async_client_integration_test
Additional Description: see above
Risk Level: Low. Test only, no production code change.
Testing:
ClientTearDownSplitFinfails 20 out of 20 runs on main (segfault, faulting address 0x0 in the downstream read dispatch, matching the issue) and passes with the guards.//test/integration:tcp_async_client_integration_testtarget: 200 out of 200 runs pass (--runs_per_test=200) in the CI clang image.check_formatpasses.Docs Changes: N/A
Release Notes: N/A (test only)
Platform Specific Features: N/A
Fixes #43233
Generative AI disclosure: Developed with AI assistance (Claude Code); I reviewed and understand every line and take full ownership of the submission.