Use a pair of NSSets to manage the retain/release lifecycle of layers.

include/mbgl/style/layers/custom_layer.hpp

@@ -54,19 +54,32 @@ using CustomLayerContextLostFunction = void (*)(void* context);
  */
 using CustomLayerDeinitializeFunction = void (*)(void* context);
 
+/**
+ * Called from `CustomLayer`'s destructor.
+ * This provides a mechanism to handle any necessary clean-up using the provided `peer` object.
+ * For example, if a platform-native peer object has a raw pointer to the CustomLayer it could be
+ * set to NULL.
+ *
+ * This function is called from CustomLayer, unlike the above functions that are passed into the
+ * private implementation.
+ */
+using CustomLayerDeallocationFunction = void (*)(util::unique_any *peer);
+
 class CustomLayer : public Layer {
 public:
     CustomLayer(const std::string& id,
                 CustomLayerInitializeFunction,
                 CustomLayerRenderFunction,
                 CustomLayerContextLostFunction,
                 CustomLayerDeinitializeFunction,
+                CustomLayerDeallocationFunction,
                 void* context);
 
     CustomLayer(const std::string& id,
                 CustomLayerInitializeFunction,
                 CustomLayerRenderFunction,
                 CustomLayerDeinitializeFunction,
+                CustomLayerDeallocationFunction,
                 void* context);
 
     ~CustomLayer() final;
@@ -87,6 +100,8 @@ class CustomLayer : public Layer {
     std::unique_ptr<Layer> cloneRef(const std::string& id) const final;
 
     CustomLayer(const CustomLayer&) = delete;
+
+    CustomLayerDeallocationFunction deallocationFn = nullptr;
 };
 
 template <>

platform/darwin/src/MGLOpenGLStyleLayer.mm

@@ -15,10 +15,9 @@
     when creating an OpenGL style layer.
  */
 void MGLPrepareCustomStyleLayer(void *context) {
-
-    // Pair retain/release during rendering (see MGLFinishCustomStyleLayer)
-    id retaineee = (__bridge id)context;
-    MGLOpenGLStyleLayer *layer = (__bridge MGLOpenGLStyleLayer*)CFBridgingRetain(retaineee);
+    // Note, that the layer is retained/released by MGLStyle, ensuring that the layer
+    // is alive during rendering
+    MGLOpenGLStyleLayer *layer = (__bridge MGLOpenGLStyleLayer*)context;
 
     [layer didMoveToMapView:layer.style.mapView];
 }
@@ -51,11 +50,43 @@ void MGLDrawCustomStyleLayer(void *context, const mbgl::style::CustomLayerRender
     when creating an OpenGL style layer.
  */
 void MGLFinishCustomStyleLayer(void *context) {
-    // Release the layer (since we retained it in the initialization)
-    MGLOpenGLStyleLayer *layer = CFBridgingRelease(context);
+    // Note, that the layer is retained/released by MGLStyle, ensuring that the layer
+    // is alive during rendering
+    MGLOpenGLStyleLayer *layer = (__bridge MGLOpenGLStyleLayer*)context;
+
     [layer willMoveFromMapView:layer.style.mapView];
 }
 
+
+/**
+ Function to be called when the core `CustomLayer` (not the Impl) gets deallocated.
+ It's possible taht at this stage the Obj-C style layer is being deallocated (but that case is detected).
+ */
+void MGLDeallocateCustomStyleLayer(mbgl::util::unique_any *peer) {
+
+    // We know that the peer object contains a LayerWrapper with a weak pointer to
+    // our custom layer. We can use this to safely access the layer, and clear out the
+    // raw pointer.
+    //
+    // If we don't do this rawLayer can become a dangling pointer (which was previously being
+    // accessed via the description method)
+
+    if (!(peer && peer->has_value()))
+        return;
+
+    LayerWrapper *wrapper = mbgl::util::any_cast<LayerWrapper>(peer);
+
+    if (!wrapper)
+        return;
+
+    // If the MGLStyleLayer is currently being dealloc'd (and trigger the CustomLayer destructor, and
+    // this function) then layer here will be nil (even though wrapper->layer may appear to be non-nil)
+    MGLStyleLayer *layer = wrapper->layer;
+
+    layer.rawLayer = NULL;
+}
+
+
 /**
  An `MGLOpenGLStyleLayer` is a style layer that is rendered by OpenGL code that
  you provide.
@@ -107,7 +138,9 @@ - (instancetype)initWithIdentifier:(NSString *)identifier {
                                                             MGLPrepareCustomStyleLayer,
                                                             MGLDrawCustomStyleLayer,
                                                             MGLFinishCustomStyleLayer,
+                                                            MGLDeallocateCustomStyleLayer,
                                                             (__bridge void*)self);
+
     return self = [super initWithPendingLayer:std::move(layer)];
 }
 
@@ -127,11 +160,21 @@ - (void)setStyle:(MGLStyle *)style {
 
 - (void)addToStyle:(MGLStyle *)style belowLayer:(MGLStyleLayer *)otherLayer {
     self.style = style;
+
+    // We need to ensure that this layer is retained, so that any references from layer impl's
+    // e.g. contexts) are still valid
+    [style addToManagedLayers:self];
+
     [super addToStyle:style belowLayer:otherLayer];
 }
 
 - (void)removeFromStyle:(MGLStyle *)style {
     [super removeFromStyle:style];
+
+    // We need to ensure that this layer is now released (however, if this layer is about to be
+    // used by the renderer then it will released once rendering is complete)
+    [style removeFromManagedLayers:self];
+
     self.style = nil;
 }
 

platform/darwin/src/MGLStyle.mm

@@ -84,6 +84,7 @@ @interface MGLStyle()
 @property (nonatomic, readonly) mbgl::style::Style *rawStyle;
 @property (readonly, copy, nullable) NSURL *URL;
 @property (nonatomic) NS_MUTABLE_DICTIONARY_OF(NSString *, NS_DICTIONARY_OF(NSObject *, MGLTextLanguage *) *) *localizedLayersByIdentifier;
+@property (nonatomic, readwrite) NSMutableSet *managedLayers;
 
 @end
 
@@ -172,6 +173,7 @@ + (NSURL *)trafficNightStyleURLWithVersion:(NSInteger)version {
 
 - (instancetype)initWithRawStyle:(mbgl::style::Style *)rawStyle mapView:(MGLMapView *)mapView {
     if (self = [super init]) {
+        _managedLayers = [NSMutableSet set];
         _mapView = mapView;
         _rawStyle = rawStyle;
         _localizedLayersByIdentifier = [NSMutableDictionary dictionary];
@@ -534,6 +536,16 @@ - (void)insertLayer:(MGLStyleLayer *)layer aboveLayer:(MGLStyleLayer *)sibling {
     [self didChangeValueForKey:@"layers"];
 }
 
+#pragma mark - Layer retain/release management
+
+- (void)addToManagedLayers:(MGLStyleLayer*)layer {
+    [self.managedLayers addObject:layer];
+}
+
+- (void)removeFromManagedLayers:(MGLStyleLayer*)layer {
+    [self.managedLayers removeObject:layer];
+}
+
 #pragma mark Style classes
 
 - (NS_ARRAY_OF(NSString *) *)styleClasses

platform/darwin/src/MGLStyleLayer.mm

@@ -5,9 +5,7 @@
 #include <mbgl/style/layer.hpp>
 
 @interface MGLStyleLayer ()
-
-@property (nonatomic, readonly) mbgl::style::Layer *rawLayer;
-
+@property (nonatomic, readwrite, nullable) mbgl::style::Layer *rawLayer;
 @end
 
 @implementation MGLStyleLayer {
@@ -38,10 +36,6 @@ - (void)addToStyle:(MGLStyle *)style belowLayer:(MGLStyleLayer *)otherLayer
                     "to the style more than once is invalid.", self, style];
     }
 
-    // Since we're adding self to a C++ collection, we need to retain ourselves, so that we don't
-    // end up with a dangling pointer
-    CFBridgingRetain(self);
-
     if (otherLayer) {
         const mbgl::optional<std::string> belowLayerId{otherLayer.identifier.UTF8String};
         style.rawStyle->addLayer(std::move(_pendingLayer), belowLayerId);
@@ -54,10 +48,6 @@ - (void)removeFromStyle:(MGLStyle *)style
 {
     if (self.rawLayer == style.rawStyle->getLayer(self.identifier.UTF8String)) {
         _pendingLayer = style.rawStyle->removeLayer(self.identifier.UTF8String);
-
-        // Pair the retain above, and release self, since we're now removed from the collection
-        CFTypeRef toRelease = (__bridge CFTypeRef)self;
-        CFBridgingRelease(toRelease);
     }
 }
 
@@ -111,7 +101,14 @@ - (NSString *)description
 {
     return [NSString stringWithFormat:@"<%@: %p; identifier = %@; visible = %@>",
             NSStringFromClass([self class]), (void *)self, self.identifier,
-            self.visible ? @"YES" : @"NO"];
+            self.rawLayer ? (self.visible ? @"YES" : @"NO") : @"(No raw layer)"];
+}
+
+#pragma mark - Debug methods
+
+- (void)debugResetRawLayerPeer {
+    MGLAssertStyleLayerIsValid();
+    self.rawLayer->peer.reset();
 }
 
 @end

platform/darwin/src/MGLStyleLayerRetentionManager.mm

@@ -0,0 +1,53 @@
+#import "MGLStyle.h"
+#import "MGLStyleLayer.h"
+#import "MGLStyleLayerRetentionManager_Private.h"
+#include <mbgl/style/style.hpp>
+#include <mbgl/style/layers/custom_layer.hpp>
+
+static const NSUInteger MGLStyleLayerRetentionManagerCapacityHint = 100;
+
+@interface MGLStyleLayerRetentionManager ()
+@property (nonatomic) NSMapTable<MGLStyleLayer*, NSNumber*> *retentionTable;
+@end
+
+@implementation MGLStyleLayerRetentionManager
+
+- (instancetype)init {
+    if ((self = [super init])) {
+        _retentionTable = [[NSMapTable alloc] initWithKeyOptions:NSPointerFunctionsStrongMemory|NSPointerFunctionsObjectPointerPersonality
+                                                    valueOptions:NSPointerFunctionsStrongMemory
+                                                        capacity:MGLStyleLayerRetentionManagerCapacityHint];
+    }
+    return self;
+}
+
+- (BOOL)isManagedLayer:(nullable MGLStyleLayer*)styleLayer {
+    return [self.retentionTable objectForKey:styleLayer] != nil;
+}
+
+- (void)updateRetainedLayers:(nonnull NSSet<MGLStyleLayer*>*)sourceObjects {
+
+    // Add/update the objects from the source set, with a default lifetime
+    for (id layer in sourceObjects) {
+        [self.retentionTable setObject:@(MGLStyleLayerRetentionManagerDefaultLifetime) forKey:layer];
+    }
+}
+
+- (void)decrementLifetimes {
+    // Consider double-buffering the two tables, so we don't keep allocing/deallocing tables.
+    NSMapTable *retentionTable = [[NSMapTable alloc] initWithKeyOptions:NSPointerFunctionsStrongMemory|NSPointerFunctionsObjectPointerPersonality
+                                                           valueOptions:NSPointerFunctionsStrongMemory
+                                                               capacity:MGLStyleLayerRetentionManagerCapacityHint];
+
+    for (MGLStyleLayer *layer in self.retentionTable) {
+        NSInteger lifeTime = [[self.retentionTable objectForKey:layer] integerValue];
+
+        if (lifeTime > 0) {
+            [retentionTable setObject:@(lifeTime - 1) forKey:layer];
+        }
+    }
+
+    self.retentionTable = retentionTable;
+}
+
+@end

platform/darwin/src/MGLStyleLayerRetentionManager_Private.h

@@ -0,0 +1,16 @@
+@class MGLStyle;
+
+static const NSInteger MGLStyleLayerRetentionManagerDefaultLifetime = 2;
+
+/**
+ Object use to manage the retain/release of `MGLStyleLayer`s (currently only `MGLOpenGLStyleLayer`.
+ Managed layers are given a "lifetime", and this is reset everytime `-updateRetainedLayers:` is called.
+ The lifetime is decremented with each call to `-decrementLifetimes`, when it reaches 0 the layer
+ is removed from the manager (potentially releasing it)
+ */
+@interface MGLStyleLayerRetentionManager : NSObject
+- (BOOL)isManagedLayer:(nullable MGLStyleLayer*)styleLayer;
+- (void)updateRetainedLayers:(nonnull NSSet<MGLStyleLayer*>*)sourceObjects;
+- (void)decrementLifetimes;
+@end
+

platform/darwin/src/MGLStyleLayer_Private.h

@@ -59,7 +59,7 @@ struct LayerWrapper {
  pointer value stays even after ownership of the object is transferred via
  `mbgl::Map addLayer`.
  */
-@property (nonatomic, readonly) mbgl::style::Layer *rawLayer;
+@property (nonatomic, readwrite, nullable) mbgl::style::Layer *rawLayer;
 
 /**
  Adds the mbgl style layer that this object represents to the mbgl map below the
@@ -80,6 +80,12 @@ struct LayerWrapper {
  */
 - (void)removeFromStyle:(MGLStyle *)style;
 
+
+/**
+ Debug method used for testing - it resets the peer object, essentially disconnecting the raw Layer
+ from the peer MGLStyleLayer.
+ */
+- (void)debugResetRawLayerPeer;
 @end
 
 NS_ASSUME_NONNULL_END

platform/darwin/src/MGLStyle_Private.h

@@ -23,11 +23,14 @@ namespace mbgl {
 
 @property (nonatomic, readonly, weak) MGLMapView *mapView;
 @property (nonatomic, readonly) mbgl::style::Style *rawStyle;
+@property (nonatomic, readonly) NSMutableSet *managedLayers;
 
 - (nullable NS_ARRAY_OF(MGLAttributionInfo *) *)attributionInfosWithFontSize:(CGFloat)fontSize linkColor:(nullable MGLColor *)linkColor;
 
 - (void)setStyleClasses:(NS_ARRAY_OF(NSString *) *)appliedClasses transitionDuration:(NSTimeInterval)transitionDuration;
 
+- (void)addToManagedLayers:(MGLStyleLayer*)layer;
+- (void)removeFromManagedLayers:(MGLStyleLayer*)layer;
 @end
 
 @interface MGLStyle (MGLStreetsAdditions)