Skip to content

Conversation

@tusharkhandelwal8
Copy link
Contributor

This PR addresses the critical issue where Remote Config would fail to fetch new data and get stuck on default values after an iOS device was restored from a backup. It implements a two-part solution:

  1. Fixes a Logical Initialization Race to ensure the necessary resetUserDefaults logic runs.
  2. Fixes a Data Race on a global flag to eliminate a Thread Sanitizer warning.

@tusharkhandelwal8 tusharkhandelwal8 marked this pull request as ready for review October 25, 2025 08:11
@ncooke3
Copy link
Member

ncooke3 commented Oct 29, 2025

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request effectively resolves two race conditions in the Remote Config initialization process. The first fix correctly addresses a logical race condition by ensuring database setup completes before its state is read, using a blocking wait on the database operation queue. The second fix eliminates a data race on the gIsNewDatabase global flag by synchronizing all read and write access through a dedicated serial dispatch queue. The changes are well-implemented and include clear comments explaining the reasoning. I have one minor suggestion for code conciseness.

Comment on lines +1240 to +1242
dispatch_sync(_databaseOperationQueue, ^{
// Empty block forces synchronization.
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

For conciseness, you can place the empty block on a single line. The purpose of this synchronization is already well-documented by the comment above, so the inner comment is not strictly necessary.

  dispatch_sync(_databaseOperationQueue, ^{});

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tusharkhandelwal8 there are several dispatch_syncs going on here. If we're on the main queue when dispatching synchronously, we could hold up the main queue and risk deadlocks. Is there a way to make the synchronization of the property asynchronous?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants