-
Notifications
You must be signed in to change notification settings - Fork 116
-
Notifications
You must be signed in to change notification settings - Fork 116
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
OOM if setting concurrentHTTP1ConnectionsPerHostSoftLimit: .max
#751
Labels
good first issue
Good for newcomers
Comments
fabianfett
pushed a commit
that referenced
this issue
Aug 14, 2024
…t` to `Int.max` (#763) ### Motivation: When a user wishes to make the connection pool create as many concurrent connections as possible, a natural way to achieve this would be to set `.max` to the `concurrentHTTP1ConnectionsPerHostSoftLimit` property. ```swift HTTPClient.Configuration().connectionPool = .init( idleTimeout: .hours(1), concurrentHTTP1ConnectionsPerHostSoftLimit: .max ) ``` The `concurrentHTTP1ConnectionsPerHostSoftLimit` property is of type `Int`. Setting it to `Int.max` leads to `Int.max` being passed as an argument to `Array`s `.reserveCapacity(_:)` method, causing an OOM issue. Addresses Github Issue #751 ### Modifications: Capped the argument to `self.connections.reserveCapacity(_:)` to 1024 in `HTTPConnectionPool.HTTP1Connections` ### Result: Users can now set the `concurrentHTTP1ConnectionsPerHostSoftLimit` property to `.max` without causing an OOM issue.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I would like to make the pool create as many concurrent connections as possible. So I thought the best way of spelling this is
unfortunately, the
concurrentHTTP1ConnectionsPerHostSoftLimit: .max
makes AHC OOM even before it creates any connections becauseHTTPConnectionPool.HTTP1Connections.init()
preallocates an array of that size.It shouldn't do that or at least cap it to something reasonable like 1024.
The text was updated successfully, but these errors were encountered: