Skip to content

Fix sendability issues in the connection pool #833

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 28, 2025

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Apr 28, 2025

Motivation:

The connection pool holds much of the low level logic in AHC. We should fix its sendability issues before moving to higher levels.

Modifications:

  • Make HTTP1ConnectionDelegate and HTTP2Delegate sendable, this requires passing IDs rather than connections to their methods
  • Make HTTPConnectionRequester sendable and have its methods take Sendable views of the HTTP1Connection and HTTP2Connection types
  • Add sendable views to HTTP1Connection and HTTP2Connection
  • Mark HTTP1Connection and HTTP2Connection as not sendable
  • Make HTTPRequestExecutor and HTTPExecutableRequest sendable
  • Update tests

Result:

Connection pool has stricter sendability requirements

Motivation:

The connection pool holds much of the low level logic in AHC. We should
fix its sendability issues before moving to higher levels.

Modifications:

- Make HTTP1ConnectionDelegate and HTTP2Delegate sendable, this requires
  passing IDs rather than connections to their methods
- Make HTTPConnectionRequester sendable and have its methods take
  Sendable views of the HTTP1Connection and HTTP2Connection types
- Make HTTPRequestExecutor and HTTPExecutableRequest sendable
- Update tests

Result:

Connection pool has stricter sendability requirements
@glbrntt glbrntt added the 🔨 semver/patch No public API change. label Apr 28, 2025
@glbrntt glbrntt enabled auto-merge (squash) April 28, 2025 14:12
@glbrntt glbrntt merged commit 086524f into swift-server:main Apr 28, 2025
23 of 24 checks passed
@glbrntt glbrntt deleted the strict-connection-pool branch April 29, 2025 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants