diff --git a/include/mbgl/style/layer.hpp b/include/mbgl/style/layer.hpp index 36e8db174..4db877e11 100644 --- a/include/mbgl/style/layer.hpp +++ b/include/mbgl/style/layer.hpp @@ -10,9 +10,8 @@ #include #include -#include -#include #include +#include namespace mbgl { namespace style { @@ -109,6 +108,8 @@ bool layerTypeInfoEquals(const mbgl::style::LayerTypeInfo* one, const mbgl::styl * create an instance of the desired type, calling `LayerManager`: * * auto circleLayer = LayerManager::get()->createLayer("circle", ...); + * + * NOTE: Derived class must invalidate `weakFactory` in their destructor */ class Layer { public: diff --git a/include/mbgl/style/sources/custom_geometry_source.hpp b/include/mbgl/style/sources/custom_geometry_source.hpp index 6dabe5e6e..8a7e918ef 100644 --- a/include/mbgl/style/sources/custom_geometry_source.hpp +++ b/include/mbgl/style/sources/custom_geometry_source.hpp @@ -20,6 +20,7 @@ using TileFunction = std::function; class CustomTileLoader; +// NOTE: Any derived class must invalidate `weakFactory` in the destructor class CustomGeometrySource final : public Source { public: struct TileOptions { diff --git a/include/mbgl/style/sources/geojson_source.hpp b/include/mbgl/style/sources/geojson_source.hpp index 0958ebd78..4b53b6f16 100644 --- a/include/mbgl/style/sources/geojson_source.hpp +++ b/include/mbgl/style/sources/geojson_source.hpp @@ -54,6 +54,7 @@ public: virtual std::uint8_t getClusterExpansionZoom(std::uint32_t) = 0; }; +// NOTE: Any derived class must invalidate `weakFactory` in the destructor class GeoJSONSource final : public Source { public: GeoJSONSource(std::string id, Immutable = GeoJSONOptions::defaultOptions()); diff --git a/include/mbgl/style/sources/image_source.hpp b/include/mbgl/style/sources/image_source.hpp index db139ab37..99c7de7bc 100644 --- a/include/mbgl/style/sources/image_source.hpp +++ b/include/mbgl/style/sources/image_source.hpp @@ -11,6 +11,7 @@ class AsyncRequest; namespace style { +// NOTE: Any derived class must invalidate `weakFactory` in the destructor class ImageSource final : public Source { public: ImageSource(std::string id, std::array); diff --git a/include/mbgl/style/sources/raster_dem_source.hpp b/include/mbgl/style/sources/raster_dem_source.hpp index 7f1e01fd7..a2a612eac 100644 --- a/include/mbgl/style/sources/raster_dem_source.hpp +++ b/include/mbgl/style/sources/raster_dem_source.hpp @@ -5,14 +5,14 @@ #include namespace mbgl { - namespace style { struct RasterDEMOptions { std::optional encoding = std::nullopt; }; -class RasterDEMSource : public RasterSource { +// NOTE: Any derived class must invalidate `weakFactory` in the destructor +class RasterDEMSource final : public RasterSource { public: RasterDEMSource(std::string id, variant urlOrTileset, diff --git a/include/mbgl/style/sources/raster_source.hpp b/include/mbgl/style/sources/raster_source.hpp index 15538e7ce..42cd8ddd0 100644 --- a/include/mbgl/style/sources/raster_source.hpp +++ b/include/mbgl/style/sources/raster_source.hpp @@ -5,7 +5,6 @@ #include namespace mbgl { - namespace style { class RasterSource : public TileSource { diff --git a/include/mbgl/style/sources/vector_source.hpp b/include/mbgl/style/sources/vector_source.hpp index 4448291fd..e02f8e75b 100644 --- a/include/mbgl/style/sources/vector_source.hpp +++ b/include/mbgl/style/sources/vector_source.hpp @@ -10,6 +10,7 @@ class AsyncRequest; namespace style { +// NOTE: Any derived class must invalidate `weakFactory` in the destructor class VectorSource final : public TileSource { public: VectorSource(std::string id, diff --git a/include/mbgl/util/run_loop.hpp b/include/mbgl/util/run_loop.hpp index 755f4aeb7..7ab249cee 100644 --- a/include/mbgl/util/run_loop.hpp +++ b/include/mbgl/util/run_loop.hpp @@ -7,18 +7,18 @@ #include #include -#include #include -#include -#include #include +#include +#include namespace mbgl { namespace util { using LOOP_HANDLE = void*; -class RunLoop : public Scheduler, private util::noncopyable { +// NOTE: Any derived class must invalidate `weakFactory` in the destructor +class RunLoop final : public Scheduler, private util::noncopyable { public: enum class Type : uint8_t { Default, @@ -88,6 +88,10 @@ public: class Impl; + // Allows derived classes to invalidate weak pointers in + // their destructor before their own members are torn down. + void invalidateWeakPtrsEarly() { weakFactory.invalidateWeakPtrs(); } + private: MBGL_STORE_THREAD(tid) diff --git a/platform/android/MapLibreAndroid/src/cpp/map_renderer.hpp b/platform/android/MapLibreAndroid/src/cpp/map_renderer.hpp index 84ed971c4..0d68818a4 100644 --- a/platform/android/MapLibreAndroid/src/cpp/map_renderer.hpp +++ b/platform/android/MapLibreAndroid/src/cpp/map_renderer.hpp @@ -37,8 +37,10 @@ public: * performed on the GL Thread. * * The public methods are safe to call from the main thread, others are not. + * + * NOTE: Any derived class must invalidate `weakFactory` in the destructor */ -class MapRenderer : public Scheduler { +class MapRenderer final : public Scheduler { public: static constexpr auto Name() { return "org/maplibre/android/maps/renderer/MapRenderer"; }; @@ -95,6 +97,10 @@ protected: void scheduleSnapshot(std::unique_ptr); + // Allows derived classes to invalidate weak pointers in + // their destructor before their own members are torn down. + void invalidateWeakPtrsEarly() { weakFactory.invalidateWeakPtrs(); } + private: struct MailboxData { explicit MailboxData(Scheduler*); diff --git a/src/mbgl/annotation/annotation_manager.hpp b/src/mbgl/annotation/annotation_manager.hpp index fa3b9dbf8..e1451a970 100644 --- a/src/mbgl/annotation/annotation_manager.hpp +++ b/src/mbgl/annotation/annotation_manager.hpp @@ -5,13 +5,12 @@ #include #include +#include + #include #include -#include -#include #include - -#include +#include namespace mbgl { @@ -25,7 +24,8 @@ namespace style { class Style; } // namespace style -class AnnotationManager : private util::noncopyable { +// NOTE: Any derived class must invalidate `weakFactory` in the destructor +class AnnotationManager final : private util::noncopyable { public: AnnotationManager(style::Style&); ~AnnotationManager(); diff --git a/src/mbgl/annotation/annotation_source.hpp b/src/mbgl/annotation/annotation_source.hpp index 721f08ebd..471e2348d 100644 --- a/src/mbgl/annotation/annotation_source.hpp +++ b/src/mbgl/annotation/annotation_source.hpp @@ -5,6 +5,7 @@ namespace mbgl { +// NOTE: Any derived class must invalidate `weakFactory` in the destructor class AnnotationSource final : public style::Source { public: AnnotationSource(); diff --git a/src/mbgl/renderer/render_source.hpp b/src/mbgl/renderer/render_source.hpp index 80a8dbe6d..334bd1bc8 100644 --- a/src/mbgl/renderer/render_source.hpp +++ b/src/mbgl/renderer/render_source.hpp @@ -11,10 +11,10 @@ #include #include -#include -#include #include #include +#include +#include namespace mbgl { diff --git a/src/mbgl/sprite/sprite_loader.hpp b/src/mbgl/sprite/sprite_loader.hpp index 539f6a1e0..028c60bf8 100644 --- a/src/mbgl/sprite/sprite_loader.hpp +++ b/src/mbgl/sprite/sprite_loader.hpp @@ -7,9 +7,6 @@ #include #include -#include -#include -#include #include namespace mbgl { @@ -17,7 +14,8 @@ namespace mbgl { class FileSource; class SpriteLoaderObserver; -class SpriteLoader { +// NOTE: Any derived class must invalidate `weakFactory` in the destructor +class SpriteLoader final { public: SpriteLoader(float pixelRatio, const TaggedScheduler& threadPool_); ~SpriteLoader(); diff --git a/src/mbgl/style/layers/background_layer.cpp b/src/mbgl/style/layers/background_layer.cpp index 8f8fb1242..d1b515d00 100644 --- a/src/mbgl/style/layers/background_layer.cpp +++ b/src/mbgl/style/layers/background_layer.cpp @@ -39,7 +39,9 @@ BackgroundLayer::BackgroundLayer(Immutable impl_) : Layer(std::move(impl_)) { } -BackgroundLayer::~BackgroundLayer() = default; +BackgroundLayer::~BackgroundLayer() { + weakFactory.invalidateWeakPtrs(); +} const BackgroundLayer::Impl& BackgroundLayer::impl() const { return static_cast(*baseImpl); diff --git a/src/mbgl/style/layers/circle_layer.cpp b/src/mbgl/style/layers/circle_layer.cpp index fd6a979ad..edec68c4e 100644 --- a/src/mbgl/style/layers/circle_layer.cpp +++ b/src/mbgl/style/layers/circle_layer.cpp @@ -39,7 +39,9 @@ CircleLayer::CircleLayer(Immutable impl_) : Layer(std::move(impl_)) { } -CircleLayer::~CircleLayer() = default; +CircleLayer::~CircleLayer() { + weakFactory.invalidateWeakPtrs(); +} const CircleLayer::Impl& CircleLayer::impl() const { return static_cast(*baseImpl); diff --git a/src/mbgl/style/layers/fill_extrusion_layer.cpp b/src/mbgl/style/layers/fill_extrusion_layer.cpp index ccb2df981..3f4ff9cbc 100644 --- a/src/mbgl/style/layers/fill_extrusion_layer.cpp +++ b/src/mbgl/style/layers/fill_extrusion_layer.cpp @@ -39,7 +39,9 @@ FillExtrusionLayer::FillExtrusionLayer(Immutable impl_) : Layer(std::move(impl_)) { } -FillExtrusionLayer::~FillExtrusionLayer() = default; +FillExtrusionLayer::~FillExtrusionLayer() { + weakFactory.invalidateWeakPtrs(); +} const FillExtrusionLayer::Impl& FillExtrusionLayer::impl() const { return static_cast(*baseImpl); diff --git a/src/mbgl/style/layers/fill_layer.cpp b/src/mbgl/style/layers/fill_layer.cpp index 9bc0ae55f..2a0d81e23 100644 --- a/src/mbgl/style/layers/fill_layer.cpp +++ b/src/mbgl/style/layers/fill_layer.cpp @@ -39,7 +39,9 @@ FillLayer::FillLayer(Immutable impl_) : Layer(std::move(impl_)) { } -FillLayer::~FillLayer() = default; +FillLayer::~FillLayer() { + weakFactory.invalidateWeakPtrs(); +} const FillLayer::Impl& FillLayer::impl() const { return static_cast(*baseImpl); diff --git a/src/mbgl/style/layers/heatmap_layer.cpp b/src/mbgl/style/layers/heatmap_layer.cpp index f0469c1b4..ba9ec0c89 100644 --- a/src/mbgl/style/layers/heatmap_layer.cpp +++ b/src/mbgl/style/layers/heatmap_layer.cpp @@ -39,7 +39,9 @@ HeatmapLayer::HeatmapLayer(Immutable impl_) : Layer(std::move(impl_)) { } -HeatmapLayer::~HeatmapLayer() = default; +HeatmapLayer::~HeatmapLayer() { + weakFactory.invalidateWeakPtrs(); +} const HeatmapLayer::Impl& HeatmapLayer::impl() const { return static_cast(*baseImpl); diff --git a/src/mbgl/style/layers/hillshade_layer.cpp b/src/mbgl/style/layers/hillshade_layer.cpp index 410979118..e1af91d64 100644 --- a/src/mbgl/style/layers/hillshade_layer.cpp +++ b/src/mbgl/style/layers/hillshade_layer.cpp @@ -39,7 +39,9 @@ HillshadeLayer::HillshadeLayer(Immutable impl_) : Layer(std::move(impl_)) { } -HillshadeLayer::~HillshadeLayer() = default; +HillshadeLayer::~HillshadeLayer() { + weakFactory.invalidateWeakPtrs(); +} const HillshadeLayer::Impl& HillshadeLayer::impl() const { return static_cast(*baseImpl); diff --git a/src/mbgl/style/layers/layer.cpp.ejs b/src/mbgl/style/layers/layer.cpp.ejs index eea2a85d4..9121c9622 100644 --- a/src/mbgl/style/layers/layer.cpp.ejs +++ b/src/mbgl/style/layers/layer.cpp.ejs @@ -129,7 +129,9 @@ const LayerTypeInfo* <%- camelize(type) %>Layer::Impl::staticTypeInfo() noexcept : Layer(std::move(impl_)) { } -<%- camelize(type) %>Layer::~<%- camelize(type) %>Layer() = default; +<%- camelize(type) %>Layer::~<%- camelize(type) %>Layer() { + weakFactory.invalidateWeakPtrs(); +} const <%- camelize(type) %>Layer::Impl& <%- camelize(type) %>Layer::impl() const { return static_cast(*baseImpl); diff --git a/src/mbgl/style/layers/line_layer.cpp b/src/mbgl/style/layers/line_layer.cpp index a18ba5776..ce13ac28b 100644 --- a/src/mbgl/style/layers/line_layer.cpp +++ b/src/mbgl/style/layers/line_layer.cpp @@ -39,7 +39,9 @@ LineLayer::LineLayer(Immutable impl_) : Layer(std::move(impl_)) { } -LineLayer::~LineLayer() = default; +LineLayer::~LineLayer() { + weakFactory.invalidateWeakPtrs(); +} const LineLayer::Impl& LineLayer::impl() const { return static_cast(*baseImpl); diff --git a/src/mbgl/style/layers/location_indicator_layer.cpp b/src/mbgl/style/layers/location_indicator_layer.cpp index 580018130..b443cec97 100644 --- a/src/mbgl/style/layers/location_indicator_layer.cpp +++ b/src/mbgl/style/layers/location_indicator_layer.cpp @@ -39,7 +39,9 @@ LocationIndicatorLayer::LocationIndicatorLayer(Immutable impl_) : Layer(std::move(impl_)) { } -LocationIndicatorLayer::~LocationIndicatorLayer() = default; +LocationIndicatorLayer::~LocationIndicatorLayer() { + weakFactory.invalidateWeakPtrs(); +} const LocationIndicatorLayer::Impl& LocationIndicatorLayer::impl() const { return static_cast(*baseImpl); diff --git a/src/mbgl/style/layers/raster_layer.cpp b/src/mbgl/style/layers/raster_layer.cpp index 092766536..ff787a9db 100644 --- a/src/mbgl/style/layers/raster_layer.cpp +++ b/src/mbgl/style/layers/raster_layer.cpp @@ -39,7 +39,9 @@ RasterLayer::RasterLayer(Immutable impl_) : Layer(std::move(impl_)) { } -RasterLayer::~RasterLayer() = default; +RasterLayer::~RasterLayer() { + weakFactory.invalidateWeakPtrs(); +} const RasterLayer::Impl& RasterLayer::impl() const { return static_cast(*baseImpl); diff --git a/src/mbgl/style/layers/symbol_layer.cpp b/src/mbgl/style/layers/symbol_layer.cpp index a55daf5dd..346335e36 100644 --- a/src/mbgl/style/layers/symbol_layer.cpp +++ b/src/mbgl/style/layers/symbol_layer.cpp @@ -39,7 +39,9 @@ SymbolLayer::SymbolLayer(Immutable impl_) : Layer(std::move(impl_)) { } -SymbolLayer::~SymbolLayer() = default; +SymbolLayer::~SymbolLayer() { + weakFactory.invalidateWeakPtrs(); +} const SymbolLayer::Impl& SymbolLayer::impl() const { return static_cast(*baseImpl); diff --git a/src/mbgl/style/sources/raster_dem_source.cpp b/src/mbgl/style/sources/raster_dem_source.cpp index 68745a048..7782e7a56 100644 --- a/src/mbgl/style/sources/raster_dem_source.cpp +++ b/src/mbgl/style/sources/raster_dem_source.cpp @@ -6,6 +6,7 @@ #include #include #include + #include namespace mbgl { diff --git a/src/mbgl/tile/geojson_tile.hpp b/src/mbgl/tile/geojson_tile.hpp index c9f46d861..e2dc1ceb3 100644 --- a/src/mbgl/tile/geojson_tile.hpp +++ b/src/mbgl/tile/geojson_tile.hpp @@ -10,7 +10,8 @@ class GeoJSONData; class TileParameters; -class GeoJSONTile : public GeometryTile { +// NOTE: Any derived class must invalidate `weakFactory` in the destructor +class GeoJSONTile final : public GeometryTile { public: GeoJSONTile(const OverscaledTileID&, std::string sourceID, diff --git a/src/mbgl/util/thread_pool.hpp b/src/mbgl/util/thread_pool.hpp index b16146661..58a199c3d 100644 --- a/src/mbgl/util/thread_pool.hpp +++ b/src/mbgl/util/thread_pool.hpp @@ -7,9 +7,7 @@ #include #include -#include #include -#include #include #include #include @@ -145,6 +143,11 @@ public: mapbox::base::WeakPtr makeWeakPtr() override { return weakFactory.makeWeakPtr(); } +protected: + // Allows derived classes to invalidate weak pointers in + // their destructor before their own members are torn down. + void invalidateWeakPtrsEarly() { weakFactory.invalidateWeakPtrs(); } + private: std::vector threads; @@ -159,22 +162,25 @@ private: // Do not add members here, see `WeakPtrFactory` }; -class SequencedScheduler : public ThreadedScheduler { +class SequencedScheduler final : public ThreadedScheduler { public: SequencedScheduler() : ThreadedScheduler(1) {} + ~SequencedScheduler() override { invalidateWeakPtrsEarly(); } }; class ParallelScheduler : public ThreadedScheduler { public: ParallelScheduler(std::size_t extra) : ThreadedScheduler(1 + extra) {} + ~ParallelScheduler() override { invalidateWeakPtrsEarly(); } }; -class ThreadPool : public ParallelScheduler { +class ThreadPool final : public ParallelScheduler { public: ThreadPool() : ParallelScheduler(3) {} + ~ThreadPool() override { invalidateWeakPtrsEarly(); } }; } // namespace mbgl diff --git a/test/actor/actor.test.cpp b/test/actor/actor.test.cpp index 406f788d7..dcb0fdd63 100644 --- a/test/actor/actor.test.cpp +++ b/test/actor/actor.test.cpp @@ -80,7 +80,8 @@ TEST(Actor, DestructionBlocksOnReceive) { TEST(Actor, DestructionBlocksOnSend) { // Destruction blocks until the actor is not being sent a message. - struct TestScheduler : public Scheduler { + // NOTE: Any derived class must invalidate `weakFactory` in the destructor + struct TestScheduler final : public Scheduler { std::promise promise; std::future future; std::atomic waited;