summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMikhail Pozdnyakov <mikhail.pozdnyakov@mapbox.com>2020-01-09 15:50:48 +0200
committerMikhail Pozdnyakov <mikhail.pozdnyakov@mapbox.com>2020-01-09 16:52:13 +0200
commit3ea459233a60ee737d0b637582f94024f3763797 (patch)
tree68688c9c564e69dccb66f8a97f2ee551a8968127
parent236afc03958cc729b3b8121a1c5a72660f0e9fa2 (diff)
downloadqtlocation-mapboxgl-3ea459233a60ee737d0b637582f94024f3763797.tar.gz
[core] Fix GeoJSONVTData ownership and life cycle
Before this change, the `GeoJSONVTData` instance was retained at the scheduled lambda, which run on the worker thread represented by the `GeoJSONVTData::scheduler` class member: ``` std::weak_ptr<GeoJSONVTData> weak = shared_from_this(); scheduler->scheduleAndReplyValue( [id, weak, this]() -> TileFeatures { if (auto self = weak.lock()) { return impl.getTile(id.z, id.x, id.y).features; } return {}; }, fn); ``` It caused program termination in case `self` turned to be the last reference to `this`, as the `std::thread` destructor was called from the thread it represented. Now, only the `GeoJSONVTData::impl` class member is retained.
-rw-r--r--src/mbgl/style/sources/geojson_source_impl.cpp17
-rw-r--r--src/mbgl/util/thread_pool.hpp1
2 files changed, 7 insertions, 11 deletions
diff --git a/src/mbgl/style/sources/geojson_source_impl.cpp b/src/mbgl/style/sources/geojson_source_impl.cpp
index 9a883ba96d..e73876b429 100644
--- a/src/mbgl/style/sources/geojson_source_impl.cpp
+++ b/src/mbgl/style/sources/geojson_source_impl.cpp
@@ -13,19 +13,12 @@
namespace mbgl {
namespace style {
-class GeoJSONVTData : public GeoJSONData, public std::enable_shared_from_this<GeoJSONVTData> {
+class GeoJSONVTData : public GeoJSONData {
public:
void getTile(const CanonicalTileID& id, const std::function<void(TileFeatures)>& fn) final {
assert(fn);
- std::weak_ptr<GeoJSONVTData> weak = shared_from_this();
scheduler->scheduleAndReplyValue(
- [id, weak, this]() -> TileFeatures {
- if (auto self = weak.lock()) {
- return impl.getTile(id.z, id.x, id.y).features;
- }
- return {};
- },
- fn);
+ [id, impl = this->impl]() -> TileFeatures { return impl->getTile(id.z, id.x, id.y).features; }, fn);
}
Features getChildren(const std::uint32_t) final { return {}; }
@@ -39,8 +32,10 @@ public:
private:
friend GeoJSONData;
GeoJSONVTData(const GeoJSON& geoJSON, const mapbox::geojsonvt::Options& options)
- : impl(geoJSON, options), scheduler(Scheduler::GetSequenced()) {}
- mapbox::geojsonvt::GeoJSONVT impl;
+ : impl(std::make_shared<mapbox::geojsonvt::GeoJSONVT>(geoJSON, options)),
+ scheduler(Scheduler::GetSequenced()) {}
+
+ std::shared_ptr<mapbox::geojsonvt::GeoJSONVT> impl; // Accessed on worker thread.
std::shared_ptr<Scheduler> scheduler;
};
diff --git a/src/mbgl/util/thread_pool.hpp b/src/mbgl/util/thread_pool.hpp
index e39c1abc73..9791ff9460 100644
--- a/src/mbgl/util/thread_pool.hpp
+++ b/src/mbgl/util/thread_pool.hpp
@@ -48,6 +48,7 @@ public:
~ThreadedScheduler() override {
terminate();
for (auto& thread : threads) {
+ assert(std::this_thread::get_id() != thread.get_id());
thread.join();
}
}