Skip to content
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

http-conduit: Space leaks when using wide ResourceT #537

Closed
JackKelly-Bellroy opened this issue Jul 5, 2024 · 0 comments · Fixed by #539
Closed

http-conduit: Space leaks when using wide ResourceT #537

JackKelly-Bellroy opened this issue Jul 5, 2024 · 0 comments · Fixed by #539

Comments

@JackKelly-Bellroy
Copy link
Contributor

JackKelly-Bellroy commented Jul 5, 2024

A colleague (@kokobd) did a lot of digging into space leaks while using amazonka, and we think we've chased it down to a problem in http-conduit.

Problem: Responses cannot be garbage collected until execution leaves the surrounding ResourceT, even if the response is closed. It's quite a common idiom to have very large runResourceT blocks, which leads to space leaks until the block is left.

The problem appears to be in http-conduit:Network.HTTP.Conduit.http:

http :: MonadResource m
=> Request
-> Manager
-> m (Response (ConduitM i S.ByteString m ()))
http req man = do
(key, res) <- allocate (Client.responseOpen req man) Client.responseClose
return res { responseBody = do
HCC.bodyReaderSource $ responseBody res
release key
}

When a response body is completely read, it calls release key which closes the connection and removes it from the ResourceT. However, calling http-conduit:Network.HTTP.Conduit.responseClose cleans up the connection itself but does not tell the enclosing ResourceT that the connection has been closed. ResourceT then holds onto the dead Response until the block is left, and it does its cleanup. This is now a no-op because cleanup has already happened, so we've been holding onto the Response for no benefit.

An example of this in the wild: amazonka closes connections in this way to try and release resources as soon as it can; see brendanhay/amazonka#490 for full details.

Possible Solution: Instead of using allocate with responseClose, have Response do a ResourceT-based cleanup and put the real cleanup action only in the ResourceT. I think this works (if exceptions are properly masked):

  1. Call Client.responseOpen to generate the response.
  2. Extract the ResponseClose from the returned response and register it into the ResourceT, returning key.
  3. Replace the responseClose' field of the response with a new ResponseClose $ release key.
  4. (http-conduit already does this, listing for completeness) Attach release key to the responseBody field of the response.

Then exiting ResourceT still cleans up the response, but both an explicit responseClose and consuming the whole body will release the response more promptly.

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 a pull request may close this issue.

1 participant