Confusing EXC_BAD_ACCESS with simple Combine Future usage

I'm trying to understand the behavior I observe when running this test (see GitHub repo for Xcode project):

import XCTest
import Combine

class Item {
    deinit {
        // Randomly called by Future.deinit
        // How does this instance's retain count go to zero?
        // Shouldn't the Store retain this, preventing it from being freed?
        // The Store itself is never deinitialized
        print("item deinit")
    }
}

class Store {
    static let shared = Store()
    private let item: Item = Item()
    private let queue = DispatchQueue(label: "Store.queue")
    
    deinit {
        // Let's rule out the Store instance being deinitialized
        fatalError("store deinit")
    }
    
    func fetchItem(completion: @escaping (Item) -> Void) {
        queue.async {
            completion(self.item)
        }
    }
}

final class BadAccessPlaygroundTests: XCTestCase {
    func testBadAccess() {
        let expectation = expectation(description: "expectation")
        var task: AnyCancellable?
        
        task = Future<Item, Never> { promise in
            Store.shared.fetchItem() { result in
                promise(.success(result))
            }
        }
            .sink(
                receiveCompletion: { _ in
                    expectation.fulfill()
                    task?.cancel()
                }, receiveValue: { _ in }
            )
        
        wait(for: [expectation])
    }
}

With Xcode 15 (15A240d) on an M2 Mac Studio and an iPhone 15 Pro, I am running the test with 10000 iterations until failure (running it only once or a few times is highly unlikely to trigger the described behavior). I expect the test to never fail or crash. However:

Item is often deinitialized in a random test iteration:

Later, it may run into an EXC_BAD_ACCESS: see screenshot

I thought perhaps it was due to something environmental with XCTest, but I have tried porting the code to a similar implementation in the app itself and was able to hit an EXC_BAD_ACCESS eventually.

I'd love to know if I'm misunderstanding something, doing something unsafe or undefined.

3 Likes

hi! so i think there's at least one data race here on the mutable task variable that's captured within the receiveCompletion closure. if you run this code with the thread sanitizer enabled (and perform sufficiently many repetitions), it should catch it and explain how there is a write to task happening from the main thread, and a read from it occurring on a background thread (when the receive completion is invoked). i think the behavior is nondeterministic since there's an inherent race between the fetchItem() call completing and sink() returning.

to address the race on task, i tried moving the cancelation call to after the expectation is awaited, that way all access occurs on the main thread. this seemed to fix the sanitizer complaint (though i'm not sure if that's the appropriate behavior you're looking for), but curiously, the memory access errors would still usually occur when the test was run with many repetitions. the crashes all suggested something surrounding reference counts/deallocation of the involved types. and FWIW, i never saw Item.deinit() called, though a bug affecting reference counts seems like it could cause that behavior as a side effect. perhaps trying to find a reproduction that doesn't use Combine (or not being able to produce such an example) could further point in the direction of the underlying issue?

@jamieQ Thank you for the response and the feedback!

I'm explored this further and have been able to reproduce the behavior with both Combine and OpenCombine (see GitHub project). I also took your task suggestion into account. I also discovered that I can simplify the test case further by removing the usage of Store entirely.

I'm still puzzled as to why this is happening, and hoping for some sort of workaround or explanation. It's probably time to file a Feedback, I suppose.