Firestore 9.2.0 introduces a crash on save race condition, not present in 9.1.0

[sergiocampama]

[REQUIRED] Step 1: Describe your environment

• Xcode version: 13.4
• Firebase SDK version: 9.2.0
• Installation method: SwiftPM
• Firebase Component: Firestore
• Target platform(s): All | iOS | Catalyst | macOS | tvOS | watchOS

[REQUIRED] Step 2: Describe the problem

Starting on 9.2.0, there was some change in Firestore that crashes on a race condition when the same document is saved at the same time. I was able to trigger this by deleting a field from 2 simultaneous async tasks for the same document. For some reason, this does not happen in 9.1.0, but starts happening on 9.2.0 and still happens on 9.3.0.

The crash happens on

firebase-ios-sdk/Firestore/core/src/model/mutation.cc

Line 202 in 61dfd02

HARD_ASSERT(value.has_value());

. I haven't understood yet why it happens on 9.2.0 but not 9.1.0, given the code was added in 9.0.0.

Furthermore, on our production (luckily unreleased) app, this crash results in an unrecoverable state, where the only solution is to delete the app, reset the device and reinstall the app for the database to recover. I assume because the mutation is saved as incomplete and retried at some later time.

I would expect there to be way more testing and discussions when adding HARD_FAILs into the codebase, as this code is most likely going to be released in an iOS app, and if there's a bug like this one, it can result in very angry users and developers (me being one of them). Again, I'll have to pin my app to an older version of Firebase until this issue is resolved. I was lucky to have caught this before we released an update with this version.

Philosophy wise, Firebase should strive hard to not add HARD_FAILs into the codebase at all, and instead when an unexpected state occurs, it should just return an error on the action. While I understand that it could be a developer error to save the same document from 2 different places, this used to not crash and just work, and hence this became part of the public API. Changing this without a notice in the release notes is not what I would expect from a project following semantic versioning.

Steps to reproduce:

git clone https://github.com/sergiocampama/firestore
cd firestore

swift run
# the app should work, since it's using 9.1.0

# edit Package.swift to use 9.2.0 instead

swift run
# the app should crash now

Disclaimer: This code accesses a newly and empty created firebase app and is limited into what it can read and write. Also it's empty and does not have any production or development values, it is only created for this bug report to ease investigation. It will likely be deleted once this is solved.

Relevant Code:

cc: @dconeybe @wu-hui

[wu-hui]

This is likely the same root cause of #9985

A fix is being worked on, I'll try it out with your repo too.

[sergiocampama]

Tested the repo with the wuandy/OverlyNestedFieldFix branch and it works as expected. Thanks for the quick turnaround!