From ff35359afccc5348f824cd3e1cd531c7be1a0648 Mon Sep 17 00:00:00 2001 From: Michael Rebello Date: Thu, 9 Jan 2025 09:51:12 -0800 Subject: [PATCH 1/2] fix cycle Signed-off-by: Michael Rebello --- .../AppSources/MenuView.swift | 14 ++-- .../Clients/URLSessionHTTPClient.swift | 74 +++++++++++++++++-- 2 files changed, 73 insertions(+), 15 deletions(-) diff --git a/Examples/ElizaSharedSources/AppSources/MenuView.swift b/Examples/ElizaSharedSources/AppSources/MenuView.swift index c9efb390..64b7fcf4 100644 --- a/Examples/ElizaSharedSources/AppSources/MenuView.swift +++ b/Examples/ElizaSharedSources/AppSources/MenuView.swift @@ -38,7 +38,7 @@ extension MessagingConnectionType: Identifiable { } struct MenuView: View { - private func createClient(withProtocol networkProtocol: NetworkProtocol) + private static func createClient(withProtocol networkProtocol: NetworkProtocol) -> Connectrpc_Eliza_V1_ElizaServiceClient { let host = "https://demo.connectrpc.com" @@ -87,7 +87,7 @@ struct MenuView: View { destination: LazyNavigationView { MessagingView( viewModel: UnaryMessagingViewModel( - client: self.createClient(withProtocol: .connect) + client: Self.createClient(withProtocol: .connect) ) ) } @@ -100,7 +100,7 @@ struct MenuView: View { destination: LazyNavigationView { MessagingView( viewModel: BidirectionalStreamingMessagingViewModel( - client: self.createClient(withProtocol: .connect) + client: Self.createClient(withProtocol: .connect) ) ) } @@ -114,7 +114,7 @@ struct MenuView: View { destination: LazyNavigationView { MessagingView( viewModel: UnaryMessagingViewModel( - client: self.createClient(withProtocol: .grpc) + client: Self.createClient(withProtocol: .grpc) ) ) } @@ -128,7 +128,7 @@ struct MenuView: View { destination: LazyNavigationView { MessagingView( viewModel: UnaryMessagingViewModel( - client: self.createClient(withProtocol: .grpcWeb) + client: Self.createClient(withProtocol: .grpcWeb) ) ) } @@ -142,7 +142,7 @@ struct MenuView: View { destination: LazyNavigationView { MessagingView( viewModel: BidirectionalStreamingMessagingViewModel( - client: self.createClient(withProtocol: .grpc) + client: Self.createClient(withProtocol: .grpc) ) ) } @@ -156,7 +156,7 @@ struct MenuView: View { destination: LazyNavigationView { MessagingView( viewModel: BidirectionalStreamingMessagingViewModel( - client: self.createClient(withProtocol: .grpcWeb) + client: Self.createClient(withProtocol: .grpcWeb) ) ) } diff --git a/Libraries/Connect/Public/Implementation/Clients/URLSessionHTTPClient.swift b/Libraries/Connect/Public/Implementation/Clients/URLSessionHTTPClient.swift index 3d4ef113..60c1334c 100644 --- a/Libraries/Connect/Public/Implementation/Clients/URLSessionHTTPClient.swift +++ b/Libraries/Connect/Public/Implementation/Clients/URLSessionHTTPClient.swift @@ -25,8 +25,8 @@ open class URLSessionHTTPClient: NSObject, HTTPClientInterface, @unchecked Senda private let lock = Lock() /// Closures stored for notifying when metrics are available. private var metricsClosures = [Int: @Sendable (HTTPMetrics) -> Void]() - /// Force unwrapped to allow using `self` as the delegate. - private var session: URLSession! + /// Session used for performing requests. + private let session: URLSession /// List of active streams. /// TODO: Remove in favor of simply setting /// `URLSessionTask.delegate = ` once we are able to set iOS 15 @@ -34,12 +34,18 @@ open class URLSessionHTTPClient: NSObject, HTTPClientInterface, @unchecked Senda private var streams = [Int: URLSessionStream]() public init(configuration: URLSessionConfiguration = .default) { - super.init() + let delegate = URLSessionDelegateWrapper() self.session = URLSession( configuration: configuration, - delegate: self, + delegate: delegate, delegateQueue: .main ) + super.init() + delegate.client = self + } + + deinit { + self.session.finishTasksAndInvalidate() } @discardableResult @@ -124,9 +130,9 @@ open class URLSessionHTTPClient: NSObject, HTTPClientInterface, @unchecked Senda sendClose: { urlSessionStream.close() } ) } -} -extension URLSessionHTTPClient: URLSessionDataDelegate { + // MARK: - URLSession delegate functions + open func urlSession( _ session: URLSession, dataTask: URLSessionDataTask, didReceive response: URLResponse, completionHandler: @escaping (URLSession.ResponseDisposition) -> Void @@ -156,9 +162,7 @@ extension URLSessionHTTPClient: URLSessionDataDelegate { self.lock.perform { self.streams[task.taskIdentifier]?.requestBodyStream } ) } -} -extension URLSessionHTTPClient: URLSessionTaskDelegate { open func urlSession( _ session: URLSession, task: URLSessionTask, didCompleteWithError error: Swift.Error? ) { @@ -178,6 +182,60 @@ extension URLSessionHTTPClient: URLSessionTaskDelegate { } } +// MARK: - URLSession delegate wrapper + +/// 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. +private final class URLSessionDelegateWrapper: NSObject, @unchecked Sendable { + weak var client: URLSessionHTTPClient? +} + +extension URLSessionDelegateWrapper: URLSessionDataDelegate { + func urlSession( + _ session: URLSession, dataTask: URLSessionDataTask, didReceive response: URLResponse, + completionHandler: @escaping (URLSession.ResponseDisposition) -> Void + ) { + client?.urlSession( + session, dataTask: dataTask, didReceive: response, completionHandler: completionHandler + ) + } + + func urlSession( + _ session: URLSession, dataTask: URLSessionDataTask, didReceive data: Data + ) { + client?.urlSession(session, dataTask: dataTask, didReceive: data) + } + + func urlSession( + _ session: URLSession, task: URLSessionTask, + needNewBodyStream completionHandler: @escaping (InputStream?) -> Void + ) { + client?.urlSession(session, task: task, needNewBodyStream: completionHandler) + } +} + +extension URLSessionDelegateWrapper: URLSessionTaskDelegate { + func urlSession( + _ session: URLSession, task: URLSessionTask, didCompleteWithError error: Swift.Error? + ) { + client?.urlSession(session, task: task, didCompleteWithError: error) + } + + func urlSession( + _ session: URLSession, task: URLSessionTask, + didFinishCollecting metrics: URLSessionTaskMetrics + ) { + client?.urlSession(session, task: task, didFinishCollecting: metrics) + } +} + +// MARK: - Extensions + extension HTTPURLResponse { func formattedLowercasedHeaders() -> Headers { return self.allHeaderFields.reduce(into: Headers()) { headers, current in From 8043d87aa04f1e90c084fdd7bcbdd6dd5add962b Mon Sep 17 00:00:00 2001 From: Michael Rebello Date: Thu, 9 Jan 2025 10:01:54 -0800 Subject: [PATCH 2/2] explicit `self` Signed-off-by: Michael Rebello --- .../Implementation/Clients/URLSessionHTTPClient.swift | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Libraries/Connect/Public/Implementation/Clients/URLSessionHTTPClient.swift b/Libraries/Connect/Public/Implementation/Clients/URLSessionHTTPClient.swift index 60c1334c..059d7fc3 100644 --- a/Libraries/Connect/Public/Implementation/Clients/URLSessionHTTPClient.swift +++ b/Libraries/Connect/Public/Implementation/Clients/URLSessionHTTPClient.swift @@ -200,7 +200,7 @@ extension URLSessionDelegateWrapper: URLSessionDataDelegate { _ session: URLSession, dataTask: URLSessionDataTask, didReceive response: URLResponse, completionHandler: @escaping (URLSession.ResponseDisposition) -> Void ) { - client?.urlSession( + self.client?.urlSession( session, dataTask: dataTask, didReceive: response, completionHandler: completionHandler ) } @@ -208,14 +208,14 @@ extension URLSessionDelegateWrapper: URLSessionDataDelegate { func urlSession( _ session: URLSession, dataTask: URLSessionDataTask, didReceive data: Data ) { - client?.urlSession(session, dataTask: dataTask, didReceive: data) + self.client?.urlSession(session, dataTask: dataTask, didReceive: data) } func urlSession( _ session: URLSession, task: URLSessionTask, needNewBodyStream completionHandler: @escaping (InputStream?) -> Void ) { - client?.urlSession(session, task: task, needNewBodyStream: completionHandler) + self.client?.urlSession(session, task: task, needNewBodyStream: completionHandler) } } @@ -223,14 +223,14 @@ extension URLSessionDelegateWrapper: URLSessionTaskDelegate { func urlSession( _ session: URLSession, task: URLSessionTask, didCompleteWithError error: Swift.Error? ) { - client?.urlSession(session, task: task, didCompleteWithError: error) + self.client?.urlSession(session, task: task, didCompleteWithError: error) } func urlSession( _ session: URLSession, task: URLSessionTask, didFinishCollecting metrics: URLSessionTaskMetrics ) { - client?.urlSession(session, task: task, didFinishCollecting: metrics) + self.client?.urlSession(session, task: task, didFinishCollecting: metrics) } }