fix: secure Symbol Retained drawable lifetime and record investigations

src/mln/gfx/symbol_drawable_data.hpp

@@ -5,14 +5,19 @@
 #include <mln/util/tiny_unordered_map.hpp>
 
 #include <memory>
+#include <cstddef>
+#include <utility>
 
 namespace mln {
+class TileAtlasTextures;
 namespace style {
 enum class SymbolType : uint8_t;
 } // namespace style
 namespace gfx {
+class DrawableAtlasesTweaker;
 
 struct SymbolDrawableData : public DrawableData {
+    static constexpr std::size_t drawableType = 1;
     SymbolDrawableData(const bool isHalo_,
                        const bool bucketVariablePlacement_,
                        const style::SymbolType symbolType_,
@@ -20,7 +25,9 @@ struct SymbolDrawableData : public DrawableData {
                        const style::AlignmentType rotationAlignment_,
                        const style::SymbolPlacementType placement_,
                        const style::IconTextFitType textFit_,
-                       const bool isOffset_)
+                       const bool isOffset_,
+                       std::shared_ptr<TileAtlasTextures> atlases_,
+                       std::shared_ptr<DrawableAtlasesTweaker> atlasTweaker_)
         : isHalo(isHalo_),
           bucketVariablePlacement(bucketVariablePlacement_),
           symbolType(symbolType_),
@@ -28,7 +35,9 @@ struct SymbolDrawableData : public DrawableData {
           rotationAlignment(rotationAlignment_),
           placement(placement_),
           textFit(textFit_),
-          isOffset(isOffset_) {}
+          isOffset(isOffset_),
+          atlases(std::move(atlases_)),
+          atlasTweaker(std::move(atlasTweaker_)) {}
     ~SymbolDrawableData() override = default;
 
     const bool isHalo;
@@ -39,6 +48,10 @@ struct SymbolDrawableData : public DrawableData {
     const style::SymbolPlacementType placement;
     const style::IconTextFitType textFit;
     const bool isOffset;
+    // Paired at creation with the actual native atlas tweaker. Neither
+    // resource owns RenderTile; its current container is bound separately.
+    const std::shared_ptr<TileAtlasTextures> atlases;
+    const std::shared_ptr<DrawableAtlasesTweaker> atlasTweaker;
 };
 
 using UniqueSymbolDrawableData = std::unique_ptr<SymbolDrawableData>;

src/mln/renderer/layers/render_symbol_layer.cpp

@@ -34,6 +34,7 @@
 #include <mln/renderer/layers/collision_layer_tweaker.hpp>
 
 #include <set>
+#include <unordered_set>
 
 namespace mln {
 
@@ -195,14 +196,96 @@ bool RenderSymbolLayer::hasCrossfade() const {
     return false;
 }
 
+void RenderSymbolLayer::setRenderTiles(Immutable<std::vector<RenderTile>> owner, RenderTiles sorted) {
+    renderTilesOwner = std::move(owner);
+    renderTiles = std::move(sorted);
+    currentTileBindings.clear();
+    renderTilesSameSource = false;
+    if (!renderTilesOwner.get() || !renderTiles) return;
+
+    std::unordered_set<const RenderTile*> addresses;
+    addresses.reserve(renderTilesOwner->size());
+    for (const auto& tile : *renderTilesOwner) addresses.insert(&tile);
+    for (const auto& ref : *renderTiles) {
+        if (!addresses.contains(&ref.get())) {
+            // Do not dereference a reference from another prepare generation.
+            renderTiles.reset();
+            return;
+        }
+    }
+    for (const auto& ref : *renderTiles) {
+        const auto& tile = ref.get();
+        currentTileBindings.emplace(&tile, TileBinding{tile.getOverscaledTileID(), tile.getAtlasTextures()});
+    }
+    renderTilesSameSource = true;
+}
+
+bool RenderSymbolLayer::bindRenderTile(gfx::Drawable& drawable, const RenderTile& tile) {
+    const auto it = currentTileBindings.find(&tile);
+    if (!renderTilesSameSource || it == currentTileBindings.end() ||
+        !drawable.getTileID() || *drawable.getTileID() != it->second.id) return false;
+    drawable.setRenderTile(renderTilesOwner, &tile);
+    return true;
+}
+
+bool RenderSymbolLayer::rebindTileDrawables(const RenderTile& tile,
+                                           const std::shared_ptr<Bucket>& bucket,
+                                           const TileAtlasTexturesPtr& atlases) {
+    const auto binding = currentTileBindings.find(&tile);
+    if (!renderTilesSameSource || binding == currentTileBindings.end()) return false;
+    const auto& tileID = binding->second.id;
+    auto* group = static_cast<TileLayerGroup*>(layerGroup.get());
+    if (!group) return false;
+    const auto count = group->getDrawableCount(passes, tileID);
+    bool compatible = bucket && layerTweaker && atlases && binding->second.atlases == atlases;
+    group->visitDrawables(passes, tileID, [&](gfx::Drawable& drawable) {
+        if (drawable.getLayerTweaker() != layerTweaker || drawable.getBucket() != bucket ||
+            drawable.getType() != gfx::SymbolDrawableData::drawableType || !drawable.getData() ||
+            !drawable.getTileID() || *drawable.getTileID() != tileID ||
+            drawable.getRenderPass() != passes) {
+            compatible = false;
+            return;
+        }
+        const auto& data = static_cast<const gfx::SymbolDrawableData&>(*drawable.getData());
+        if (!data.atlasTweaker || data.atlases != atlases || drawable.getTweakers().size() != 1 ||
+            drawable.getTweakers().front() != data.atlasTweaker) compatible = false;
+        if (!atlases || (data.symbolType == SymbolType::Text ? !atlases->glyph : !atlases->icon) ||
+            (data.symbolType != SymbolType::Text && data.symbolType != SymbolType::IconRGBA &&
+             data.symbolType != SymbolType::IconSDF)) compatible = false;
+    });
+    if (!compatible || group->getDrawableCount(passes, tileID) != count) {
+        // updateTile() means ANY updated. Preflight the entire text/icon/halo
+        // group before changing any member, so partial reuse cannot survive.
+        removeTile(passes, tileID);
+        return false;
+    }
+    if (!count) return false;
+    group->visitDrawables(passes, tileID, [&](gfx::Drawable& drawable) {
+        if (!bindRenderTile(drawable, tile)) compatible = false;
+    });
+    if (!compatible || group->getDrawableCount(passes, tileID) != count) {
+        removeTile(passes, tileID);
+        return false;
+    }
+    return true;
+}
+
 void RenderSymbolLayer::prepare(const LayerPrepareParameters& params) {
-    renderTiles = params.source->getRenderTilesSortedByYPosition();
+    // Keep the Y order and fading tiles of the Symbol source path, while
+    // taking both views from the same completed source prepare.
+    auto owner = params.source->getRawRenderTiles();
+    auto sorted = params.source->getRenderTilesSortedByYPosition();
+    setRenderTiles(std::move(owner), std::move(sorted));
+    placementData.clear();
+    if (!renderTilesSameSource) {
+        renderTileIDs.clear();
+        removeAllDrawables();
+        return;
+    }
 
     updateRenderTileIDs();
     addRenderPassesFromTiles();
 
-    placementData.clear();
-
     for (const RenderTile& renderTile : *renderTiles) {
         auto* bucket = static_cast<SymbolBucket*>(renderTile.getBucket(*baseImpl));
         if (bucket && bucket->bucketLeaderID == getID() && static_cast<Bucket*>(bucket)->check(SYM_GUARD_LOC)) {
@@ -475,6 +558,53 @@ std::size_t RenderSymbolLayer::removeAllDrawables() {
     return stats.drawablesRemoved - oldValue;
 }
 
+bool RenderSymbolLayer::appendRetainedTileResult(SymbolRenderResult& result, const gfx::Drawable& drawable) const {
+    // Only compare a possibly old pointer as an address. IDs were copied while
+    // source prepare owned a live Tile; never call through the old RenderTile.
+    const auto it = currentTileBindings.find(drawable.getRenderTile());
+    if (!renderTilesSameSource || it == currentTileBindings.end() || !drawable.getTileID() ||
+        *drawable.getTileID() != it->second.id ||
+        drawable.getType() != gfx::SymbolDrawableData::drawableType || !drawable.getData()) {
+        result.complete = false;
+        return false;
+    }
+    const auto& data = static_cast<const gfx::SymbolDrawableData&>(*drawable.getData());
+    const auto& atlases = data.atlases;
+    if ((data.symbolType != SymbolType::Text && data.symbolType != SymbolType::IconRGBA &&
+         data.symbolType != SymbolType::IconSDF) ||
+        !atlases || atlases != it->second.atlases || !data.atlasTweaker ||
+        drawable.getTweakers().size() != 1 || drawable.getTweakers().front() != data.atlasTweaker ||
+        !drawable.getShader()) {
+        result.complete = false;
+        return false;
+    }
+    const auto& expected = data.symbolType == SymbolType::Text ? atlases->glyph : atlases->icon;
+    if (!expected || expected->needsUpload() ||
+        drawable.getTexture(shaders::idSymbolImageTexture) != expected ||
+        (drawable.getShader()->getSamplerLocation(shaders::idSymbolImageIconTexture) &&
+         (!atlases->icon || atlases->icon->needsUpload() ||
+          drawable.getTexture(shaders::idSymbolImageIconTexture) != atlases->icon))) {
+        result.complete = false;
+        return false;
+    }
+    result.retain(atlases);
+    result.retain(atlases->glyph);
+    result.retain(atlases->icon);
+    for (std::size_t id = 0; id < shaders::maxTextureCountPerShader; ++id) {
+        const auto& texture = drawable.getTexture(id);
+        if (texture && texture->needsUpload()) {
+            result.complete = false;
+            return false;
+        }
+        result.retain(texture);
+        if (texture) {
+            result.append(texture->getSize());
+            result.append(texture->getFormat());
+        }
+    }
+    return result.complete;
+}
+
 SymbolRenderResult RenderSymbolLayer::retainedRenderResult(float fadeChange, uint64_t frame) {
     SymbolRenderResult result;
     result.append(layerIndex);
@@ -483,7 +613,13 @@ SymbolRenderResult RenderSymbolLayer::retainedRenderResult(float fadeChange, uin
     // A hidden layer is admitted only after processChanges has removed both
     // groups (the orchestrator verifies registry membership separately).
     if (!isLayerRenderable()) return result;
-    if (collisionTileLayerGroup && !collisionTileLayerGroup->empty()) result.complete = false;
+    if (!renderTilesSameSource || (collisionTileLayerGroup && !collisionTileLayerGroup->empty())) {
+        result.complete = false;
+        return result;
+    }
+    // Keep TileRenderData/LayoutResult (including atlas slots) alive across
+    // completed Full evidence. The per-prepare owner address is NOT an input.
+    result.resources.emplace_back(std::make_shared<Immutable<std::vector<RenderTile>>>(renderTilesOwner));
     if (!layerGroup || layerGroup->empty()) {
         // Empty is provable only if there are no renderable Symbol buckets.
         if (renderTiles) for (const auto& tileRef : *renderTiles) {
@@ -501,7 +637,10 @@ SymbolRenderResult RenderSymbolLayer::retainedRenderResult(float fadeChange, uin
     // RenderSymbolLayer is final and creates only this tweaker and the native
     // atlas tweaker. Unknown placement-capable layers never call this method.
     const auto& tweaker = static_cast<const SymbolLayerTweaker&>(*layerTweaker);
-    if (tweaker.getRenderResultFrame() != frame) result.complete = false;
+    if (tweaker.getRenderResultFrame() != frame) {
+        result.complete = false;
+        return result;
+    }
     result.appendResult(tweaker.getRenderResult());
     result.retain(layerGroup);
     result.append(layerGroup->getEnabled());
@@ -509,12 +648,14 @@ SymbolRenderResult RenderSymbolLayer::retainedRenderResult(float fadeChange, uin
     std::size_t visited = 0;
     visitLayerGroupDrawables(*layerGroup, [&](gfx::Drawable& drawable) {
         ++visited;
+        if (!result.complete) return;
         if (drawable.getIsCustom() || drawable.getLayerTweaker() != layerTweaker ||
             !drawable.getBucket() || !drawable.getTileID() || !drawable.getData() ||
             !drawable.getShader() || drawable.getTweakers().size() != 1) {
             result.complete = false;
             return;
         }
+        if (!appendRetainedTileResult(result, drawable)) return;
         result.append(drawable.getID());
         if (!SymbolShaderProvenance::get().supports(*drawable.getShader())) result.complete = false;
         result.retain(drawable.getTweakers().front()); // immutable native atlas/sampler policy
@@ -588,29 +729,6 @@ SymbolRenderResult RenderSymbolLayer::retainedRenderResult(float fadeChange, uin
                 if (indices->empty()) result.complete = false;
             }
         }
-        const auto* tile = drawable.getRenderTile();
-        const auto& atlases = tile ? tile->getAtlasTextures() : TileAtlasTexturesPtr{};
-        if (!atlases) {
-            result.complete = false;
-            return;
-        }
-        result.retain(atlases);
-        result.retain(atlases->glyph);
-        result.retain(atlases->icon);
-        for (std::size_t id = 0; id < shaders::maxTextureCountPerShader; ++id) {
-            const auto& texture = drawable.getTexture(id);
-            result.retain(texture);
-            if (texture) {
-                result.append(texture->getSize());
-                result.append(texture->getFormat());
-                if (texture->needsUpload()) result.complete = false;
-            }
-        }
-        const auto& expected = data.symbolType == SymbolType::Text ? atlases->glyph : atlases->icon;
-        if (!expected || expected->needsUpload() ||
-            drawable.getTexture(shaders::idSymbolImageTexture) != expected) result.complete = false;
-        if (drawable.getShader()->getSamplerLocation(shaders::idSymbolImageIconTexture) &&
-            drawable.getTexture(shaders::idSymbolImageIconTexture) != atlases->icon) result.complete = false;
     });
     if (visited != layerGroup->getDrawableCount()) result.complete = false;
     return result;
@@ -623,7 +741,7 @@ void RenderSymbolLayer::update(gfx::ShaderRegistry& shaders,
                                const PaintParameters&,
                                const RenderTree&,
                                UniqueChangeRequestVec& changes) {
-    if (!renderTiles || renderTiles->empty() || passes == RenderPass::None) {
+    if (!renderTilesSameSource || !renderTiles || renderTiles->empty() || passes == RenderPass::None) {
         removeAllDrawables();
         return;
     }
@@ -732,6 +850,12 @@ void RenderSymbolLayer::update(gfx::ShaderRegistry& shaders,
         const auto& renderData = *optRenderData;
         const auto& bucket = static_cast<const SymbolBucket&>(*renderData.bucket);
         const auto& evaluated = getEvaluated<SymbolLayerProperties>(renderData.layerProperties);
+        const auto& atlases = tile.getAtlasTextures();
+        if (!atlases || (bucket.hasTextData() && !atlases->glyph) ||
+            ((bucket.hasIconData() || bucket.hasSdfIconData() || bucket.iconsInText) && !atlases->icon)) {
+            removeTile(passes, tileID);
+            continue;
+        }
 
         const auto prevBucketID = getRenderTileBucketID(tileID);
         if (prevBucketID != util::SimpleIdentity::Empty && prevBucketID != bucket.getID()) {
@@ -807,18 +931,14 @@ void RenderSymbolLayer::update(gfx::ShaderRegistry& shaders,
         // If we already have drawables for this tile, update them.
         // Just update the drawables we already created
         auto updateExisting = [&](gfx::Drawable& drawable) {
-            if (drawable.getLayerTweaker() != layerTweaker) {
-                // This drawable was produced on a previous style/bucket, and should not be updated.
-                return false;
-            }
-
             propertiesAsUniforms.first.clear();
             propertiesAsUniforms.second.clear();
 
             updateTileDrawable(drawable, bucket, bucketPaintProperties, evaluated);
             return true;
         };
-        if (updateTile(passes, tileID, std::move(updateExisting))) {
+        if (rebindTileDrawables(tile, renderData.bucket, atlases) &&
+            updateTile(passes, tileID, std::move(updateExisting))) {
             // re-create collision drawables
             if (collisionTileLayerGroup) {
                 collisionTileLayerGroup->removeDrawables(passes, tileID);
@@ -855,11 +975,6 @@ void RenderSymbolLayer::update(gfx::ShaderRegistry& shaders,
             }
         };
 
-        const auto& atlases = tile.getAtlasTextures();
-        if (!atlases) {
-            continue;
-        }
-
         if (bucket.hasIconData() && atlases->icon) {
             addRenderables(bucket.icon, SymbolType::IconRGBA);
         }
@@ -879,7 +994,7 @@ void RenderSymbolLayer::update(gfx::ShaderRegistry& shaders,
     using RawVertexVec = std::vector<std::uint8_t>; // raw buffer for vertexes of <int16_t, 4>
     struct TileInfo {
         RawVertexVec textVertices, iconVertices;
-        gfx::DrawableTweakerPtr textTweaker, iconTweaker;
+        std::shared_ptr<gfx::DrawableAtlasesTweaker> textTweaker, iconTweaker;
     };
 
     mln::unordered_map<UnwrappedTileID, TileInfo> tileCache;
@@ -1031,13 +1146,14 @@ void RenderSymbolLayer::update(gfx::ShaderRegistry& shaders,
             for (auto& drawable : builder->clearDrawables()) {
                 drawable->setTileID(tileID);
                 drawable->setLayerTweaker(layerTweaker);
-                drawable->setRenderTile(renderTilesOwner, &tile);
+                if (!bindRenderTile(*drawable, tile)) continue;
 
                 PaintPropertyBindersBase& textBinders = bucketPaintProperties.textBinders;
                 PaintPropertyBindersBase& iconBinders = bucketPaintProperties.iconBinders;
                 drawable->setBinders(renderable.renderData.bucket, isText ? &textBinders : &iconBinders);
 
                 const bool hasVariablePlacement = bucket.hasVariablePlacement;
+                drawable->setType(gfx::SymbolDrawableData::drawableType);
                 drawable->setData(std::make_unique<gfx::SymbolDrawableData>(
                     /*.isHalo=*/isHalo,
                     /*.bucketVaraiblePlacement=*/hasVariablePlacement,
@@ -1046,7 +1162,9 @@ void RenderSymbolLayer::update(gfx::ShaderRegistry& shaders,
                     /*.rotationAlignment=*/values.rotationAlignment,
                     /*.placement=*/bucketLayout.get<SymbolPlacement>(),
                     /*.textFit=*/bucketLayout.get<IconTextFit>(),
-                    /*.isOffset=*/isOffset));
+                    /*.isOffset=*/isOffset,
+                    atlases,
+                    isText ? tileInfo.textTweaker : tileInfo.iconTweaker));
 
                 tileLayerGroup->addDrawable(passes, tileID, std::move(drawable));
                 ++stats.drawablesAdded;

src/mln/renderer/layers/render_symbol_layer.hpp

@@ -11,6 +11,7 @@
 #include <unordered_map>
 
 namespace mln {
+class TileAtlasTextures;
 
 namespace style {
 
@@ -82,6 +83,20 @@ protected:
     std::size_t removeTile(RenderPass, const OverscaledTileID &) override;
 
 private:
+    friend struct SymbolRetainedLifetimeTestAccess;
+    // Validate addresses before reading any reference-based tile input.
+    void setRenderTiles(Immutable<std::vector<RenderTile>>, RenderTiles);
+    bool bindRenderTile(gfx::Drawable&, const RenderTile&);
+    bool appendRetainedTileResult(SymbolRenderResult&, const gfx::Drawable&) const;
+    bool rebindTileDrawables(const RenderTile&, const std::shared_ptr<Bucket>&,
+                             const std::shared_ptr<TileAtlasTextures>&);
+    struct TileBinding {
+        OverscaledTileID id;
+        std::shared_ptr<TileAtlasTextures> atlases;
+    };
+    std::unordered_map<const RenderTile*, TileBinding> currentTileBindings;
+    bool renderTilesSameSource = false;
+
     void transition(const TransitionParameters &) override;
     void evaluate(const PropertyEvaluationParameters &) override;
     bool hasTransition() const override;