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

Resolved - Swift Access Races in multi-threading causing memory issues #53

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>IDEDidComputeMac32BitWarning</key>
<true/>
</dict>
</plist>
28 changes: 20 additions & 8 deletions Source/Classes/ActionCableClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ open class ActionCableClient {

//MARK: Socket
fileprivate(set) var socket : WebSocket
fileprivate let queue = DispatchQueue(label: "Custom_Queue")

/// Reconnection Strategy
///
Expand Down Expand Up @@ -419,6 +420,8 @@ extension ActionCableClient {
}

fileprivate func onMessage(_ message: Message) {

queue.sync {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this should use ActionCableSerialQueue ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Imperiopolis

I think sync operation can also be done with ActionCableSerialQueue. Btw, thanks for your thoughts.

switch(message.messageType) {
case .unrecognized:
break
Expand All @@ -429,15 +432,22 @@ extension ActionCableClient {
DispatchQueue.main.async(execute: callback)
}
case .message:

if let channel = channels[message.channelName!] {
// Notify Channel
channel.onMessage(message)

if let callback = onChannelReceive {
DispatchQueue.main.async(execute: { callback(channel, message.data, message.error) } )
DispatchQueue.main.async(execute: { [weak self] in
guard let weakSelf = self else { return }
if weakSelf.onChannelReceive != nil {
callback(channel, message.data, message.error)
}
} )
}
}
case .confirmSubscription:

if let channel = unconfirmedChannels.removeValue(forKey: message.channelName!) {
self.channels.updateValue(channel, forKey: channel.uid)

Expand All @@ -448,6 +458,7 @@ extension ActionCableClient {
DispatchQueue.main.async(execute: { callback(channel) })
}
}

case .rejectSubscription:
// Remove this channel from the list of unconfirmed subscriptions
if let channel = unconfirmedChannels.removeValue(forKey: message.channelName!) {
Expand All @@ -460,13 +471,13 @@ extension ActionCableClient {
}
}
case .hibernateSubscription:
if let channel = channels.removeValue(forKey: message.channelName!) {
// Add channel into unconfirmed channels
unconfirmedChannels[channel.uid] = channel

// We want to treat this like an unsubscribe.
fallthrough
}
if let channel = channels.removeValue(forKey: message.channelName!) {
// Add channel into unconfirmed channels
unconfirmedChannels[channel.uid] = channel
// We want to treat this like an unsubscribe.
fallthrough
}
case .cancelSubscription:
if let channel = channels.removeValue(forKey: message.channelName!) {

Expand All @@ -478,6 +489,7 @@ extension ActionCableClient {
}
}
}
}
}

fileprivate func onData(_ data: Data) {
Expand Down