Archives/gristlabs_grist-core
1
0
Fork 0
mirror of https://github.com/gristlabs/grist-core.git synced 2024-10-27 20:44:07 +00:00
Code Issues Projects Releases Wiki Activity

(core) Fix an insidious bug in RefCountMap, manifesting as JS errors some time after import.

Browse Source 
Summary:
After an import from inside a document, one minute later, an important
QuerySet would get disposed, leaving the view section in a bad state,
and manifesting as JS errors on subsequent operations. (Might not
*always* happen because switching pages would prevent it from
manifesting, I think.)

Bad state that I've seen after transforms is probably explainable as
this bug, which is unrelated. Reproduction was hard because who knew one
had to wait a minute?!

Test Plan:
Added a unittest for the fix in QuerySet, and a browser test that
fails without the fix (JS errors, bad state), and passes with.

Reviewers: paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D2653
This commit is contained in:
Dmitry S 2020-11-06 01:58:24 -05:00
parent e2226c3ab7
commit 4febd90758
2 changed files with 22 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
app/client/models/QuerySet.ts
View File

@ -98,6 +98,12 @@ export class QuerySetManager extends Disposable {
public purgeKey(queryKey: string) { public purgeKey(queryKey: string) {
this._queryMap.purgeKey(queryKey); this._queryMap.purgeKey(queryKey);
} }
// For testing: set gracePeriodMs, returning the previous value.
public testSetGracePeriodMs(ms: number): number {
return this._queryMap.testSetGracePeriodMs(ms);
}
} }
/** /**

20
app/common/RefCountMap.ts
View File

@ -80,14 +80,18 @@ export class RefCountMap<Key, Value> implements IDisposable {
this._map.clear(); this._map.clear();
} }
// For testing: set gracePeriodMs, returning the previous value.
public testSetGracePeriodMs(ms: number): number {
const prev = this._gracePeriodMs;
this._gracePeriodMs = ms;
return prev;
}
private _useKey(key: Key): RefCountValue<Value> { private _useKey(key: Key): RefCountValue<Value> {
const r = this._map.get(key); const r = this._map.get(key);
if (r) { if (r) {
r.count += 1; r.count += 1;
if (r.disposeTimeout) { r.unsetTimeout();
clearTimeout(r.disposeTimeout);
r.disposeTimeout = undefined;
}
return r; return r;
} }
const value = this._createKey.call(null, key); const value = this._createKey.call(null, key);
@ -116,6 +120,7 @@ export class RefCountMap<Key, Value> implements IDisposable {
if (r) { if (r) {
this._map.delete(key); this._map.delete(key);
r.count = 0; r.count = 0;
r.unsetTimeout(); // Important, to avoid timeout triggering after the same key is re-added.
this._disposeKey.call(null, key, r.value); this._disposeKey.call(null, key, r.value);
} }
} }
@ -128,4 +133,11 @@ class RefCountValue<Value> {
public count: number = 1; public count: number = 1;
public disposeTimeout?: ReturnType<typeof setTimeout> = undefined; public disposeTimeout?: ReturnType<typeof setTimeout> = undefined;
constructor(public value: Value) {} constructor(public value: Value) {}
public unsetTimeout() {
if (this.disposeTimeout) {
clearTimeout(this.disposeTimeout);
this.disposeTimeout = undefined;
}
}
} }