SwiftUI: Could this be a potential race condition in Firestore?

Question

I have two functions add(_:completion:) and get(query:completion:) which I feel may cause errors in my code. Since the add(_:completion:) appends a Book instance to the @Published books property it might cause a race condition with the listener. This is how I imagine it might happen:

1. I add a book to the books collection in Firestore using add(_:completion:)
2. Once the book is added the listener gets fired and at the same time the original add(_:completion:) function is about to execute it's completion handler
3. If the add(_:completion:) completion handler is executed before the get(query:completion:) listener block then there will be no issues because the subsequent call to the listener block will just overwrite the self.books property by doing self.books = books. However, if the get(query:completion:) listener block is executed before, then the subsequent execution of the add(_:completion:) completion handler will append a duplicate book to the end of the self.books property by doing self.books.append(book).

I've marked all the important parts of my code with //<<< Over here!!!

class BookRepository: ObservableObject {
    @Published private(set) var books = [Book]()
    private var listener: ListenerRegistration?

    func add(_ book: Book, completion: @escaping (Error?) -> Void) {
        let _ = try! db.collection(.books).addDocument(from: book) { error in
            if error == nil { self.books.append(book) }  //<<< (1) Over here!!! Might get called at the same time as (2)
            completion(error)
        }
    }
    func get(query: Query, completion: @escaping (BookError?) -> Void) {
        // Storing listener to single property so multiple calls to get(query:completion:) won't create multiple listeners
        listener = query.addSnapshotListener { snapshot, error in
            var err: BookError?

            // Make sure no network errors
            guard error != nil else { completion(.networkError); return }
            precondition(snapshot != nil, "If there's no error snapshot must exist")
            
            // Convert documents to Book instances
            let documents = snapshot!.documents
            let books = documents.compactMap { try? $0.data(as: Book.self) }
            
            // Check if all documents were successfully decoded and if I didn't try to access an empty collection
            if documents.count != books.count || documents.isEmpty {
                err = documents.isEmpty
                            ? .emptyQuery
                            : .incorrectBookData(self.getBadData(documents, books))  // Retrieves ID's of documents which weren't decoded into Books
            }
            
            // Assign books to the @Published property to update UI
            self.books = books  //<<< (2) Over here!!! Might get called at the same time as (1)
            
            // Call completion handler
            completion(err)
        }
    }

    enum BookError: Error {
         case networkError
         case emptyQuery
         case incorrectBookData([String])
    }

    func getBadData(_ docs: [QueryDocumentSnapshot], _ books: [Book]) -> [String] {
         //...
    }
}

Am I correct in assuming this might cause a race condition? What is the sequence of how this will execute?

Answer 1

From what you've posted, it sounds like there could be a race condition here.

You might want to solve this by making sure the add and get methods are never executing at the same time. You could use some kind of operation queue for this.