[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.
This commit is contained in:
Mikhail Pozdnyakov
2020-01-09 15:50:48 +02:00
parent 236afc0395
commit 3ea459233a
2 changed files with 7 additions and 11 deletions

View File

@@ -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;
};

View File

@@ -48,6 +48,7 @@ public:
~ThreadedScheduler() override {
terminate();
for (auto& thread : threads) {
assert(std::this_thread::get_id() != thread.get_id());
thread.join();
}
}