Skip to content

fix(http2_adapter): fix inconsistent cache key format causing memory leak#2481

Merged
AlexV525 merged 3 commits intocfug:mainfrom
kangqiao:fix/http2-adapter-cache-key-inconsistency
Feb 2, 2026
Merged

fix(http2_adapter): fix inconsistent cache key format causing memory leak#2481
AlexV525 merged 3 commits intocfug:mainfrom
kangqiao:fix/http2-adapter-cache-key-inconsistency

Conversation

@kangqiao
Copy link
Contributor

Description

Fixes inconsistent cache key format in _ConnectionManager that causes memory leak.

Problem

In connection_manager_imp.dart, the cache key format is inconsistent:

Location Variable Format Example
Line 50 getConnection transportCacheKey scheme://host:port https://example.com:443
Line 91 _connect domain host:port example.com:443

When idle timeout fires (line 146), _transportsMap.remove(domain) fails to find the key, causing entries to never be removed.

Impact

  • Memory leak: _transportsMap entries are never cleaned up
  • Map pollution over long-running applications

Solution

Changed domain format to match transportCacheKey:

// Before
final domain = '${uri.host}:${uri.port}';

// After
final domain = '${uri.scheme}://${uri.host}:${uri.port}';

…anager

The cache key format was inconsistent between getConnection() and _connect(),
causing _transportsMap.remove() to fail silently during idle timeout cleanup.

- getConnection uses: '${uri.scheme}://${uri.host}:${uri.port}'
- _connect was using: '${uri.host}:${uri.port}'

This led to memory leaks as entries were never removed from _transportsMap.
@kangqiao kangqiao requested a review from a team as a code owner January 27, 2026 07:36
@AlexV525
Copy link
Member

@kangqiao Good catch! Would you add a test case regarding the fix?

- Add `cachedConnectionsCount` getter to ConnectionManager for testing
- Extract `_getCacheKey` method to ensure consistent cache key format
- Add test case to verify connections are properly removed after idle timeout

This test validates the fix from cfug#2481 where inconsistent cache key format
caused _transportsMap.remove() to fail silently, leading to memory leaks.
@github-actions
Copy link
Contributor

github-actions bot commented Feb 2, 2026

Code Coverage Report: Only Changed Files listed

Package Base Coverage New Coverage Difference
plugins/http2_adapter/lib/src/connection_manager_imp.dart 🟢 78.83% 🟢 82.14% 🟢 3.31%
Overall Coverage 🟢 88.1% 🟢 88.33% 🟢 0.23%

Minimum allowed coverage is 0%, this run produced 88.33%

Copy link
Member

@AlexV525 AlexV525 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AlexV525 AlexV525 merged commit 0e2185b into cfug:main Feb 2, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants