Fix race condition between setting developer mode and config fetch #5600
Conversation
|
More investigation still needed for |
| [self fetchWithExpirationDuration:_settings.minimumFetchInterval | ||
| completionHandler:completionHandler]; | ||
| __block NSTimeInterval minimumFetchInterval; | ||
| dispatch_sync(_queue, ^{ |
There was a problem hiding this comment.
In general dispatch_sync should be avoided unless required because the more dispatch_sync is used the easier it is to introduce a deadlock even when modifying code not directly related. I wonder if it can be updated to either:
- Use
atomicproperties or_Atomicvars for variables involved in the race condition - Use
dispatch_asynce.g. :
dispatch_async(_queue, ^{
[self fetchWithExpirationDuration:self->_settings.minimumFetchInterval completionHandler:completionHandler];
}
EDITIED:
It looks like #2 should be better though it may require modification of fetchWithExpirationDuration... method to account for calling from the queue.
There was a problem hiding this comment.
#2 is probably better as remote config's mutex lock is all guarded by this queue. The fetch is assigned to async again inside https://github.com/firebase/firebase-ios-sdk/blob/master/FirebaseRemoteConfig/Sources/RCNFetch.m#L139
So using another mechanism might be more expensive since we already have a queue just for that.
|
Testing was done manually with the new API tests in #5602. They'll be merged once automation is added for them. All other automated tests passed on commit before changelog update. |
Fix #4740
Insure that minimumFetchInterval setting is applied before fetching. Otherwise there will be indeterministic behavior when trying to verify console changes in an app.