Fix tightlooping when flush()ing without any logs

The promise would resolve immediately, nulling out `flushPromise`. This would
then immediately be set from `new Promise((resolve, reject) => {...})` turning
it back into non-null `flushPromise`. The resolve handler was called so the
next `flush()` would see "oh yes, there is a non-null `flushPromise`" then
promptly try to set `flushAgainPromise` which chains off the resolved
`flushPromise` which relied on `flushPromise` being `null`ed out after
`resolve()`, causing the chained `flush()` to see "oh yes, there is a non-null
`flushPromise`" which... ad infinitum.

This PR fixes it by making the nulling out asynchronous but the fact it took
me this long to debug this issue indicates to me that this is a terrible piece
of code. Will re-write.
pull/3000/head
Kegan Dougal 2017-01-23 17:37:57 +00:00
parent 2ecf65f057
commit f3df86872b
1 changed files with 4 additions and 4 deletions

View File

@ -201,13 +201,11 @@ class IndexedDBLogStore {
if (!this.db) {
// not connected yet or user rejected access for us to r/w to
// the db.
this.flushPromise = null;
reject(new Error("No connected database"));
return;
}
const lines = this.logger.flush();
if (lines.length === 0) {
this.flushPromise = null;
resolve();
return;
}
@ -217,18 +215,20 @@ class IndexedDBLogStore {
let lastModStore = txn.objectStore("logslastmod");
lastModStore.put(this._generateLastModifiedTime());
txn.oncomplete = (event) => {
this.flushPromise = null;
resolve();
};
txn.onerror = (event) => {
console.error(
"Failed to flush logs : ", event
);
this.flushPromise = null;
reject(
new Error("Failed to write logs: " + event.target.errorCode)
);
}
}).then(() => {
this.flushPromise = null;
}, (err) => {
this.flushPromise = null;
});
return this.flushPromise;
}