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.
Commit Message: test: fix null async tcp client dereference in tcp_async_client_integration_test
Additional Description:
test_network_async_tcp_filterdestroys itsAsyncTcpClientin the firstonData()whenkill_after_on_datais set. With half close enabled, the downstream payload and FIN can bedelivered to the filter in two separate read dispatches, so a second
onData()(and the downstreamonEvent()close handling) ran against the already destroyed client and dereferenced it, causingthe intermittent segfault reported in the issue. The coalescing of payload and FIN into one read
event is nondeterministic, which is why the flake was hard to reproduce. This guards the filter
onData()and downstreamonEvent()paths against a null client and adds a deterministicregression test,
ClientTearDownSplitFin, that delivers the FIN as a separate write so the secondread dispatch always occurs.
Risk Level: Low (test only, no production code change).
Testing:
ClientTearDownSplitFinfails 20 out of 20 runs onmain(segfault, faulting address0x0 in the downstream read dispatch, matching the crash in the issue) and passes after the guards.
The full
//test/integration:tcp_async_client_integration_testtarget passes 200 out of 200 runs(
--runs_per_test=200) locally in the CI docker image (clang), zero failures.check_formatpasses.Docs Changes: N/A
Release Notes: N/A (test only)
Platform Specific Features: N/A
Fixes #43233
Generative AI disclosure: This change was developed with AI assistance (Claude Code). I have
reviewed and understand every line, the read dispatch and half close race that produces the crash,
and the deterministic reproducer, and I take full ownership of the submission and of addressing
review feedback.