Skip to content

Commit 0fafd41

Browse files
committed
Abort onSnapshotListeners on terminate.
1 parent b9244a5 commit 0fafd41

File tree

4 files changed

+72
-8
lines changed

4 files changed

+72
-8
lines changed

packages/firestore/src/core/component_provider.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,5 +485,6 @@ export class OnlineComponentProvider {
485485
async terminate(): Promise<void> {
486486
await remoteStoreShutdown(this.remoteStore);
487487
this.datastore?.terminate();
488+
this.eventManager?.terminate();
488489
}
489490
}

packages/firestore/src/core/event_manager.ts

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
import { debugAssert, debugCast } from '../util/assert';
1919
import { wrapInUserErrorIfRecoverable } from '../util/async_queue';
20-
import { FirestoreError } from '../util/error';
20+
import { Code, FirestoreError } from '../util/error';
2121
import { EventHandler } from '../util/misc';
2222
import { ObjectMap } from '../util/obj_map';
2323

@@ -64,19 +64,17 @@ export interface EventManager {
6464
onUnlisten?: (query: Query, disableRemoteListen: boolean) => Promise<void>;
6565
onFirstRemoteStoreListen?: (query: Query) => Promise<void>;
6666
onLastRemoteStoreUnlisten?: (query: Query) => Promise<void>;
67+
terminate(): void;
6768
}
6869

6970
export function newEventManager(): EventManager {
7071
return new EventManagerImpl();
7172
}
7273

7374
export class EventManagerImpl implements EventManager {
74-
queries = new ObjectMap<Query, QueryListenersInfo>(
75-
q => canonifyQuery(q),
76-
queryEquals
77-
);
75+
queries: ObjectMap<Query, QueryListenersInfo> = newQueriesObjectMap();
7876

79-
onlineState = OnlineState.Unknown;
77+
onlineState: OnlineState = OnlineState.Unknown;
8078

8179
snapshotsInSyncListeners: Set<Observer<void>> = new Set();
8280

@@ -98,6 +96,20 @@ export class EventManagerImpl implements EventManager {
9896
* still listening to the cache.
9997
*/
10098
onLastRemoteStoreUnlisten?: (query: Query) => Promise<void>;
99+
100+
terminate(): void {
101+
errorAllTargets(
102+
this,
103+
new FirestoreError(Code.ABORTED, 'Firestore shutting down')
104+
);
105+
}
106+
}
107+
108+
function newQueriesObjectMap(): ObjectMap<Query, QueryListenersInfo> {
109+
return new ObjectMap<Query, QueryListenersInfo>(
110+
q => canonifyQuery(q),
111+
queryEquals
112+
);
101113
}
102114

103115
function validateEventManager(eventManagerImpl: EventManagerImpl): void {
@@ -334,6 +346,20 @@ export function removeSnapshotsInSyncListener(
334346
eventManagerImpl.snapshotsInSyncListeners.delete(observer);
335347
}
336348

349+
function errorAllTargets(eventManager: EventManager, error: FirestoreError) {
350+
const eventManagerImpl = debugCast(eventManager, EventManagerImpl);
351+
const queries = eventManagerImpl.queries;
352+
353+
// Prevent further access by clearing ObjectMap.
354+
eventManagerImpl.queries = newQueriesObjectMap();
355+
356+
queries.forEach((_, queryInfo) => {
357+
for (const listener of queryInfo.listeners) {
358+
listener.onError(error);
359+
}
360+
});
361+
}
362+
337363
// Call all global snapshot listeners that have been set.
338364
function raiseSnapshotsInSyncEvent(eventManagerImpl: EventManagerImpl): void {
339365
eventManagerImpl.snapshotsInSyncListeners.forEach(observer => {

packages/firestore/src/core/firestore_client.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,8 @@ export class FirestoreClient {
118118
appCheckToken: string,
119119
user: User
120120
) => Promise<void> = () => Promise.resolve();
121+
private _isShutdown: boolean = false;
122+
private onTerminatedListener: (() => void) | undefined;
121123
_uninitializedComponentsProvider?: {
122124
_offline: OfflineComponentProvider;
123125
_offlineKind: 'memory' | 'persistent';
@@ -174,6 +176,13 @@ export class FirestoreClient {
174176
this.appCheckCredentialListener = listener;
175177
}
176178

179+
setOnTermintatedListener(listener: () => void): void {
180+
this.onTerminatedListener = listener;
181+
if (this._isShutdown) {
182+
listener();
183+
}
184+
}
185+
177186
/**
178187
* Checks that the client has not been terminated. Ensures that other methods on //
179188
* this class cannot be called after the client is terminated. //
@@ -188,9 +197,16 @@ export class FirestoreClient {
188197
}
189198

190199
terminate(): Promise<void> {
200+
if (this._isShutdown) {
201+
return Promise.resolve();
202+
}
191203
this.asyncQueue.enterRestrictedMode();
192204
const deferred = new Deferred();
193205
this.asyncQueue.enqueueAndForgetEvenWhileRestricted(async () => {
206+
if (this._isShutdown) {
207+
deferred.resolve();
208+
return;
209+
}
194210
try {
195211
if (this._onlineComponents) {
196212
await this._onlineComponents.terminate();
@@ -204,7 +220,11 @@ export class FirestoreClient {
204220
// tokens.
205221
this.authCredentials.shutdown();
206222
this.appCheckCredentials.shutdown();
223+
this._isShutdown = true;
207224
deferred.resolve();
225+
if (this.onTerminatedListener) {
226+
this.onTerminatedListener();
227+
}
208228
} catch (e) {
209229
const firestoreError = wrapInUserErrorIfRecoverable(
210230
e as Error,
@@ -239,7 +259,7 @@ export async function setOfflineComponentProvider(
239259
});
240260

241261
// When a user calls clearPersistence() in one client, all other clients
242-
// need to be terminated to allow the delete to succeed.
262+
// need to be restarted to allow the delete to succeed.
243263
offlineComponentProvider.persistence.setDatabaseDeletedListener(() =>
244264
client.terminate()
245265
);

packages/firestore/test/integration/api/database.test.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ import {
6363
FieldPath,
6464
newTestFirestore,
6565
SnapshotOptions,
66-
newTestApp
66+
newTestApp,
67+
FirestoreError
6768
} from '../util/firebase_export';
6869
import {
6970
apiDescribe,
@@ -1442,6 +1443,22 @@ apiDescribe('Database', persistence => {
14421443
});
14431444
});
14441445

1446+
it('query listener throws error on termination', async () => {
1447+
return withTestDoc(persistence, async (docRef, firestore) => {
1448+
const deferred: Deferred<FirestoreError> = new Deferred();
1449+
const unsubscribe = onSnapshot(docRef, snapshot => {}, deferred.resolve);
1450+
1451+
await terminate(firestore);
1452+
1453+
await expect(deferred.promise)
1454+
.to.eventually.haveOwnProperty('message')
1455+
.equal('Firestore shutting down');
1456+
1457+
// Call should proceed without error.
1458+
unsubscribe();
1459+
});
1460+
});
1461+
14451462
it('can wait for pending writes', async () => {
14461463
await withTestDoc(persistence, async (docRef, firestore) => {
14471464
// Prevent pending writes receiving acknowledgement.

0 commit comments

Comments
 (0)