Swift: Struct thread safe array crashing with NSLock

Question

I'm trying to implement a thread-safe array component in the most efficient and safe way, backed by unit tests.

So far, I would prefer a struct array, to keep a value type and not a reference type.

But when I run the test below, I still have random crashes that I don't explain :

Here's my ThreadSafe array class :

public struct SafeArray<T>: RangeReplaceableCollection {
    public typealias Element = T
    public typealias Index = Int
    public typealias SubSequence = SafeArray<T>
    public typealias Indices = Range<Int>

    private var array: [T]
    
    private var locker = NSLock()
    private func lock() { locker.lock() }
    private func unlock() { locker.unlock() }

    // MARK: - Public methods

    // MARK: - Initializers
    public init<S>(_ elements: S) where S: Sequence, SafeArray.Element == S.Element {
        array = [S.Element](elements)
    }

    public init() { self.init([]) }

    public init(repeating repeatedValue: SafeArray.Element, count: Int) {
        let array = Array(repeating: repeatedValue, count: count)
        self.init(array)
    }
}

extension SafeArray {
    // Single  action

    public func get() -> [T] {
        lock(); defer { unlock() }
        return Array(array)
    }

    public mutating func set(_ array: [T]) {
        lock(); defer { unlock() }
        self.array = Array(array)
    }
}

And here's my XCUnitTest code :

final class ConcurrencyTests: XCTestCase {
    
    private let concurrentQueue1 = DispatchQueue.init(label: "concurrentQueue1",
                                                      qos: .background,
                                                      attributes: .concurrent,
                                                      autoreleaseFrequency: .inherit,
                                                      target: nil)

    private let concurrentQueue2 = DispatchQueue.init(label: "concurrentQueue2",
                                                      qos: .background,
                                                      attributes: .concurrent,
                                                      autoreleaseFrequency: .inherit,
                                                      target: nil)

    
    private var safeArray = SafeArray(["test"])

    func wait(for expectations: XCTestExpectation, timeout seconds: TimeInterval) {
        wait(for: [expectations], timeout: seconds)
    }

    func waitForMainRunLoop() {
        let mainRunLoopExpectation = expectation(description: "mainRunLoopExpectation")
        DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { mainRunLoopExpectation.fulfill() }
        wait(for: mainRunLoopExpectation, timeout: 0.5)
    }

    func waitFor(_ timeout: TimeInterval) {
        let mainRunLoopExpectation = expectation(description: "timeoutExpectation")
        DispatchQueue.main.asyncAfter(deadline: .now() + timeout) { mainRunLoopExpectation.fulfill() }
        wait(for: mainRunLoopExpectation, timeout: timeout + 0.5)
    }
    
    override func setUpWithError() throws {
        try super.setUpWithError()
        safeArray = SafeArray(["test"])
    }

    func testSafeArrayGet() {
        var thread1: Thread!
        var thread2: Thread!
        
        concurrentQueue1.async {
            thread1 = Thread.current
            let startTime = Date()
            for i in 0...1_000_000 {
                self.safeArray.set(["modification"])
                print("modification \(i)")
            }
            print("time modification: \(Date().timeIntervalSince(startTime))")
        }

        concurrentQueue2.async {
            thread2 = Thread.current
            let startTime = Date()
            for i in 0...1_000_000 {
                let _ = self.safeArray.get()
                print("read \(i)")
            }
            print("time read: \(Date().timeIntervalSince(startTime))")
        }

        waitFor(10)

        XCTAssert(!thread1.isMainThread && !thread2.isMainThread)
        XCTAssert(thread1 != thread2)
    }
}

Edit: Event with a class and a simple approach to make it thread safe, I get a crash. Here's a very simple test that crashes :

class TestClass {
        var test = ["test"]
        let nsLock = NSLock()
        
        func safeSet(_ string: String) {
            nsLock.lock()
            test[0] = string // crash
            nsLock.unlock()
        }
    }
    
    func testStructThreadSafety() {
        let testClass = TestClass()
        
        DispatchQueue.concurrentPerform(iterations: 1_000_000) { i in
            testClass.safeSet("modification \(i)")
            let _ = testClass.test[0]
        }
        
        XCTAssert(true)
    }

Why is it crashing? What am I doing wrong?

Note that if I make it a class I don't get crashes, but I would prefer to keep it a struct.

As an alternative, you could use actor, not class and not worry about all this boilerplate code. — cora, Jun 14 '22 at 15:40
The question was whether he could get thread-safe array with value-type semantics. An actor is a reference type. — Rob, Jun 14 '22 at 17:26
FWIW, synchronized access to value type (such as `struct`) doesn't quite make sense. We use value types so they are copied as needed, avoiding unintended sharing, ensuring thread-safety. Yes, one _can_ access from multiple threads by accessing the value type as property of a reference type, `self` in this example. But this is contrary to the whole point of value types. If one wants to share amongst different threads, you should not use a value type. Or, if you do have a value type property, it is the wrapping reference type’s responsibility to synchronize it, not the value type’s. — Rob, Jun 14 '22 at 17:39
Thanks for your answer @Rob . So if I understand correctly, the only way to accomplish a thread safe wrapper for arrays and dictionaries would be to use a class right ? — Xys, Jun 15 '22 at 09:39
@rob I edited my question with a simple test case at the end. Even with a class container, I get a crash.. — Xys, Jun 22 '22 at 14:21

Swift: Struct thread safe array crashing with NSLock

0 Answers0