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

Prevents data races when creating assembles concurrently #40

Merged
merged 6 commits into from
Jan 16, 2025
Merged
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
2 changes: 1 addition & 1 deletion .github/workflows/transloaditkit-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ jobs:
strategy:
matrix:
os: ["macos-latest"]
swift: ["5.10"]
swift: ["6.0.2"]
runs-on: ${{ matrix.os }}
steps:
- name: Extract Branch Name
Expand Down
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,9 @@ playground.xcworkspace
# Package.resolved
.build/
# Add this line if you want to avoid checking in Xcode SPM integration.
# .swiftpm/xcode
.swiftpm/xcode

.vscode/

# CocoaPods
# We recommend against adding the Pods directory to your .gitignore. However
Expand Down
4 changes: 2 additions & 2 deletions Package.resolved
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
"repositoryURL": "https://github.com/tus/TUSKit",
"state": {
"branch": null,
"revision": "03d4421a6165a48f8fc1cda785ff30274e06370c",
"version": "3.3.0"
"revision": "cf691cb712b470088a14e17058c0902f2e2ac2c5",
"version": "3.4.2"
}
}
]
Expand Down
2 changes: 1 addition & 1 deletion Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ let package = Package(
],
dependencies: [
// Dependencies declare other packages that this package depends on.
.package(name: "TUSKit", url: "https://github.com/tus/TUSKit", from: "3.3.0")
.package(name: "TUSKit", url: "https://github.com/tus/TUSKit", from: "3.4.2")
],
targets: [
// Targets are the basic building blocks of a package. A target can define a module or a test suite.
Expand Down
38 changes: 31 additions & 7 deletions Sources/TransloaditKit/Transloadit.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public final class Transloadit {
}

typealias FileId = UUID
var pollers = [[URL]: TransloaditPoller]()
let pollers = TransloaditPollers()

// "private" -- only exposed for unit testing
let api: TransloaditAPI
Expand Down Expand Up @@ -201,10 +201,10 @@ public final class Transloadit {

let poller = TransloaditPoller(transloadit: self, didFinish: { [weak self] in
guard let self = self else { return }
self.pollers[files] = nil
self.pollers.remove(for: files)
})

if let existingPoller = self.pollers[files], existingPoller === poller {
if let existingPoller = self.pollers.get(for: files), existingPoller === poller {
assertionFailure("Transloadit: Somehow already got a poller for this url and these files")
}

Expand Down Expand Up @@ -234,7 +234,8 @@ public final class Transloadit {
}
})

pollers[files] = poller
pollers.register(poller, for: files)

return poller
}

Expand Down Expand Up @@ -273,10 +274,10 @@ public final class Transloadit {

let poller = TransloaditPoller(transloadit: self, didFinish: { [weak self] in
guard let self = self else { return }
self.pollers[files] = nil
self.pollers.remove(for: files)
})

if let existingPoller = self.pollers[files], existingPoller === poller {
if let existingPoller = self.pollers.get(for: files), existingPoller === poller {
assertionFailure("Transloadit: Somehow already got a poller for this url and these files")
}

Expand Down Expand Up @@ -306,7 +307,7 @@ public final class Transloadit {
}
})

pollers[files] = poller
pollers.register(poller, for: files)
return poller
}

Expand Down Expand Up @@ -436,6 +437,29 @@ extension Transloadit: TUSClientDelegate {
}
}

class TransloaditPollers {
private var pollers = [[URL]: TransloaditPoller]()
private var syncQueue = DispatchQueue(label: "com.transloadit.pollers")

func register(_ poller: TransloaditPoller, for files: [URL]) {
syncQueue.sync {
self.pollers[files] = poller
}
}

func remove(for files: [URL]) {
syncQueue.sync {
self.pollers[files] = nil
}
}

func get(for files: [URL]) -> TransloaditPoller? {
return syncQueue.sync {
return self.pollers[files]
}
}
}


/// Small helper function to get an `Assembly` out of a context from TUSKit.
/// - Parameter context: A dictionary that's passed when uploading a file via TUSKit
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ import Foundation

extension TransloaditAPI: URLSessionDataDelegate {
public func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) {
guard let completionHandler = callbacks[task] else {
guard let completionHandler = callbacks.get(for: task) else {
return
}

defer { callbacks[task] = nil }
defer { callbacks.remove(for: task) }

if let error {
completionHandler.callback(.failure(error))
Expand All @@ -22,6 +22,6 @@ extension TransloaditAPI: URLSessionDataDelegate {
}

public func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, didReceive data: Data) {
callbacks[dataTask]?.data.append(data)
callbacks.get(for: dataTask)?.data.append(data)
}
}
40 changes: 31 additions & 9 deletions Sources/TransloaditKit/TransloaditAPI.swift
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ final class TransloaditAPI: NSObject {
}()

private let credentials: Transloadit.Credentials
var callbacks: [URLSessionTask: URLSessionCompletionHandler] = [:]
let callbacks = TransloaditCallbacks()

init(credentials: Transloadit.Credentials, session: URLSession) {
self.credentials = credentials
Expand Down Expand Up @@ -76,7 +76,7 @@ final class TransloaditAPI: NSObject {
}

let task = session.uploadTask(with: request.request, fromFile: request.httpBody)
callbacks[task] = URLSessionCompletionHandler(callback: { result in
callbacks.register(URLSessionCompletionHandler(callback: { result in
switch result {
case .failure(let error):
completion(.failure(.couldNotCreateAssembly(error)))
Expand All @@ -95,7 +95,7 @@ final class TransloaditAPI: NSObject {
completion(.failure(TransloaditAPIError.couldNotCreateAssembly(error)))
}
}
})
}), for: task)
task.resume()
}

Expand All @@ -118,7 +118,7 @@ final class TransloaditAPI: NSObject {
}

let task = session.uploadTask(with: request.request, fromFile: request.httpBody)
callbacks[task] = URLSessionCompletionHandler(callback: { result in
callbacks.register(URLSessionCompletionHandler(callback: { result in
switch result {
case .failure(let error):
completion(.failure(.couldNotCreateAssembly(error)))
Expand All @@ -137,7 +137,7 @@ final class TransloaditAPI: NSObject {
completion(.failure(TransloaditAPIError.couldNotCreateAssembly(error)))
}
}
})
}), for: task)
task.resume()
}

Expand Down Expand Up @@ -304,7 +304,7 @@ final class TransloaditAPI: NSObject {
}

let task = session.dataTask(with: makeRequest())
callbacks[task] = URLSessionCompletionHandler(callback: { result in
callbacks.register(URLSessionCompletionHandler(callback: { result in
switch result {
case .failure:
completion(.failure(.couldNotFetchStatus))
Expand All @@ -318,7 +318,7 @@ final class TransloaditAPI: NSObject {
completion(.failure(.couldNotFetchStatus))
}
}
})
}), for: task)
task.resume()
}

Expand All @@ -330,7 +330,7 @@ final class TransloaditAPI: NSObject {
}

let task = session.dataTask(with: makeRequest())
callbacks[task] = URLSessionCompletionHandler(callback: { result in
callbacks.register(URLSessionCompletionHandler(callback: { result in
switch result {
case .failure:
completion(.failure(.couldNotFetchStatus))
Expand All @@ -344,11 +344,33 @@ final class TransloaditAPI: NSObject {
completion(.failure(.couldNotFetchStatus))
}
}
})
}), for: task)
task.resume()
}
}

class TransloaditCallbacks {
private var callbacks = [URLSessionTask: URLSessionCompletionHandler]()
private let syncQueue = DispatchQueue(label: "com.transloadit.callbacks")

func register(_ callback: URLSessionCompletionHandler, for task: URLSessionTask) {
syncQueue.sync {
callbacks[task] = callback
}
}

func remove(for task: URLSessionTask) {
syncQueue.sync {
callbacks[task] = nil
}
}

func get(for task: URLSessionTask) -> URLSessionCompletionHandler? {
return syncQueue.sync {
return callbacks[task]
}
}
}

extension String {

Expand Down
21 changes: 21 additions & 0 deletions Tests/TransloaditKitTests/TransloaditKitTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,27 @@ class TransloaditKitTests: XCTestCase {
XCTAssertEqual(numFiles, fileDelegate.finishedUploads.count)
XCTAssertEqual(numFiles, fileDelegate.startedUploads.count)
}

func testConcurrentAssemblyCreation() throws {
let expect = expectation(description: "Wait for all assemblies to be created")
expect.expectedFulfillmentCount = 3

DispatchQueue.concurrentPerform(iterations: 3) { _ in
do {
let (files, serverAssembly) = try Network.prepareForUploadingFiles(data: data)
let numFiles = files.count
let _ = createAssembly(files) { _ in
expect.fulfill()
}
} catch {
XCTFail("Failed with error \(error)")
}
}

wait(for: [expect], timeout: 10)

try transloadit.reset()
}

func testCanReset() throws {
XCTAssertEqual(0, transloadit.remainingUploads)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@
"repositoryURL": "https://github.com/ProxymanApp/atlantis",
"state": {
"branch": null,
"revision": "5145a7041ec71421d09653db87dcc80c81792004",
"version": "1.24.0"
"revision": "7246950a6a36454ed1d7830166284f202dc14ad2",
"version": "1.26.0"
}
},
{
"package": "TUSKit",
"repositoryURL": "https://github.com/tus/TUSKit",
"state": {
"branch": null,
"revision": "03d4421a6165a48f8fc1cda785ff30274e06370c",
"version": "3.3.0"
"revision": "cf691cb712b470088a14e17058c0902f2e2ac2c5",
"version": "3.4.2"
}
}
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ final class MyUploader: ObservableObject {
let transloadit: Transloadit

init(backgroundUploader: Bool = false) {
let credentials = Transloadit.Credentials(key: "OsCOAe4ro8CyNsHTp8pdhSiyEzuqwBue", secret: "jB5gZqmkiu2sdSwc7pko8iajD9ailws1eYUtwoKj")
let credentials = Transloadit.Credentials(key: "nOumdyAazaiVnkLDLBiERm0gEmIeeIeW", secret: "FfL2HOBEBxyXIp7zsXZguCMjuaIwhWkVqshF48lB")

if backgroundUploader {
self.transloadit = Transloadit(credentials: credentials, sessionConfiguration: .background(withIdentifier: "com.transloadit.bg_sample"))
Expand Down