summaryrefslogtreecommitdiff
path: root/src/mbgl/tile
diff options
context:
space:
mode:
authorChris Loer <chris.loer@gmail.com>2018-03-28 15:40:57 -0700
committerChris Loer <chris.loer@mapbox.com>2018-03-29 09:04:23 -0700
commite5a3a7a4510bbbf2fc97363ab0084fb4bc3dd448 (patch)
tree9e8c81aa79cc1214b89a0485147645053641a7c1 /src/mbgl/tile
parentf9b6d5f09c844ad367c54d34677d71c9da757bba (diff)
downloadqtlocation-mapboxgl-e5a3a7a4510bbbf2fc97363ab0084fb4bc3dd448.tar.gz
[core] Fix potential race condition crash in symbol querying..
Second half of fix for issue #11538. If a global placement took place between the time a tile's non-symbol layout updated and the time new symbol buckets arrived, the tile's new FeatureIndex would be committed, but the global CollisionIndex would be generated against the old symbolBuckets. The mismatch could cause the CollisionIndex to contain indices that didn't exist in the new FeatureIndex, and attempting to access them would generate an out-of-bounds exception.
Diffstat (limited to 'src/mbgl/tile')
-rw-r--r--src/mbgl/tile/geometry_tile.cpp29
-rw-r--r--src/mbgl/tile/geometry_tile.hpp4
2 files changed, 25 insertions, 8 deletions
diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp
index 7fb0c9922d..0c8a92d8b7 100644
--- a/src/mbgl/tile/geometry_tile.cpp
+++ b/src/mbgl/tile/geometry_tile.cpp
@@ -130,8 +130,10 @@ void GeometryTile::onLayout(LayoutResult result, const uint64_t resultCorrelatio
// replacing a tile at a different zoom that _did_ have symbols.
(void)resultCorrelationID;
nonSymbolBuckets = std::move(result.nonSymbolBuckets);
- pendingFeatureIndex = { std::move(result.featureIndex) };
- pendingData = { std::move(result.tileData) };
+ // It is possible for multiple onLayouts to be called before an onPlacement is called, in which
+ // case we will discard previous pending data
+ pendingFeatureIndex = {{ false, std::move(result.featureIndex) }};
+ pendingData = {{ false, std::move(result.tileData) }};
observer->onTileChanged(*this);
}
@@ -142,6 +144,17 @@ void GeometryTile::onPlacement(PlacementResult result, const uint64_t resultCorr
pending = false;
}
symbolBuckets = std::move(result.symbolBuckets);
+ // When symbolBuckets arrive, mark pendingData/FeatureIndex as "ready for commit" at the
+ // time of the next global placement. We are counting on these symbolBuckets being
+ // in sync with the data/index in the last `onLayout` call, even though the correlation IDs
+ // may not be the same (e.g. "setShowCollisionBoxes" could bump the correlation ID while
+ // waiting for glyph dependencies)
+ if (pendingData) {
+ pendingData->first = true;
+ }
+ if (pendingFeatureIndex) {
+ pendingFeatureIndex->first = true;
+ }
if (result.glyphAtlasImage) {
glyphAtlasImage = std::move(*result.glyphAtlasImage);
}
@@ -214,12 +227,16 @@ Bucket* GeometryTile::getBucket(const Layer::Impl& layer) const {
}
void GeometryTile::commitFeatureIndex() {
- if (pendingFeatureIndex) {
- featureIndex = std::move(*pendingFeatureIndex);
+ // We commit our pending FeatureIndex and GeometryTileData when:
+ // 1) An `onPlacement` result has delivered us updated symbolBuckets since we received the pending data
+ // 2) A global placement has run, synchronizing the global CollisionIndex with the latest
+ // symbolBuckets (and thus with the latest FeatureIndex/GeometryTileData)
+ if (pendingFeatureIndex && pendingFeatureIndex->first) {
+ featureIndex = std::move(pendingFeatureIndex->second);
pendingFeatureIndex = nullopt;
}
- if (pendingData) {
- data = std::move(*pendingData);
+ if (pendingData && pendingData->first) {
+ data = std::move(pendingData->second);
pendingData = nullopt;
}
}
diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp
index 0c6adf0337..8843d6e579 100644
--- a/src/mbgl/tile/geometry_tile.hpp
+++ b/src/mbgl/tile/geometry_tile.hpp
@@ -125,9 +125,9 @@ private:
std::unordered_map<std::string, std::shared_ptr<Bucket>> nonSymbolBuckets;
std::unique_ptr<FeatureIndex> featureIndex;
- optional<std::unique_ptr<FeatureIndex>> pendingFeatureIndex;
+ optional<std::pair<bool,std::unique_ptr<FeatureIndex>>> pendingFeatureIndex;
std::unique_ptr<const GeometryTileData> data;
- optional<std::unique_ptr<const GeometryTileData>> pendingData;
+ optional<std::pair<bool, std::unique_ptr<const GeometryTileData>>> pendingData;
optional<AlphaImage> glyphAtlasImage;
optional<PremultipliedImage> iconAtlasImage;