Skip to content

Replace @synchronized(self) in GIDGoogleUser with a private lock - #652

Merged
w-goog merged 8 commits into
mainfrom
fix/googleuser-auth-state-lock
Oct 20, 2026
Merged

w-goog merged 8 commits into
mainfrom
fix/googleuser-auth-state-lock

Conversation

@w-goog

@w-goog w-goog commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor
  • Change @synchronized(self) to synchronize on a private lock, to prevent other callers from battling for the user object's lock. The private lock remains recursive, because AppAuth calls didChangeState / KVO observers may need to re-enter.
  • Update the refresh path to use said lock.
  • Update emmSupport and encodeWithCoder to use said lock.
  • Known follow-up: Other readers grab authState without the lock. Tracked in GIDGoogleUser.authState is sometimes read without a lock #648.

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

Copy link
Copy Markdown

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 replaces @synchronized(self) with a recursive lock (_authStateLock) in GIDGoogleUser to serialize auth state updates and reads, avoiding potential deadlocks during KVO notifications. Feedback on the changes identifies a potential crash in refreshTokensIfNeededWithCompletion: if lastTokenAdditionalParameters is nil, as passing nil to addEntriesFromDictionary: raises an exception. A guard or fallback dictionary is recommended to prevent this crash.

Comment thread GoogleSignIn/Sources/GIDGoogleUser.m Resolved
Comment thread GoogleSignIn/Sources/GIDGoogleUser.m Resolved
Comment thread GoogleSignIn/Sources/GIDGoogleUser.m Resolved
@w-goog
w-goog force-pushed the fix/googleuser-auth-state-lock branch from a6d3717 to 988f058 Compare October 15, 2026 23:11
@w-goog
w-goog requested a review from brnnmrls October 16, 2026 19:29
Comment thread GoogleSignIn/Sources/GIDGoogleUser.m Resolved
Comment thread GoogleSignIn/Sources/GIDGoogleUser.m Resolved
@w-goog
w-goog force-pushed the fix/googleuser-auth-state-lock branch from 988f058 to 0e17431 Compare October 19, 2026 20:04
@w-goog w-goog changed the title Replace @synchronized in GIDGoogleUser with an auth state lock Oct 19, 2026
@w-goog
w-goog requested a review from brnnmrls October 19, 2026 20:19
Comment thread GoogleSignIn/Sources/GIDGoogleUser.m Resolved
Base automatically changed from fix/googleuser-snapshot-scopes-config-profile to main October 20, 2026 16:06
@w-goog
w-goog force-pushed the fix/googleuser-auth-state-lock branch from 0cfb17d to 75884b5 Compare October 20, 2026 16:06
@w-goog
w-goog merged commit a49a465 into main Oct 20, 2026
22 of 29 checks passed
@w-goog
w-goog deleted the fix/googleuser-auth-state-lock branch October 20, 2026 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants