diff --git a/Firestore/CHANGELOG.md b/Firestore/CHANGELOG.md index 0aba04ead59..483146b765c 100644 --- a/Firestore/CHANGELOG.md +++ b/Firestore/CHANGELOG.md @@ -1,5 +1,7 @@ # Unreleased +- [fixed] Addressed race condition during the teardown of idle streams (#490). + # v0.9.3 - [changed] Improved performance loading documents matching a query. - [changed] Cleanly shut down idle write streams. diff --git a/Firestore/Source/Remote/FSTStream.m b/Firestore/Source/Remote/FSTStream.m index 614f6110d39..ac51cb99950 100644 --- a/Firestore/Source/Remote/FSTStream.m +++ b/Firestore/Source/Remote/FSTStream.m @@ -343,6 +343,9 @@ - (void)tearDown { - (void)closeWithFinalState:(FSTStreamState)finalState error:(nullable NSError *)error { FSTAssert(finalState == FSTStreamStateError || error == nil, @"Can't provide an error when not in an error state."); + FSTAssert(self.delegate, + @"closeWithFinalState should only be called for a started stream that has an active " + @"delegate."); [self.workerDispatchQueue verifyIsCurrentQueue]; [self cancelIdleCheck]; @@ -388,9 +391,6 @@ - (void)closeWithFinalState:(FSTStreamState)finalState error:(nullable NSError * } // Clear the delegates to avoid any possible bleed through of events from GRPC. - FSTAssert(_delegate, - @"closeWithFinalState should only be called for a started stream that has an active " - @"delegate."); _delegate = nil; } @@ -515,7 +515,11 @@ - (void)handleStreamMessage:(id)value { */ - (void)handleStreamClose:(nullable NSError *)error { FSTLog(@"%@ %p close: %@", NSStringFromClass([self class]), (__bridge void *)self, error); - FSTAssert([self isStarted], @"Can't handle server close in non-started state."); + + if (![self isStarted]) { // The stream could have already been closed by the idle close timer. + FSTLog(@"%@ Ignoring server close for already closed stream.", NSStringFromClass([self class])); + return; + } // In theory the stream could close cleanly, however, in our current model we never expect this // to happen because if we stop a stream ourselves, this callback will never be called. To @@ -615,7 +619,9 @@ - (void)notifyStreamOpen { } - (void)notifyStreamInterruptedWithError:(nullable NSError *)error { - [self.delegate watchStreamWasInterruptedWithError:error]; + id delegate = self.delegate; + self.delegate = nil; + [delegate watchStreamWasInterruptedWithError:error]; } - (void)watchQuery:(FSTQueryData *)query { @@ -701,7 +707,9 @@ - (void)notifyStreamOpen { } - (void)notifyStreamInterruptedWithError:(nullable NSError *)error { - [self.delegate writeStreamWasInterruptedWithError:error]; + id delegate = self.delegate; + self.delegate = nil; + [delegate writeStreamWasInterruptedWithError:error]; } - (void)tearDown {