Skip to content

Commit a1e346f

Browse files
authored
Refactor AsyncQueue's "delayed scheduling" support and add cancellation. (#489)
* Introduces a DelayedOperation helper class in AsyncQueue to encapsulate delayed op logic. * Adds cancellation support which I want to use in #412 * Updates the idle timer in persistent_stream.ts to use new cancellation support. * Remove delayedOperationsCount in favor of keeping delayedOperations populated correctly at all times. * Fixes a preexisting issue in AsyncQueue.schedule() where the returned promise would always be resolved with undefined, instead of the result of your op. Also remove an AnyDuringMigration usage.
1 parent 2db58a0 commit a1e346f

File tree

3 files changed

+175
-86
lines changed

3 files changed

+175
-86
lines changed

packages/firestore/src/remote/persistent_stream.ts

+25-11
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import { Connection, Stream } from './connection';
3131
import { JsonProtoSerializer } from './serializer';
3232
import { WatchChange } from './watch_change';
3333
import { isNullOrUndefined } from '../util/types';
34+
import { CancelablePromise } from '../util/promise';
3435

3536
const LOG_TAG = 'PersistentStream';
3637

@@ -154,7 +155,7 @@ export abstract class PersistentStream<
154155
ListenerType extends PersistentStreamListener
155156
> {
156157
private state: PersistentStreamState;
157-
private idle = false;
158+
private inactivityTimerPromise: CancelablePromise<void> | null = null;
158159
private stream: Stream<SendType, ReceiveType> | null = null;
159160

160161
protected backoff: ExponentialBackoff;
@@ -245,16 +246,25 @@ export abstract class PersistentStream<
245246
}
246247

247248
/**
248-
* Initializes the idle timer. If no write takes place within one minute, the
249-
* WebChannel stream will be closed.
249+
* Marks this stream as idle. If no further actions are performed on the
250+
* stream for one minute, the stream will automatically close itself and
251+
* notify the stream's onClose() handler with Status.OK. The stream will then
252+
* be in a !isStarted() state, requiring the caller to start the stream again
253+
* before further use.
254+
*
255+
* Only streams that are in state 'Open' can be marked idle, as all other
256+
* states imply pending network operations.
250257
*/
251258
markIdle(): void {
252-
this.idle = true;
253-
this.queue
254-
.schedule(() => {
255-
return this.handleIdleCloseTimer();
256-
}, IDLE_TIMEOUT_MS)
257-
.catch((err: FirestoreError) => {
259+
// Starts the idle time if we are in state 'Open' and are not yet already
260+
// running a timer (in which case the previous idle timeout still applies).
261+
if (this.isOpen() && this.inactivityTimerPromise === null) {
262+
this.inactivityTimerPromise = this.queue.scheduleWithDelay(
263+
() => this.handleIdleCloseTimer(),
264+
IDLE_TIMEOUT_MS
265+
);
266+
267+
this.inactivityTimerPromise.catch((err: FirestoreError) => {
258268
// When the AsyncQueue gets drained during testing, pending Promises
259269
// (including these idle checks) will get rejected. We special-case
260270
// these cancelled idle checks to make sure that these specific Promise
@@ -266,6 +276,7 @@ export abstract class PersistentStream<
266276
}`
267277
);
268278
});
279+
}
269280
}
270281

271282
/** Sends a message to the underlying stream. */
@@ -276,7 +287,7 @@ export abstract class PersistentStream<
276287

277288
/** Called by the idle timer when the stream should close due to inactivity. */
278289
private handleIdleCloseTimer(): Promise<void> {
279-
if (this.isOpen() && this.idle) {
290+
if (this.isOpen()) {
280291
// When timing out an idle stream there's no reason to force the stream into backoff when
281292
// it restarts so set the stream state to Initial instead of Error.
282293
return this.close(PersistentStreamState.Initial);
@@ -286,7 +297,10 @@ export abstract class PersistentStream<
286297

287298
/** Marks the stream as active again. */
288299
private cancelIdleCheck() {
289-
this.idle = false;
300+
if (this.inactivityTimerPromise) {
301+
this.inactivityTimerPromise.cancel();
302+
this.inactivityTimerPromise = null;
303+
}
290304
}
291305

292306
/**

packages/firestore/src/util/async_queue.ts

+146-75
Original file line numberDiff line numberDiff line change
@@ -17,33 +17,120 @@
1717
import { assert, fail } from './assert';
1818
import * as log from './log';
1919
import { AnyDuringMigration, AnyJs } from './misc';
20-
import { Deferred } from './promise';
20+
import { Deferred, CancelablePromise } from './promise';
2121
import { Code, FirestoreError } from './error';
2222

23-
type DelayedOperation<T> = {
24-
// tslint:disable-next-line:no-any Accept any return type from setTimeout().
25-
handle: any;
26-
op: () => Promise<T>;
27-
deferred: Deferred<T>;
28-
};
23+
// tslint:disable-next-line:no-any Accept any return type from setTimeout().
24+
type TimerHandle = any;
25+
26+
/**
27+
* Represents an operation scheduled to be run in the future on an AsyncQueue.
28+
*
29+
* It is created via DelayedOperation.createAndSchedule().
30+
*
31+
* Supports cancellation (via cancel()) and early execution (via skipDelay()).
32+
*/
33+
class DelayedOperation<T> implements CancelablePromise<T> {
34+
// handle for use with clearTimeout(), or null if the operation has been
35+
// executed or canceled already.
36+
private timerHandle: TimerHandle | null;
37+
38+
private readonly deferred = new Deferred<T>();
39+
40+
private constructor(
41+
private asyncQueue: AsyncQueue,
42+
private op: () => Promise<T>
43+
) {}
44+
45+
/**
46+
* Creates and returns a DelayedOperation that has been scheduled to be
47+
* executed on the provided asyncQueue after the provided delayMs.
48+
*/
49+
static createAndSchedule<T>(
50+
asyncQueue: AsyncQueue,
51+
op: () => Promise<T>,
52+
delayMs: number
53+
): DelayedOperation<T> {
54+
const delayedOp = new DelayedOperation(asyncQueue, op);
55+
delayedOp.start(delayMs);
56+
return delayedOp;
57+
}
58+
59+
/**
60+
* Starts the timer. This is called immediately after construction by
61+
* createAndSchedule().
62+
*/
63+
private start(delayMs: number): void {
64+
this.timerHandle = setTimeout(() => this.handleDelayElapsed(), delayMs);
65+
}
66+
67+
/**
68+
* Queues the operation to run immediately (if it hasn't already been run or
69+
* canceled).
70+
*/
71+
skipDelay(): void {
72+
return this.handleDelayElapsed();
73+
}
74+
75+
/**
76+
* Cancels the operation if it hasn't already been executed or canceled. The
77+
* promise will be rejected.
78+
*
79+
* As long as the operation has not yet been run, calling cancel() provides a
80+
* guarantee that the operation will not be run.
81+
*/
82+
cancel(reason?: string): void {
83+
if (this.timerHandle !== null) {
84+
this.clearTimeout();
85+
this.deferred.reject(
86+
new FirestoreError(
87+
Code.CANCELLED,
88+
'Operation cancelled' + (reason ? ': ' + reason : '')
89+
)
90+
);
91+
}
92+
}
93+
94+
// Promise implementation.
95+
readonly [Symbol.toStringTag]: 'Promise';
96+
then = this.deferred.promise.then.bind(this.deferred.promise);
97+
catch = this.deferred.promise.catch.bind(this.deferred.promise);
98+
99+
private handleDelayElapsed(): void {
100+
this.asyncQueue.schedule(() => {
101+
if (this.timerHandle !== null) {
102+
this.clearTimeout();
103+
return this.op().then(result => {
104+
return this.deferred.resolve(result);
105+
});
106+
} else {
107+
return Promise.resolve();
108+
}
109+
});
110+
}
111+
112+
private clearTimeout() {
113+
if (this.timerHandle) {
114+
clearTimeout(this.timerHandle);
115+
this.timerHandle = null;
116+
}
117+
}
118+
}
29119

30120
export class AsyncQueue {
31121
// The last promise in the queue.
32122
private tail: Promise<AnyJs | void> = Promise.resolve();
33123

34124
// A list with timeout handles and their respective deferred promises.
35125
// Contains an entry for each operation that is queued to run in the future
36-
// (i.e. it has a delay that has not yet elapsed). Prior to cleanup, this list
37-
// may also contain entries that have already been run (in which case `handle` is
38-
// null).
126+
// (i.e. it has a delay that has not yet elapsed).
39127
private delayedOperations: Array<DelayedOperation<AnyJs>> = [];
40128

41129
// The number of operations that are queued to be run in the future (i.e. they
42-
// have a delay that has not yet elapsed). Unlike `delayedOperations`, this
43-
// is guaranteed to only contain operations that have not yet been run.
44-
//
45-
// Visible for testing.
46-
delayedOperationsCount = 0;
130+
// have a delay that has not yet elapsed). Used for testing.
131+
get delayedOperationsCount() {
132+
return this.delayedOperations.length;
133+
}
47134

48135
// visible for testing
49136
failure: Error;
@@ -55,47 +142,10 @@ export class AsyncQueue {
55142
/**
56143
* Adds a new operation to the queue. Returns a promise that will be resolved
57144
* when the promise returned by the new operation is (with its value).
58-
*
59-
* Can optionally specify a delay (in milliseconds) to wait before queuing the
60-
* operation.
61145
*/
62-
schedule<T>(op: () => Promise<T>, delay?: number): Promise<T> {
63-
if (this.failure) {
64-
fail(
65-
'AsyncQueue is already failed: ' +
66-
(this.failure.stack || this.failure.message)
67-
);
68-
}
69-
70-
if ((delay || 0) > 0) {
71-
this.delayedOperationsCount++;
72-
const delayedOp: DelayedOperation<T> = {
73-
handle: null,
74-
op,
75-
deferred: new Deferred<T>()
76-
};
77-
delayedOp.handle = setTimeout(() => {
78-
this.scheduleInternal(() => {
79-
return delayedOp.op().then(result => {
80-
delayedOp.deferred.resolve(result);
81-
});
82-
});
83-
delayedOp.handle = null;
84-
85-
this.delayedOperationsCount--;
86-
if (this.delayedOperationsCount === 0) {
87-
this.delayedOperations = [];
88-
}
89-
}, delay);
90-
this.delayedOperations.push(delayedOp);
91-
return delayedOp.deferred.promise;
92-
} else {
93-
return this.scheduleInternal(op);
94-
}
95-
}
96-
97-
private scheduleInternal<T>(op: () => Promise<T>): Promise<T> {
98-
this.tail = this.tail.then(() => {
146+
schedule<T>(op: () => Promise<T>): Promise<T> {
147+
this.verifyNotFailed();
148+
const newTail = this.tail.then(() => {
99149
this.operationInProgress = true;
100150
return op()
101151
.catch(error => {
@@ -118,11 +168,45 @@ export class AsyncQueue {
118168
// and return the rejected Promise.
119169
throw error;
120170
})
121-
.then(() => {
171+
.then(result => {
122172
this.operationInProgress = false;
173+
return result;
123174
});
124175
});
125-
return this.tail as AnyDuringMigration;
176+
this.tail = newTail;
177+
return newTail;
178+
}
179+
180+
/**
181+
* Schedules an operation to be run on the AsyncQueue once the specified
182+
* `delayMs` has elapsed. The returned DelayedOperationResult can be
183+
* used to cancel the operation prior to its running.
184+
*/
185+
scheduleWithDelay<T>(
186+
op: () => Promise<T>,
187+
delayMs: number
188+
): CancelablePromise<T> {
189+
this.verifyNotFailed();
190+
191+
const delayedOp = DelayedOperation.createAndSchedule(this, op, delayMs);
192+
this.delayedOperations.push(delayedOp);
193+
194+
delayedOp.catch(err => {}).then(() => {
195+
// NOTE: indexOf / slice are O(n), but delayedOperations is expected to be small.
196+
const index = this.delayedOperations.indexOf(delayedOp);
197+
assert(index >= 0, 'Delayed operation not found.');
198+
this.delayedOperations.slice(index, 1);
199+
});
200+
return delayedOp;
201+
}
202+
203+
private verifyNotFailed(): void {
204+
if (this.failure) {
205+
fail(
206+
'AsyncQueue is already failed: ' +
207+
(this.failure.stack || this.failure.message)
208+
);
209+
}
126210
}
127211

128212
/**
@@ -143,26 +227,13 @@ export class AsyncQueue {
143227
* scheduled with a delay can be rejected or queued for immediate execution.
144228
*/
145229
drain(executeDelayedTasks: boolean): Promise<void> {
146-
this.delayedOperations.forEach(entry => {
147-
if (entry.handle) {
148-
clearTimeout(entry.handle);
149-
if (executeDelayedTasks) {
150-
this.scheduleInternal(entry.op).then(
151-
entry.deferred.resolve,
152-
entry.deferred.reject
153-
);
154-
} else {
155-
entry.deferred.reject(
156-
new FirestoreError(
157-
Code.CANCELLED,
158-
'Operation cancelled by shutdown'
159-
)
160-
);
161-
}
230+
this.delayedOperations.forEach(delayedOp => {
231+
if (executeDelayedTasks) {
232+
delayedOp.skipDelay();
233+
} else {
234+
delayedOp.cancel('shutdown');
162235
}
163236
});
164-
this.delayedOperations = [];
165-
this.delayedOperationsCount = 0;
166237
return this.schedule(() => Promise.resolve());
167238
}
168239
}

packages/firestore/src/util/promise.ts

+4
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ export interface Rejecter {
2424
(reason?: Error): void;
2525
}
2626

27+
export interface CancelablePromise<T> extends Promise<T> {
28+
cancel(): void;
29+
}
30+
2731
export class Deferred<R> {
2832
promise: Promise<R>;
2933
resolve: Resolver<R>;

0 commit comments

Comments
 (0)