Skip to content

Commit 7305b35

Browse files
authored
fix(Storage): Addressing issues with race conditions (#2452)
1 parent 12ccd9a commit 7305b35

File tree

5 files changed

+39
-41
lines changed

5 files changed

+39
-41
lines changed

Amplify/Core/Support/AmplifyTask+OperationTaskAdapters.swift

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,14 @@ public class AmplifyInProcessReportingOperationTaskAdapter<Request: AmplifyOpera
6969
public init(operation: AmplifyInProcessReportingOperation<Request, InProcess, Success, Failure>) {
7070
self.operation = operation
7171
self.childTask = ChildTask(parent: operation)
72-
resultToken = operation.subscribe(resultListener: resultListener)
73-
inProcessToken = operation.subscribe(inProcessListener: inProcessListener)
72+
resultToken = operation.subscribe(resultListener: { [weak self] result in
73+
guard let self = self else { return }
74+
self.resultListener(result)
75+
})
76+
inProcessToken = operation.subscribe(inProcessListener: { [weak self] inProcess in
77+
guard let self = self else { return }
78+
self.inProcessListener(inProcess)
79+
})
7480
}
7581

7682
deinit {

Amplify/Core/Support/AtomicDictionary.swift

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,41 +7,43 @@
77

88
import Foundation
99

10-
actor AtomicDictionary<Key: Hashable, Value> {
10+
final class AtomicDictionary<Key: Hashable, Value> {
11+
private let lock: NSLocking
1112
private var value: [Key: Value]
1213

1314
init(initialValue: [Key: Value] = [Key: Value]()) {
15+
self.lock = NSLock()
1416
self.value = initialValue
1517
}
1618

1719
var count: Int {
18-
value.count
20+
lock.execute { value.count }
1921
}
2022

2123
var keys: [Key] {
22-
Array(value.keys)
24+
lock.execute { Array(value.keys) }
2325
}
2426

2527
var values: [Value] {
26-
Array(value.values)
28+
lock.execute { Array(value.values) }
2729
}
2830

2931
// MARK: - Functions
3032

3133
func getValue(forKey key: Key) -> Value? {
32-
value[key]
34+
lock.execute { value[key] }
3335
}
3436

3537
func removeAll() {
36-
value = [:]
38+
lock.execute { value = [:] }
3739
}
3840

3941
@discardableResult
4042
func removeValue(forKey key: Key) -> Value? {
41-
value.removeValue(forKey: key)
43+
return lock.execute { value.removeValue(forKey: key) }
4244
}
4345

4446
func set(value: Value, forKey key: Key) {
45-
self.value[key] = value
47+
lock.execute { self.value[key] = value }
4648
}
4749
}

Amplify/DefaultPlugins/AWSHubPlugin/AWSHubPlugin.swift

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,18 +60,14 @@ final public class AWSHubPlugin: HubCategoryPlugin {
6060
isIncluded filter: HubFilter? = nil,
6161
listener: @escaping HubListener) -> UnsubscribeToken {
6262
let filteredListener = FilteredListener(for: channel, filter: filter, listener: listener)
63-
Task {
64-
await dispatcher.insert(filteredListener)
65-
}
63+
dispatcher.insert(filteredListener)
6664

6765
let unsubscribeToken = UnsubscribeToken(channel: channel, id: filteredListener.id)
6866
return unsubscribeToken
6967
}
7068

7169
public func removeListener(_ token: UnsubscribeToken) {
72-
Task {
73-
await dispatcher.removeListener(withId: token.id)
74-
}
70+
dispatcher.removeListener(withId: token.id)
7571
}
7672

7773
// MARK: - Custom Plugin methods
@@ -80,8 +76,8 @@ final public class AWSHubPlugin: HubCategoryPlugin {
8076
///
8177
/// - Parameter token: The UnsubscribeToken of the listener to check
8278
/// - Returns: True if the dispatcher has a listener registered with `token`
83-
public func hasListener(withToken token: UnsubscribeToken) async -> Bool {
84-
return await dispatcher.hasListener(withId: token.id)
79+
public func hasListener(withToken token: UnsubscribeToken) -> Bool {
80+
return dispatcher.hasListener(withId: token.id)
8581
}
8682

8783
}

Amplify/DefaultPlugins/AWSHubPlugin/Internal/HubChannelDispatcher.swift

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -25,22 +25,22 @@ final class HubChannelDispatcher {
2525
///
2626
/// - Parameter id: The ID of the listener to check
2727
/// - Returns: True if the dispatcher has a listener registered with `id`
28-
func hasListener(withId id: UUID) async -> Bool {
29-
return await listenersById.getValue(forKey: id) != nil
28+
func hasListener(withId id: UUID) -> Bool {
29+
return listenersById.getValue(forKey: id) != nil
3030
}
3131

3232
/// Inserts `listener` into the `listenersById` dictionary by its ID
3333
///
3434
/// - Parameter listener: The listener to add
35-
func insert(_ listener: FilteredListener) async {
36-
await listenersById.set(value: listener, forKey: listener.id)
35+
func insert(_ listener: FilteredListener) {
36+
listenersById.set(value: listener, forKey: listener.id)
3737
}
3838

3939
/// Removes the listener identified by `id` from the `listeners` dictionary
4040
///
4141
/// - Parameter id: The ID of the listener to remove
42-
func removeListener(withId id: UUID) async {
43-
await listenersById.removeValue(forKey: id)
42+
func removeListener(withId id: UUID) {
43+
listenersById.removeValue(forKey: id)
4444
}
4545

4646
/// Dispatches `payload` to all listeners on `channel`
@@ -63,7 +63,7 @@ final class HubChannelDispatcher {
6363
/// after you issue an `await Amplify.reset()`, you may wish to add additional sleep around your code
6464
/// that calls `await Amplify.reset()`.
6565
func destroy() async {
66-
await listenersById.removeAll()
66+
listenersById.removeAll()
6767
messageQueue.cancelAllOperations()
6868
await withCheckedContinuation { continuation in
6969
messageQueue.addBarrierBlock {
@@ -75,16 +75,14 @@ final class HubChannelDispatcher {
7575

7676
extension HubChannelDispatcher: HubDispatchOperationDelegate {
7777
var listeners: [FilteredListener] {
78-
get async {
79-
return Array(await listenersById.values)
80-
}
78+
return Array(listenersById.values)
8179
}
8280
}
8381

8482
protocol HubDispatchOperationDelegate: AnyObject {
8583
/// Used to let a dispatch operation retrieve the list of listeners at the time of invocation, rather than the time
8684
/// of queuing.
87-
var listeners: [FilteredListener] { get async }
85+
var listeners: [FilteredListener] { get }
8886
}
8987

9088
final class HubDispatchOperation: Operation {
@@ -121,14 +119,12 @@ final class HubDispatchOperation: Operation {
121119
return
122120
}
123121

124-
Task {
125-
guard let listeners = await delegate?.listeners else {
126-
return
127-
}
128-
129-
let dispatcher = SerialDispatcher(channel: channel, payload: payload)
130-
dispatcher.dispatch(to: listeners)
122+
guard let listeners = delegate?.listeners else {
123+
return
131124
}
125+
126+
let dispatcher = SerialDispatcher(channel: channel, payload: payload)
127+
dispatcher.dispatch(to: listeners)
132128
}
133129

134130
}

AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Support/Internal/StorageBackgroundEventsRegistryTests.swift

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,10 @@ class StorageBackgroundEventsRegistryTests: XCTestCase {
4040
XCTAssertFalse(otherHandled)
4141
}
4242

43-
Task {
44-
handleEvents(for: identifier)
45-
handleEvents(for: otherIdentifier)
46-
}
47-
4843
await waitForExpectations([done])
44+
45+
handleEvents(for: identifier)
46+
handleEvents(for: otherIdentifier)
4947
}
5048

5149
func testHandlingUnregisteredIdentifier() async throws {

0 commit comments

Comments
 (0)