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.

Sign up for GitHub

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

Memory leak from HubConnection and HubConnectionConnectionDelegate #297

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Memory leak from HubConnection and HubConnectionConnectionDelegate #297

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
915985c
add some weak refs
kfiliosNovibet Mar 22, 2024
33814bc
fix memory leak between HubConnection and HubConnectionDelegate
kfiliosNovibet Mar 23, 2024
1fbffd7
fix signings
kfiliosNovibet Mar 23, 2024
c74f527
fix test url
kfiliosNovibet Mar 23, 2024
76cdbc9
rollback extra edited files
kfiliosNovibet Mar 26, 2024
77bf9fb
remove DEVELOPMENT_TEAM
kfiliosNovibet Mar 26, 2024
55ef919
remove wrong weak
kfiliosNovibet Mar 29, 2024
398a4a3
more leaks fix, from closure due to are passed by reference and self …
kfiliosNovibet Mar 29, 2024
fe73574
thread leak
kfiliosNovibet Mar 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions Examples/HubSamplePhone/Info.plist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
<string>1</string>
<key>LSRequiresIPhoneOS</key>
<true/>
<key>NSAppTransportSecurity</key>
<dict>
<key>NSAllowsArbitraryLoads</key>
<true/>
</dict>
<key>UILaunchStoryboardName</key>
<string>LaunchScreen</string>
<key>UIMainStoryboardFile</key>
Expand All @@ -41,10 +46,5 @@
<string>UIInterfaceOrientationLandscapeLeft</string>
<string>UIInterfaceOrientationLandscapeRight</string>
</array>
<key>NSAppTransportSecurity</key>
<dict>
<key>NSAllowsArbitraryLoads</key>
<true/>
</dict>
</dict>
</plist>
11 changes: 5 additions & 6 deletions Examples/HubSamplePhone/ViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import SignalRClient

class ViewController: UIViewController, UITableViewDelegate, UITableViewDataSource {
// Update the Url accordingly
private let serverUrl = "http://192.168.86.250:5000/chat" // /chat or /chatLongPolling or /chatWebSockets
private let serverUrl = "http://127.0.0.1:5000/chat" // /chat or /chatLongPolling or /chatWebSockets
private let dispatchQueue = DispatchQueue(label: "hubsamplephone.queue.dispatcheueuq")

private var chatHubConnection: HubConnection?
Expand All @@ -29,20 +29,19 @@ class ViewController: UIViewController, UITableViewDelegate, UITableViewDataSour
self.chatTableView.delegate = self
self.chatTableView.dataSource = self
}

override func viewDidAppear(_ animated: Bool) {
let alert = UIAlertController(title: "Enter your Name", message:"", preferredStyle: UIAlertController.Style.alert)
alert.addTextField() { textField in textField.placeholder = "Name"}
let OKAction = UIAlertAction(title: "OK", style: .default) { action in
self.name = alert.textFields?.first?.text ?? "John Doe"

self.chatHubConnectionDelegate = ChatHubConnectionDelegate(controller: self)
self.chatHubConnection = HubConnectionBuilder(url: URL(string: self.serverUrl)!)
.withLogging(minLogLevel: .debug)
.withAutoReconnect()
.withHubConnectionDelegate(delegate: self.chatHubConnectionDelegate!)
.build()

self.chatHubConnection!.on(method: "NewMessage", callback: {(user: String, message: String) in
self.appendMessage(message: "\(user): \(message)")
})
Expand All @@ -51,11 +50,11 @@ class ViewController: UIViewController, UITableViewDelegate, UITableViewDataSour
alert.addAction(OKAction)
self.present(alert, animated: true)
}

override func viewWillDisappear(_ animated: Bool) {
chatHubConnection?.stop()
}

override func didReceiveMemoryWarning() {
super.didReceiveMemoryWarning()
}
Expand Down
6 changes: 0 additions & 6 deletions Examples/SignalRClient.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@
7391A94722510170007BEF23 /* TransferFormat.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7391A91922510170007BEF23 /* TransferFormat.swift */; };
7391A94822510170007BEF23 /* HubProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7391A91A22510170007BEF23 /* HubProtocol.swift */; };
7391A94922510170007BEF23 /* HubProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7391A91A22510170007BEF23 /* HubProtocol.swift */; };
7391A94A22510170007BEF23 /* Util.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7391A91B22510170007BEF23 /* Util.swift */; };
7391A94B22510170007BEF23 /* Util.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7391A91B22510170007BEF23 /* Util.swift */; };
7391A94E22510170007BEF23 /* JSONHubProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7391A91D22510170007BEF23 /* JSONHubProtocol.swift */; };
7391A94F22510170007BEF23 /* JSONHubProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7391A91D22510170007BEF23 /* JSONHubProtocol.swift */; };
7391A95022510170007BEF23 /* HttpConnection.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7391A91E22510170007BEF23 /* HttpConnection.swift */; };
Expand Down Expand Up @@ -207,7 +205,6 @@
7391A91822510170007BEF23 /* NegotiationResponse.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NegotiationResponse.swift; sourceTree = "<group>"; };
7391A91922510170007BEF23 /* TransferFormat.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TransferFormat.swift; sourceTree = "<group>"; };
7391A91A22510170007BEF23 /* HubProtocol.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = HubProtocol.swift; sourceTree = "<group>"; };
7391A91B22510170007BEF23 /* Util.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Util.swift; sourceTree = "<group>"; };
7391A91D22510170007BEF23 /* JSONHubProtocol.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = JSONHubProtocol.swift; sourceTree = "<group>"; };
7391A91E22510170007BEF23 /* HttpConnection.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = HttpConnection.swift; sourceTree = "<group>"; };
7391A91F22510170007BEF23 /* ConnectionDelegate.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ConnectionDelegate.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -380,7 +377,6 @@
7391A91822510170007BEF23 /* NegotiationResponse.swift */,
7391A91922510170007BEF23 /* TransferFormat.swift */,
7391A91A22510170007BEF23 /* HubProtocol.swift */,
7391A91B22510170007BEF23 /* Util.swift */,
7391A91D22510170007BEF23 /* JSONHubProtocol.swift */,
7391A91E22510170007BEF23 /* HttpConnection.swift */,
7391A91F22510170007BEF23 /* ConnectionDelegate.swift */,
Expand Down Expand Up @@ -629,7 +625,6 @@
7391A92F22510170007BEF23 /* ServerInvocationHandler.swift in Sources */,
7391A94322510170007BEF23 /* HttpClientProtocol.swift in Sources */,
7391A93122510170007BEF23 /* DefaultHttpClient.swift in Sources */,
7391A94B22510170007BEF23 /* Util.swift in Sources */,
7391A93F22510170007BEF23 /* HandshakeProtocol.swift in Sources */,
5ABD6A632439AE6B00BFA93D /* ReconnectableConnection.swift in Sources */,
7391A94722510170007BEF23 /* TransferFormat.swift in Sources */,
Expand Down Expand Up @@ -667,7 +662,6 @@
7391A92E22510170007BEF23 /* ServerInvocationHandler.swift in Sources */,
7391A94222510170007BEF23 /* HttpClientProtocol.swift in Sources */,
7391A93022510170007BEF23 /* DefaultHttpClient.swift in Sources */,
7391A94A22510170007BEF23 /* Util.swift in Sources */,
7391A93E22510170007BEF23 /* HandshakeProtocol.swift in Sources */,
5ABD6A622439AE6B00BFA93D /* ReconnectableConnection.swift in Sources */,
7391A94622510170007BEF23 /* TransferFormat.swift in Sources */,
Expand Down
29 changes: 20 additions & 9 deletions Sources/SignalRClient/HubConnection.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ public class HubConnection {
// TODO: add negative test (e.g. invalid protocol)
let handshakeRequest = HandshakeProtocol.createHandshakeRequest(hubProtocol: hubProtocol)
logger.log(logLevel: .debug, message: "Sending handshake request: \(handshakeRequest)")
connection.send(data: "\(handshakeRequest)".data(using: .utf8)!) { error in
connection.send(data: "\(handshakeRequest)".data(using: .utf8)!) { [weak self] error in
guard let self else { return }
if let e = error {
self.logger.log(logLevel: .error, message: "Sending handshake request failed: \(e)")
// TODO: (BUG) if this fails when reconnecting the callback should not be called and there
Expand Down Expand Up @@ -156,7 +157,8 @@ public class HubConnection {
let invocationData = try hubProtocol.writeMessage(message: invocationMessage)
resetKeepAlive()

connection.send(data: invocationData, sendDidComplete: { error in
connection.send(data: invocationData, sendDidComplete: { [weak self] error in
guard let self else { return }
if error == nil {
self.resetKeepAlive()
}
Expand Down Expand Up @@ -281,7 +283,8 @@ public class HubConnection {
let cancelInvocationMessage = CancelInvocationMessage(invocationId: streamHandle.invocationId)
do {
let cancelInvocationData = try hubProtocol.writeMessage(message: cancelInvocationMessage)
connection.send(data: cancelInvocationData, sendDidComplete: {error in
connection.send(data: cancelInvocationData, sendDidComplete: { [weak self] error in
guard let self else { return }
if let e = error {
self.logger.log(logLevel: .error, message: "Sending cancellation of server side streaming hub returned error: \(e)")
self.callbackQueue.async {
Expand Down Expand Up @@ -312,7 +315,8 @@ public class HubConnection {
let invocationMessage = invocationHandler.createInvocationMessage(invocationId: id, method: method, arguments: arguments, streamIds: [])
let invocationData = try hubProtocol.writeMessage(message: invocationMessage)

connection.send(data: invocationData) { error in
connection.send(data: invocationData) { [weak self] error in
guard let self else { return }
if let e = error {
self.logger.log(logLevel: .error, message: "Invoking server hub method \(method) returned error: \(e)")
self.failInvocationWithError(invocationHandler: invocationHandler, invocationId: id, error: e)
Expand Down Expand Up @@ -364,17 +368,20 @@ public class HubConnection {
// TODO: (BUG) if this fails when reconnecting the callback should not be called and there
// will be no further reconnect attempts
logger.log(logLevel: .error, message: "Parsing handshake response failed: \(e)")
callbackQueue.async {
callbackQueue.async { [weak self] in
guard let self else { return }
self.delegate?.connectionDidFailToOpen(error: e)
}
return
}
if originalHandshakeStatus.isReconnect {
callbackQueue.async {
callbackQueue.async { [weak self] in
guard let self else { return }
self.delegate?.connectionDidReconnect()
}
} else {
callbackQueue.async {
callbackQueue.async { [weak self] in
guard let self else { return }
self.delegate?.connectionDidOpen(hubConnection: self)
}
resetKeepAlive()
Expand Down Expand Up @@ -515,7 +522,10 @@ public class HubConnection {
}
logger.log(logLevel: .debug, message: "Resetting keep alive")
keepAlivePingTask!.cancel()
keepAlivePingTask = DispatchWorkItem { self.sendKeepAlivePing() }
keepAlivePingTask = DispatchWorkItem { [weak self] in
guard let self else { return }
self.sendKeepAlivePing()
}
hubConnectionQueue.asyncAfter(deadline: DispatchTime.now() + keepAliveInterval, execute: keepAlivePingTask!)
}
}
Expand All @@ -531,7 +541,8 @@ public class HubConnection {
do {
let cachedPingMessage = try hubProtocol.writeMessage(message: PingMessage.instance)
logger.log(logLevel: .debug, message: "Sending keep alive")
connection.send(data: cachedPingMessage, sendDidComplete: { error in
connection.send(data: cachedPingMessage, sendDidComplete: { [weak self] error in
guard let self else { return }
if let error = error {
self.logger.log(logLevel: .error, message: "Keep alive send error: \(error.localizedDescription)")
} else {
Expand Down
2 changes: 1 addition & 1 deletion Sources/SignalRClient/HubConnectionBuilder.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public class HubConnectionBuilder {
private let httpConnectionOptions = HttpConnectionOptions()
private let hubConnectionOptions = HubConnectionOptions()
private var logger: Logger = NullLogger()
private var delegate: HubConnectionDelegate?
private weak var delegate: HubConnectionDelegate?
private var reconnectPolicy: ReconnectPolicy = NoReconnectPolicy()
private var permittedTransportTypes: TransportType = .all
private var transportFactory: ((Logger, TransportType) -> TransportFactory) =
Expand Down
14 changes: 10 additions & 4 deletions Sources/SignalRClient/LongPollingTransport.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import Foundation

public class LongPollingTransport: Transport {

public var delegate: TransportDelegate?
public weak var delegate: TransportDelegate?

private let logger: Logger
private let closeQueue = DispatchQueue(label: "LongPollingTransportCloseQueue")
Expand Down Expand Up @@ -44,6 +44,7 @@ public class LongPollingTransport: Transport {
return
}
httpClient.post(url: url, body: data) { (responseOptional, errorOptional) in
//If for any reason you need self instance please added as weak to prevent ARC count up and lead to memory leak
if let error = errorOptional {
sendDidComplete(error)
} else if let response = responseOptional {
Expand All @@ -57,12 +58,14 @@ public class LongPollingTransport: Transport {
}

public func close() {
closeQueue.sync {
closeQueue.sync { [weak self] in
guard let self else { return }
if !closeCalled {
closeCalled = true
active = false
self.logger.log(logLevel: .debug, message: "Sending LongPolling session DELETE request...")
self.httpClient?.delete(url: self.url!, completionHandler: { (_, errorOptional) in
self.httpClient?.delete(url: self.url!, completionHandler: { [weak self] (_, errorOptional) in
guard let self else { return }
if let error = errorOptional {
self.logger.log(logLevel: .error, message: "Error while DELETE-ing long polling session: \(error)")
self.delegate?.transportDidClose(error)
Expand All @@ -81,7 +84,10 @@ public class LongPollingTransport: Transport {
if self.active {
let pollUrl = self.getPollUrl()
self.logger.log(logLevel: .debug, message: "Polling \(pollUrl)")
self.httpClient?.get(url: pollUrl, completionHandler: self.handlePollResponse(response:error:))
self.httpClient?.get(url: pollUrl, completionHandler: { [weak self] response,error in
guard let self else { return }
self.handlePollResponse(response: response, error: error)
})
} else {
self.logger.log(logLevel: .debug, message: "Long Polling transport polling complete.")
self.close()
Expand Down
2 changes: 1 addition & 1 deletion Sources/SignalRClient/WebsocketsTransport.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public class WebsocketsTransport: NSObject, Transport, URLSessionWebSocketDelega

private var isTransportClosed = false

public var delegate: TransportDelegate?
public weak var delegate: TransportDelegate?
public let inherentKeepAlive = false

init(logger: Logger) {
Expand Down
6 changes: 6 additions & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.