Skip to content

Commit 521e9ba

Browse files
authored
fix(API): DataRace - subscription cancel and OperationTaskMapper on reset (#1684)
1 parent dfbf18d commit 521e9ba

File tree

6 files changed

+44
-30
lines changed

6 files changed

+44
-30
lines changed

AmplifyPlugins/API/APICategoryPlugin.xcodeproj/xcshareddata/xcschemes/AWSAPICategoryPlugin.xcscheme

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@
9090
buildConfiguration = "Debug"
9191
selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
9292
selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
93+
enableThreadSanitizer = "YES"
9394
launchStyle = "0"
9495
useCustomWorkingDirectory = "NO"
9596
ignoresPersistentStateOnLaunch = "NO"

AmplifyPlugins/API/AWSAPICategoryPlugin/AWSAPIPlugin+Resettable.swift

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@ extension AWSAPIPlugin: Resettable {
1313
public func reset(onComplete: @escaping BasicClosure) {
1414
mapper.reset()
1515

16-
mapper = OperationTaskMapper()
17-
1816
let waitForReset = DispatchSemaphore(value: 0)
1917
session.reset { waitForReset.signal() }
2018
waitForReset.wait()

AmplifyPlugins/API/AWSAPICategoryPlugin/Operation/AWSGraphQLSubscriptionOperation.swift

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ final public class AWSGraphQLSubscriptionOperation<R: Decodable>: GraphQLSubscri
2121
var subscriptionItem: SubscriptionItem?
2222
var apiAuthProviderFactory: APIAuthProviderFactory
2323

24+
private let subscriptionQueue = DispatchQueue(label: "AWSGraphQLSubscriptionOperation.subscriptionQueue")
25+
2426
init(request: GraphQLOperationRequest<R>,
2527
pluginConfig: AWSAPICategoryPluginConfiguration,
2628
subscriptionConnectionFactory: SubscriptionConnectionFactory,
@@ -42,11 +44,14 @@ final public class AWSGraphQLSubscriptionOperation<R: Decodable>: GraphQLSubscri
4244
}
4345

4446
override public func cancel() {
45-
if let subscriptionItem = subscriptionItem, let subscriptionConnection = subscriptionConnection {
46-
subscriptionConnection.unsubscribe(item: subscriptionItem)
47-
let subscriptionEvent = SubscriptionEvent<GraphQLResponse<R>>.connection(.disconnected)
48-
dispatchInProcess(data: subscriptionEvent)
47+
subscriptionQueue.sync {
48+
if let subscriptionItem = subscriptionItem, let subscriptionConnection = subscriptionConnection {
49+
subscriptionConnection.unsubscribe(item: subscriptionItem)
50+
let subscriptionEvent = SubscriptionEvent<GraphQLResponse<R>>.connection(.disconnected)
51+
dispatchInProcess(data: subscriptionEvent)
52+
}
4953
}
54+
5055
super.cancel()
5156
dispatch(result: .successfulVoid)
5257
finish()
@@ -90,27 +95,28 @@ final public class AWSGraphQLSubscriptionOperation<R: Decodable>: GraphQLSubscri
9095
let pluginOptions = request.options.pluginOptions as? AWSPluginOptions
9196

9297
// Retrieve the subscription connection
93-
do {
94-
subscriptionConnection = try subscriptionConnectionFactory
95-
.getOrCreateConnection(for: endpointConfig,
96-
authService: authService,
97-
authType: pluginOptions?.authType,
98-
apiAuthProviderFactory: apiAuthProviderFactory)
99-
} catch {
100-
let error = APIError.operationError("Unable to get connection for api \(endpointConfig.name)", "", error)
101-
dispatch(result: .failure(error))
102-
finish()
103-
return
98+
subscriptionQueue.sync {
99+
do {
100+
subscriptionConnection = try subscriptionConnectionFactory
101+
.getOrCreateConnection(for: endpointConfig,
102+
authService: authService,
103+
authType: pluginOptions?.authType,
104+
apiAuthProviderFactory: apiAuthProviderFactory)
105+
} catch {
106+
let error = APIError.operationError("Unable to get connection for api \(endpointConfig.name)", "", error)
107+
dispatch(result: .failure(error))
108+
finish()
109+
return
110+
}
111+
112+
// Create subscription
113+
114+
subscriptionItem = subscriptionConnection?.subscribe(requestString: request.document,
115+
variables: request.variables,
116+
eventHandler: { [weak self] event, _ in
117+
self?.onAsyncSubscriptionEvent(event: event)
118+
})
104119
}
105-
106-
// Create subscription
107-
108-
subscriptionItem = subscriptionConnection?.subscribe(requestString: request.document,
109-
variables: request.variables,
110-
eventHandler: { [weak self] event, _ in
111-
self?.onAsyncSubscriptionEvent(event: event)
112-
})
113-
114120
}
115121

116122
private func onAsyncSubscriptionEvent(event: SubscriptionItemEvent) {

AmplifyPlugins/API/AWSAPICategoryPlugin/URLSessionBehavior/OperationTaskMapper.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ class OperationTaskMapper {
6969
OperationTaskMapper.concurrencyQueue.sync {
7070
operations.values.forEach { $0.cancelOperation() }
7171
tasks.values.forEach { $0.cancel() }
72+
operations = [:]
73+
tasks = [:]
74+
operationIdsByTaskId = [:]
75+
taskIdsByOperationId = [:]
7276
}
7377
}
7478

AmplifyPlugins/API/AWSAPICategoryPluginTests/Operation/AWSGraphQLSubscriptionOperationCancelTests.swift

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,13 @@ class AWSGraphQLSubscriptionOperationCancelTests: XCTestCase {
170170
let connectionCreation = expectation(description: "connection factory called")
171171
let mockSubscriptionConnectionFactory = MockSubscriptionConnectionFactory(onGetOrCreateConnection: { _, _, _, _ in
172172
connectionCreation.fulfill()
173-
sleep(5)
174-
throw APIError.invalidConfiguration("something went wrong", "", nil)
173+
return MockSubscriptionConnection(onSubscribe: { (_, _, eventHandler) -> SubscriptionItem in
174+
let item = SubscriptionItem(requestString: "", variables: nil, eventHandler: { _, _ in
175+
})
176+
eventHandler(.connection(.connecting), item)
177+
return item
178+
}, onUnsubscribe: {_ in
179+
})
175180
})
176181

177182
setUp(subscriptionConnectionFactory: mockSubscriptionConnectionFactory)
@@ -184,7 +189,7 @@ class AWSGraphQLSubscriptionOperationCancelTests: XCTestCase {
184189
let receivedFailure = expectation(description: "Received failure")
185190
receivedFailure.isInverted = true
186191
let receivedValue = expectation(description: "Received value for connecting")
187-
receivedValue.isInverted = true
192+
receivedValue.assertForOverFulfill = false
188193

189194
let valueListener: GraphQLSubscriptionOperation<JSONValue>.InProcessListener = { _ in
190195
receivedValue.fulfill()

AmplifyPlugins/API/Podfile.lock

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,4 +116,4 @@ SPEC CHECKSUMS:
116116

117117
PODFILE CHECKSUM: 54e87158e45936fe60756d4a417b89eb64ea1c51
118118

119-
COCOAPODS: 1.10.1
119+
COCOAPODS: 1.11.2

0 commit comments

Comments
 (0)