fix: make render orchestrator teardown safe

test/renderer/render_orchestrator_error.test.cpp

@@ -4,13 +4,29 @@
 #include <mbgl/renderer/render_orchestrator.hpp>
 #include <mbgl/renderer/renderer_observer.hpp>
 #include <mbgl/renderer/retained_underlay_cache.hpp>
+#include <mbgl/renderer/render_source.hpp>
+#include <mbgl/renderer/render_source_observer.hpp>
+#include <mbgl/renderer/tile_render_data.hpp>
+#include <mbgl/style/sources/geojson_source_impl.hpp>
+#include <mbgl/tile/tile.hpp>
+#include <mbgl/tile/tile_observer.hpp>
 
 #include <gtest/gtest.h>
 
+#include <atomic>
 #include <stdexcept>
 
 using namespace mbgl;
 
+namespace mbgl {
+struct RenderOrchestratorTeardownTestAccess {
+    static void add(RenderOrchestrator& orchestrator, std::unique_ptr<RenderSource> source) {
+        source->setObserver(&orchestrator);
+        orchestrator.renderSources.emplace(source->baseImpl->id, std::move(source));
+    }
+};
+} // namespace mbgl
+
 namespace {
 
 class TestUniformBuffers final : public gfx::UniformBufferArray {
@@ -108,3 +124,128 @@ TEST(RenderOrchestratorError, FailedRangeCannotReuseAndCleanRetryCanRecover) {
     cache.markBuildSucceeded();
     EXPECT_EQ(cache.evaluate(key, 1, true), RetainedUnderlayDecision::Reuse);
 }
+
+namespace {
+class TeardownTile final : public Tile {
+public:
+    TeardownTile(TileObserver* observer_)
+        : Tile(Kind::Geometry, OverscaledTileID{0, 0, {0, 0, 0}}, "step101-source", observer_) {}
+
+    ~TeardownTile() override { observer->onTileAction(id, sourceID, TileOperation::Cancelled); }
+    std::unique_ptr<TileRenderData> createRenderData() override { return nullptr; }
+    void cancel() override {}
+    bool layerPropertiesUpdated(const Immutable<style::LayerProperties>&) override { return true; }
+    void emitChanged() { observer->onTileChanged(*this); }
+    void emitError() { observer->onTileError(*this, std::make_exception_ptr(std::runtime_error("tile error"))); }
+    void emitAction() { observer->onTileAction(id, sourceID, TileOperation::Cancelled); }
+};
+
+class TeardownSource final : public RenderSource {
+public:
+    TeardownSource(const TaggedScheduler& scheduler_, std::atomic<int>& deferred_, std::atomic<int>& renderJobs_,
+                   std::atomic<int>& detached_)
+        : RenderSource(makeMutable<style::GeoJSONSource::Impl>(
+              "step101-source", style::GeoJSONOptions::defaultOptions())),
+          scheduler(scheduler_), deferred(deferred_), renderJobs(renderJobs_), detached(detached_),
+          tile(std::make_unique<TeardownTile>(static_cast<TileObserver*>(this))) {}
+
+    ~TeardownSource() override {
+        if (observer != oldObserver) ++detached;
+        tile.reset(); // A pending tile emits Cancelled synchronously during source destruction.
+        scheduler.schedule([&count = deferred] { ++count; });
+        scheduler.runOnRenderThread([&count = renderJobs] { ++count; });
+    }
+
+    void expectDetachFrom(RenderSourceObserver* observer_) { oldObserver = observer_; }
+
+    void emitAll() {
+        tile->emitChanged();
+        tile->emitError();
+        tile->emitAction();
+    }
+    bool isLoaded() const override { return false; }
+    void update(Immutable<style::Source::Impl>, const std::vector<Immutable<style::LayerProperties>>&,
+                bool, bool, const TileParameters&) override {}
+    std::unique_ptr<RenderItem> createRenderItem() override { return nullptr; }
+    void prepare(const SourcePrepareParameters&) override {}
+    void updateFadingTiles() override {}
+    bool hasFadingTiles() const override { return false; }
+    RenderSourceReadinessStats getReadinessDiagnostics() const override {
+        return {.tileBacked = true,
+                .tileCount = std::nullopt,
+                .incompleteTileCount = std::nullopt,
+                .loadedTileCount = std::nullopt,
+                .hasFadingTiles = false};
+    }
+    std::unordered_map<std::string, std::vector<Feature>> queryRenderedFeatures(
+        const ScreenLineString&, const TransformState&,
+        const std::unordered_map<std::string, const RenderLayer*>&,
+        const RenderedQueryOptions&, const mat4&) const override { return {}; }
+    std::vector<Feature> querySourceFeatures(const SourceQueryOptions&) const override { return {}; }
+    void reduceMemoryUse() override {}
+    void dumpDebugLogs() const override {}
+
+private:
+    TaggedScheduler scheduler;
+    std::atomic<int>& deferred;
+    std::atomic<int>& renderJobs;
+    std::atomic<int>& detached;
+    RenderSourceObserver* oldObserver = nullptr;
+    std::unique_ptr<TeardownTile> tile;
+};
+
+class CountingSourceObserver final : public RenderSourceObserver {
+public:
+    void onTileChanged(RenderSource&, const OverscaledTileID&) override { ++changed; }
+    void onTileError(RenderSource&, const OverscaledTileID&, std::exception_ptr) override { ++errors; }
+    void onTileAction(RenderSource&, TileOperation, const OverscaledTileID&, const std::string&) override { ++actions; }
+    int changed = 0;
+    int errors = 0;
+    int actions = 0;
+};
+} // namespace
+
+TEST(RenderOrchestratorTeardown, NullObserverAndDetachCoverEveryTileCallback) {
+    TaggedScheduler scheduler{Scheduler::GetBackground(), {}};
+    std::atomic<int> deferred{0};
+    std::atomic<int> renderJobs{0};
+    std::atomic<int> detached{0};
+    auto source = std::make_unique<TeardownSource>(scheduler, deferred, renderJobs, detached);
+    source->emitAll(); // Constructor default NullObserver.
+
+    CountingSourceObserver observer;
+    source->setObserver(&observer);
+    source->expectDetachFrom(&observer);
+    source->emitAll();
+    EXPECT_EQ(observer.changed, 1);
+    EXPECT_EQ(observer.errors, 1);
+    EXPECT_EQ(observer.actions, 1);
+
+    source->setObserver(nullptr);
+    source->emitAll();
+    source.reset(); // Cancelled must not reach the detached observer.
+    EXPECT_EQ(detached.load(), 1);
+    EXPECT_EQ(observer.changed, 1);
+    EXPECT_EQ(observer.errors, 1);
+    EXPECT_EQ(observer.actions, 1);
+    scheduler.waitForEmpty();
+    scheduler.runRenderJobs();
+}
+
+TEST(RenderOrchestratorTeardown, PendingTileCancellationAndSecondDrainRepeatSafely) {
+    TaggedScheduler scheduler{Scheduler::GetBackground(), {}};
+    std::atomic<int> deferred{0};
+    std::atomic<int> renderJobs{0};
+    std::atomic<int> detached{0};
+    for (int iteration = 0; iteration < 20; ++iteration) {
+        auto orchestrator = std::make_unique<RenderOrchestrator>(false, scheduler, std::nullopt);
+        auto source = std::make_unique<TeardownSource>(scheduler, deferred, renderJobs, detached);
+        source->expectDetachFrom(orchestrator.get());
+        RenderOrchestratorTeardownTestAccess::add(*orchestrator, std::move(source));
+        orchestrator.reset();
+        EXPECT_EQ(detached.load(), iteration + 1); // Source was detached before its tile cancelled.
+        EXPECT_EQ(deferred.load(), iteration + 1); // Work queued by source destruction was drained.
+        scheduler.runRenderJobs(); // Render jobs have a separate backend-owned queue.
+        EXPECT_EQ(renderJobs.load(), iteration + 1);
+    }
+}

test/renderer/retained_underlay_cache.test.cpp

@@ -387,6 +387,8 @@ TEST(RetainedUnderlayRevision, EquivalentIntraFrameRemoveAddDoesNotAdvanceTopolo
     const RetainedUnderlayTopologySnapshot initial{
         .boundary = 32,
         .staticPrefix = {{3, "sea-area-fill", 1}, {31, "clip-outline", 1}},
+        .continuousEndExclusive = std::nullopt,
+        .staticSuffix = {},
     };
     const auto settledRevision = tracker.observe(initial);
 
@@ -404,6 +406,8 @@ TEST(RetainedUnderlayRevision, PersistentTopologyChangeAdvancesOnce) {
     const RetainedUnderlayTopologySnapshot initial{
         .boundary = 32,
         .staticPrefix = {{3, "sea-area-fill", 1}, {31, "clip-outline", 1}},
+        .continuousEndExclusive = std::nullopt,
+        .staticSuffix = {},
     };
     const auto initialRevision = tracker.observe(initial);
 
@@ -419,6 +423,8 @@ TEST(RetainedUnderlayRevision, OrderingAndIndexChangesAdvanceTopology) {
     RetainedUnderlayTopologySnapshot topology{
         .boundary = 32,
         .staticPrefix = {{3, "sea-area-fill", 1}, {31, "clip-outline", 1}},
+        .continuousEndExclusive = std::nullopt,
+        .staticSuffix = {},
     };
     const auto initialRevision = tracker.observe(topology);
 
@@ -992,7 +998,9 @@ TEST(RenderSegmentDiagnostics, ReadinessSnapshotMapsSourcesToSegmentsWithoutSide
                               .contentRevision = 42,
                               .eventType = "TileMutation",
                               .reason = "tile-mutation",
-                              .sourceId = "dynamic"};
+                              .sourceId = "dynamic",
+                              .affectedSegmentIndices = {},
+                              .dependencyScope = "none"};
     state.recordRendererReadiness(readiness);
 
     const auto first = state.renderSegmentSnapshot();