Skip to content

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

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

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

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
434ef41
Added sync queue to operate for confirm subscription onMessage issue …
Oct 30, 2018
d6fce26
Swift Race Access while getting socket messages in multi-threading is…
Nov 5, 2018
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
8 changes: 8 additions & 0 deletions ActionCableClient.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist
View file
Open in desktop
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
View file
Open in desktop
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 {
Copy link

@Imperiopolis Imperiopolis Jan 2, 2019

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

@fahidattique55 fahidattique55 Jan 3, 2019

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