[BUG] Fix io_context lifecycle management issues (#400)#410
Merged
Conversation
The intentional leak pattern applied in messaging_client now prevents heap corruption during static destruction. This allows the previously skipped tests to run safely in CI environments. Changes: - Remove TODO comments referencing Issues #315 and #348 - Remove GTEST_SKIP() for io_context lifecycle issues in CI - Update comments to reference Issue #400 and the applied fix - Tests now pass with CI=true environment variable Affected tests: - ErrorHandlingTest.ConnectToInvalidHost - ErrorHandlingTest.ConnectToInvalidPort - ErrorHandlingTest.ConnectionRefused - ErrorHandlingTest.RecoveryAfterConnectionFailure - ConnectionLifecycleTest.ClientConnectionToNonExistentServer - MultiConnectionLifecycleTest.SequentialConnections
Document the re-enabled integration tests in both English and Korean CHANGELOG files under the Fixed section.
Contributor
Performance ComparisonBase Branch ResultsNo base results PR Branch ResultsNo PR results |
Add wait_for_connection_attempt helper to test_helpers.h to ensure async connection operations (resolve/connect) complete before test cleanup. This prevents heap corruption that occurs when: 1. Test initiates async connection to non-existent server 2. Test exits immediately without waiting 3. TearDown calls stop_client() while async operations are in progress 4. Pending handlers access invalidated memory during cleanup Modified tests: - ErrorHandlingTest.ConnectToInvalidHost - ErrorHandlingTest.ConnectToInvalidPort - ErrorHandlingTest.ConnectionRefused - ErrorHandlingTest.RecoveryAfterConnectionFailure - ConnectionLifecycleTest.ClientConnectionToNonExistentServer - MultiConnectionLifecycleTest.SequentialConnections The fix uses error_callback to detect connection failures and waits for either successful connection or error callback before allowing test cleanup to proceed. Fixes heap corruption in CI (Issue #400)
Document the test helper function and test modifications that prevent heap corruption from incomplete async connection operations.
Contributor
Performance ComparisonBase Branch ResultsNo base results PR Branch ResultsNo PR results |
kcenon
added a commit
that referenced
this pull request
Apr 13, 2026
* [TEST] Remove io_context lifecycle CI skips (Issue #400) The intentional leak pattern applied in messaging_client now prevents heap corruption during static destruction. This allows the previously skipped tests to run safely in CI environments. Changes: - Remove TODO comments referencing Issues #315 and #348 - Remove GTEST_SKIP() for io_context lifecycle issues in CI - Update comments to reference Issue #400 and the applied fix - Tests now pass with CI=true environment variable Affected tests: - ErrorHandlingTest.ConnectToInvalidHost - ErrorHandlingTest.ConnectToInvalidPort - ErrorHandlingTest.ConnectionRefused - ErrorHandlingTest.RecoveryAfterConnectionFailure - ConnectionLifecycleTest.ClientConnectionToNonExistentServer - MultiConnectionLifecycleTest.SequentialConnections * docs: update CHANGELOG for io_context lifecycle fix (#400) Document the re-enabled integration tests in both English and Korean CHANGELOG files under the Fixed section. * fix(test): wait for async connection attempts to complete before cleanup Add wait_for_connection_attempt helper to test_helpers.h to ensure async connection operations (resolve/connect) complete before test cleanup. This prevents heap corruption that occurs when: 1. Test initiates async connection to non-existent server 2. Test exits immediately without waiting 3. TearDown calls stop_client() while async operations are in progress 4. Pending handlers access invalidated memory during cleanup Modified tests: - ErrorHandlingTest.ConnectToInvalidHost - ErrorHandlingTest.ConnectToInvalidPort - ErrorHandlingTest.ConnectionRefused - ErrorHandlingTest.RecoveryAfterConnectionFailure - ConnectionLifecycleTest.ClientConnectionToNonExistentServer - MultiConnectionLifecycleTest.SequentialConnections The fix uses error_callback to detect connection failures and waits for either successful connection or error callback before allowing test cleanup to proceed. Fixes heap corruption in CI (Issue #400) * docs: update CHANGELOG with wait_for_connection_attempt helper Document the test helper function and test modifications that prevent heap corruption from incomplete async connection operations.
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.
Summary
Changes
The intentional leak pattern (no-op deleter) already applied to
messaging_client'sio_contextprevents heap corruption during static destruction. This fix removes the CI environment skips that were previously necessary.Test Changes
Removed CI skips from:
ErrorHandlingTest.ConnectToInvalidHostErrorHandlingTest.ConnectToInvalidPortErrorHandlingTest.ConnectionRefusedErrorHandlingTest.RecoveryAfterConnectionFailureConnectionLifecycleTest.ClientConnectionToNonExistentServerMultiConnectionLifecycleTest.SequentialConnectionsDocumentation
Test Results
All tests pass with
CI=trueenvironment variable:Root Cause Analysis
The io_context lifecycle issues were caused by:
The intentional leak pattern applied in messaging_client ensures:
Test plan
CI=trueenvironment variableCloses #400