-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Failed to open DB: LOCK: already held by process #3967
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
Comments
This comment has been minimized.
This comment has been minimized.
Are you possibly accessing Firestore from multiple threads very early in your application? |
Yes, right on launch on querying data from HealthKit, then saving the result into Firestore. This is sometimes called multiple times. Should I be accessing my Firestore instance from the main thread in this scenario? |
It should be the case that this is all thread-safe, but given your description it seems like there may be a race in the initialization somewhere. I'll see if I can reproduce based on that. This suggests that a workaround for now would be to call |
OK I will call once and reuse instance like you suggest and will let you know if the crash goes away. |
Reported in #3967. Users sometimes seeing a crash when multiple threads concurrently try to create Firestore instances. By inspection I found that there's a race here where two threads can check for presence in the cache, see nothing, proceed to creation, both store a value in the cache, and both return their copies. This would lead to two Firestores being created for a single App. This replaces `dispatch_sync` on a serial queue with good old `@synchronized` blocks. `dispatch_sync` will self-deadlock if called from a thread already on the queue while `@synchronized` allows recursion. No new tests are added because several tests like `testDependencyDoesntBlock` already cover this case.
I haven't been able to reproduce this issue locally, but tracing through the initialization logic I did find a race and I've proposed a fix in #3984. You can try that out by adding the following to your
Note that any existing lines for |
We released a new version yesterday and now this is our most common crash appearing for about 1% of users. Our new release contains three things related to this:
Based on environment posted in issue and above comments I would guess its #3 then since we also are using firestore early in the application. |
* Fix race condition in FIRComponentContainer instance creation Reported in #3967. Users sometimes seeing a crash when multiple threads concurrently try to create Firestore instances. By inspection I found that there's a race here where two threads can check for presence in the cache, see nothing, proceed to creation, both store a value in the cache, and both return their copies. This would lead to two Firestores being created for a single App. This replaces `dispatch_sync` on a serial queue with good old `@synchronized` blocks. `dispatch_sync` will self-deadlock if called from a thread already on the queue while `@synchronized` allows recursion. No new tests are added because several tests like `testDependencyDoesntBlock` already cover this case. * format
* Fix race condition in FIRComponentContainer instance creation Reported in #3967. Users sometimes seeing a crash when multiple threads concurrently try to create Firestore instances. By inspection I found that there's a race here where two threads can check for presence in the cache, see nothing, proceed to creation, both store a value in the cache, and both return their copies. This would lead to two Firestores being created for a single App. This replaces `dispatch_sync` on a serial queue with good old `@synchronized` blocks. `dispatch_sync` will self-deadlock if called from a thread already on the queue while `@synchronized` allows recursion. No new tests are added because several tests like `testDependencyDoesntBlock` already cover this case. * format
* Fix race condition in FIRComponentContainer instance creation (#3984) * Fix race condition in FIRComponentContainer instance creation Reported in #3967. Users sometimes seeing a crash when multiple threads concurrently try to create Firestore instances. By inspection I found that there's a race here where two threads can check for presence in the cache, see nothing, proceed to creation, both store a value in the cache, and both return their copies. This would lead to two Firestores being created for a single App. This replaces `dispatch_sync` on a serial queue with good old `@synchronized` blocks. `dispatch_sync` will self-deadlock if called from a thread already on the queue while `@synchronized` allows recursion. No new tests are added because several tests like `testDependencyDoesntBlock` already cover this case. * format * Update Core CHANGELOG * Update Core CHANGELOG again
@simonbengtsson have you been able to reproduce the issue locally? If so, does applying the fix as described above address it? |
I have not been able to reproduce, but we will release a new version in a few days so can confirm then if noone have been able to before that. |
OK. We haven't been able to reproduce either. The fix is based on inspection and the result is strictly better than what came before. My worry is that this may not have been the only cause. Please do let us know what you see. In the meantime this fix will end up in the next Firebase release. |
I have running a new version in production with the PR fix and I have yet to see the crash come up. This resolved the crashes I was seeing |
* Update versions for Release 6.10.0 * Cherrypick #3983 and include Database in release. (#3986) * Remove log argument that causes SocketRocket crash (#3983) * Add FirebaseDatabase to release. * Update CHANGELOG with version. * Cherrypick for #3984 along with CHANGELOG update. (#3988) * Fix race condition in FIRComponentContainer instance creation (#3984) * Fix race condition in FIRComponentContainer instance creation Reported in #3967. Users sometimes seeing a crash when multiple threads concurrently try to create Firestore instances. By inspection I found that there's a race here where two threads can check for presence in the cache, see nothing, proceed to creation, both store a value in the cache, and both return their copies. This would lead to two Firestores being created for a single App. This replaces `dispatch_sync` on a serial queue with good old `@synchronized` blocks. `dispatch_sync` will self-deadlock if called from a thread already on the queue while `@synchronized` allows recursion. No new tests are added because several tests like `testDependencyDoesntBlock` already cover this case. * format * Update Core CHANGELOG * Update Core CHANGELOG again * Fix merge from CHANGELOG
Uh oh!
There was an error while loading. Please reload this page.
Full error:
Xcode version: 10.3
Using Firebase (6.6.0)
Using FirebaseAnalytics (6.1.0)
Using FirebaseAnalyticsInterop (1.3.0)
Using FirebaseAuth (6.2.2)
Using FirebaseAuthInterop (1.0.0)
Using FirebaseCore (6.2.0)
Using FirebaseCoreDiagnostics (1.0.1)
Using FirebaseCoreDiagnosticsInterop (1.0.0)
Using FirebaseDatabase (6.0.0)
Using FirebaseDynamicLinks (4.0.2)
Using FirebaseFirestore (1.4.3)
Using FirebaseInstanceID (4.2.3)
Using FirebaseMessaging (4.1.2)
Using FirebaseStorage (3.4.0)
Using FirebaseUI (8.0.4)
Can't reproduce this problem. I am not deleting FIRApp instances, I'm only accessing the database via
Firestore.firestore()
or[[FIRFirestore firestore]
which should provide the same instance back. The only thing I can think of is[FIRApp configure];
getting called twice with thread race, but that is supposed to be thread-safe as well.Found this in another issue #3778 from Will. @wilhuff Is there any other scenarios which might cause this error?
The text was updated successfully, but these errors were encountered: