From b42a665b2fc3010622994c4548842463bad4a1d9 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 13 Mar 2018 17:27:48 -0700 Subject: [PATCH] Fix race condition in VectorTileWorkerSource#reloadTile --- src/source/vector_tile_worker_source.js | 24 ++--- .../source/vector_tile_worker_source.test.js | 101 ++++++++++++++++++ 2 files changed, 112 insertions(+), 13 deletions(-) diff --git a/src/source/vector_tile_worker_source.js b/src/source/vector_tile_worker_source.js index 46d08ec96f3..b02cdf4d94b 100644 --- a/src/source/vector_tile_worker_source.js +++ b/src/source/vector_tile_worker_source.js @@ -147,22 +147,20 @@ class VectorTileWorkerSource implements WorkerSource { const workerTile = loaded[uid]; workerTile.showCollisionBoxes = params.showCollisionBoxes; + const done = (err, data) => { + const reloadCallback = workerTile.reloadCallback; + if (reloadCallback) { + delete workerTile.reloadCallback; + workerTile.parse(workerTile.vectorTile, vtSource.layerIndex, vtSource.actor, reloadCallback); + } + callback(err, data); + }; + if (workerTile.status === 'parsing') { - workerTile.reloadCallback = callback; + workerTile.reloadCallback = done; } else if (workerTile.status === 'done') { - workerTile.parse(workerTile.vectorTile, this.layerIndex, this.actor, done.bind(workerTile)); - } - - } - - function done(err, data) { - if (this.reloadCallback) { - const reloadCallback = this.reloadCallback; - delete this.reloadCallback; - this.parse(this.vectorTile, vtSource.layerIndex, vtSource.actor, reloadCallback); + workerTile.parse(workerTile.vectorTile, this.layerIndex, this.actor, done); } - - callback(err, data); } } diff --git a/test/unit/source/vector_tile_worker_source.test.js b/test/unit/source/vector_tile_worker_source.test.js index 5814a6cb1d9..90787a68bc7 100644 --- a/test/unit/source/vector_tile_worker_source.test.js +++ b/test/unit/source/vector_tile_worker_source.test.js @@ -51,6 +51,107 @@ test('VectorTileWorkerSource#removeTile removes loaded tile', (t) => { t.end(); }); +test('VectorTileWorkerSource#reloadTile reloads a previously-loaded tile', (t) => { + const source = new VectorTileWorkerSource(null, new StyleLayerIndex()); + const parse = t.spy(); + + source.loaded = { + '0': { + status: 'done', + parse + } + }; + + const callback = t.spy(); + source.reloadTile({ uid: 0 }, callback); + t.equal(parse.callCount, 1); + + parse.firstCall.args[3](); + t.equal(callback.callCount, 1); + + t.end(); +}); + +test('VectorTileWorkerSource#reloadTile queues a reload when parsing is in progress', (t) => { + const source = new VectorTileWorkerSource(null, new StyleLayerIndex()); + const parse = t.spy(); + + source.loaded = { + '0': { + status: 'done', + parse + } + }; + + const callback1 = t.spy(); + const callback2 = t.spy(); + source.reloadTile({ uid: 0 }, callback1); + t.equal(parse.callCount, 1); + + source.loaded[0].status = 'parsing'; + source.reloadTile({ uid: 0 }, callback2); + t.equal(parse.callCount, 1); + + parse.firstCall.args[3](); + t.equal(parse.callCount, 2); + t.equal(callback1.callCount, 1); + t.equal(callback2.callCount, 0); + + parse.secondCall.args[3](); + t.equal(callback1.callCount, 1); + t.equal(callback2.callCount, 1); + + t.end(); +}); + +test('VectorTileWorkerSource#reloadTile handles multiple pending reloads', (t) => { + // https://github.com/mapbox/mapbox-gl-js/issues/6308 + const source = new VectorTileWorkerSource(null, new StyleLayerIndex()); + const parse = t.spy(); + + source.loaded = { + '0': { + status: 'done', + parse + } + }; + + const callback1 = t.spy(); + const callback2 = t.spy(); + const callback3 = t.spy(); + source.reloadTile({ uid: 0 }, callback1); + t.equal(parse.callCount, 1); + + source.loaded[0].status = 'parsing'; + source.reloadTile({ uid: 0 }, callback2); + t.equal(parse.callCount, 1); + + parse.firstCall.args[3](); + t.equal(parse.callCount, 2); + t.equal(callback1.callCount, 1); + t.equal(callback2.callCount, 0); + t.equal(callback3.callCount, 0); + + source.reloadTile({ uid: 0 }, callback3); + t.equal(parse.callCount, 2); + t.equal(callback1.callCount, 1); + t.equal(callback2.callCount, 0); + t.equal(callback3.callCount, 0); + + parse.secondCall.args[3](); + t.equal(parse.callCount, 3); + t.equal(callback1.callCount, 1); + t.equal(callback2.callCount, 1); + t.equal(callback3.callCount, 0); + + parse.thirdCall.args[3](); + t.equal(callback1.callCount, 1); + t.equal(callback2.callCount, 1); + t.equal(callback3.callCount, 1); + + t.end(); +}); + test('VectorTileWorkerSource provides resource timing information', (t) => { const rawTileData = fs.readFileSync(path.join(__dirname, '/../../fixtures/mbsv5-6-18-23.vector.pbf'));