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

Fix retain cycle between URLSessionHTTPClient and URLSession.delegate #335

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

rebello95
Copy link
Collaborator

@rebello95 rebello95 commented Jan 9, 2025

See #334 for more information on the underlying issue.

This PR creates a private URLSessionDelegateWrapper class to act as an intermediary between URLSessionHTTPClient and URLSession.delegate. From the inline documentation I added:

/// This class exists to avoid a retain cycle between `URLSessionHTTPClient` and its underlying
/// `URLSession.delegate`. Since `URLSession` retains its `delegate` strongly, setting the
/// `URLSessionHTTPClient` directly as the `delegate` will cause a retain cycle:
/// https://developer.apple.com/documentation/foundation/urlsession/1411597-init#parameters
///
/// To work around this, `URLSessionDelegateWrapper` maintains a `weak` reference to the
/// `URLSessionHTTPClient` and passes delegate calls through to it, avoiding the retain cycle.

The client now also calls self.session.finishTasksAndInvalidate() on deinit.

This workaround isn't the cleanest, but it does solve the issue and I'm not sure there's a better way of doing it. Open to suggestions, though.

I also made the builder function in the example app static to avoid capturing self in the builder closures.

With these changes, I was able to confirm the cycle demonstrated in the above issue is resolved in the memory debugger:

Screenshot 2025-01-09 at 9 56 03 AM

Note that I avoided changing the open func signatures of URLSessionHTTPClient since that would be a breaking API change.

Resolves #334.

Signed-off-by: Michael Rebello <[email protected]>
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.

Circular reference between URLSessionHTTPClient and URLSession
1 participant