From f3df86872b4e4bc7b4e7a489993cf3e73c6a3637 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 23 Jan 2017 17:37:57 +0000 Subject: [PATCH] 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. --- src/vector/rageshake.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/vector/rageshake.js b/src/vector/rageshake.js index 3cc158864..a17c829a5 100644 --- a/src/vector/rageshake.js +++ b/src/vector/rageshake.js @@ -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; }