Skip to content

Commit da034ba

Browse files
authored
Fix multithreaded _completion access (#6068)
* Fix unguarded multithreaded access * Update changelog * fix typo in doc comments * fix nullability build error * Serialize via FIRAuthGlobalWorkQueue * remove unused code
1 parent 3e06987 commit da034ba

File tree

2 files changed

+16
-5
lines changed

2 files changed

+16
-5
lines changed

FirebaseAuth/CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
# Unreleased
2+
- [fixed] Fixed a multithreaded memory access issue on iOS (#5979).
3+
14
# v6.7.0
25
- [changed] Functionally neutral source reorganization for preliminary Swift Package Manager support. (#5856)
36

FirebaseAuth/Sources/Utilities/FIRAuthURLPresenter.m

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ @interface FIRAuthURLPresenter () <SFSafariViewControllerDelegate, FIRAuthWebVie
3939
@implementation FIRAuthURLPresenter {
4040
/** @var _isPresenting
4141
@brief Whether or not some web-based content is being presented.
42+
Accesses to this property are serialized on the global Auth work queue
43+
and thus this variable should not be read or written outside of the work queue.
4244
*/
4345
BOOL _isPresenting;
4446

@@ -63,7 +65,9 @@ @implementation FIRAuthURLPresenter {
6365
id<FIRAuthUIDelegate> _UIDelegate;
6466

6567
/** @var _completion
66-
@brief The completion handler for the current presentaion, if one is active.
68+
@brief The completion handler for the current presentation, if one is active.
69+
Accesses to this variable are serialized on the global Auth work queue
70+
and thus this variable should not be read or written outside of the work queue.
6771
@remarks This variable is also used as a flag to indicate a presentation is active.
6872
*/
6973
FIRAuthURLPresentationCompletion _Nullable _completion;
@@ -75,12 +79,16 @@ - (void)presentURL:(NSURL *)URL
7579
completion:(FIRAuthURLPresentationCompletion)completion {
7680
if (_isPresenting) {
7781
// Unable to start a new presentation on top of another.
78-
_completion(nil, [FIRAuthErrorUtils webContextAlreadyPresentedErrorWithMessage:nil]);
82+
// Invoke the new completion closure and leave the old one as-is
83+
// to be invoked when the presentation finishes.
84+
dispatch_async(dispatch_get_main_queue(), ^() {
85+
completion(nil, [FIRAuthErrorUtils webContextAlreadyPresentedErrorWithMessage:nil]);
86+
});
7987
return;
8088
}
8189
_isPresenting = YES;
8290
_callbackMatcher = callbackMatcher;
83-
_completion = completion;
91+
_completion = [completion copy];
8492
dispatch_async(dispatch_get_main_queue(), ^() {
8593
self->_UIDelegate = UIDelegate ?: [FIRAuthDefaultUIDelegate defaultUIDelegate];
8694
if ([SFSafariViewController class]) {
@@ -162,8 +170,8 @@ - (void)finishPresentationWithURL:(nullable NSURL *)URL error:(nullable NSError
162170
_callbackMatcher = nil;
163171
id<FIRAuthUIDelegate> UIDelegate = _UIDelegate;
164172
_UIDelegate = nil;
165-
FIRAuthURLPresentationCompletion completion = _completion;
166-
_completion = nil;
173+
FIRAuthURLPresentationCompletion completion = [_completion copy];
174+
_completion = NULL;
167175
void (^finishBlock)(void) = ^() {
168176
self->_isPresenting = NO;
169177
completion(URL, error);

0 commit comments

Comments
 (0)