[ios, macos, core] Added retain/release manager that holds on to the MGLOpenGLStyleLayers for the 2 trips through the render loop that are needed before they can be deallocated. Added additional tests.

7 files changed, 133 insertions, 33 deletions

diff --git a/platform/darwin/src/MGLOpenGLStyleLayer.mm b/platform/darwin/src/MGLOpenGLStyleLayer.mm
index 80805b001b..a5f8b574a0 100644
--- a/platform/darwin/src/MGLOpenGLStyleLayer.mm
+++ b/platform/darwin/src/MGLOpenGLStyleLayer.mm
@@ -50,13 +50,43 @@ void MGLDrawCustomStyleLayer(void *context, const mbgl::style::CustomLayerRender
     when creating an OpenGL style layer.
  */
 void MGLFinishCustomStyleLayer(void *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.
@@ -108,7 +138,9 @@ void MGLFinishCustomStyleLayer(void *context) {
                                                             MGLPrepareCustomStyleLayer,
                                                             MGLDrawCustomStyleLayer,
                                                             MGLFinishCustomStyleLayer,
+                                                            MGLDeallocateCustomStyleLayer,
                                                             (__bridge void*)self);
+
     return self = [super initWithPendingLayer:std::move(layer)];
 }
 
@@ -128,11 +160,21 @@ void MGLFinishCustomStyleLayer(void *context) {
 
 - (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;
 }
 
diff --git a/platform/darwin/src/MGLStyle.mm b/platform/darwin/src/MGLStyle.mm
index 695f8b55e7..3d503d77aa 100644
--- a/platform/darwin/src/MGLStyle.mm
+++ b/platform/darwin/src/MGLStyle.mm
@@ -84,10 +84,7 @@
 @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;
-
-// Used for retain/release management
-@property (nonatomic) NSMutableSet *layersForUpdating;
-@property (nonatomic) NSSet *layersForRendering;
+@property (nonatomic, readwrite) NSMutableSet *managedLayers;
 
 @end
 
@@ -176,8 +173,7 @@ static NSURL *MGLStyleURL_trafficNight;
 
 - (instancetype)initWithRawStyle:(mbgl::style::Style *)rawStyle mapView:(MGLMapView *)mapView {
     if (self = [super init]) {
-        _layersForUpdating = [NSMutableSet set];
-
+        _managedLayers = [NSMutableSet set];
         _mapView = mapView;
         _rawStyle = rawStyle;
         _localizedLayersByIdentifier = [NSMutableDictionary dictionary];
@@ -543,22 +539,13 @@ static NSURL *MGLStyleURL_trafficNight;
 #pragma mark - Layer retain/release management
 
 - (void)addToManagedLayers:(MGLStyleLayer*)layer {
-    [self.layersForUpdating addObject:layer];
+    [self.managedLayers addObject:layer];
 }
 
 - (void)removeFromManagedLayers:(MGLStyleLayer*)layer {
-    [self.layersForUpdating removeObject:layer];
-}
-
-- (void)retainLayersUsedDuringRendering {
-    self.layersForRendering = [self.layersForUpdating copy];
+    [self.managedLayers removeObject:layer];
 }
 
-- (void)releaseLayersUsedDuringRendering {
-    self.layersForRendering = nil;
-}
-
-
 #pragma mark Style classes
 
 - (NS_ARRAY_OF(NSString *) *)styleClasses
diff --git a/platform/darwin/src/MGLStyleLayer.mm b/platform/darwin/src/MGLStyleLayer.mm
index 444dbaeea3..f29142c0c7 100644
--- a/platform/darwin/src/MGLStyleLayer.mm
+++ b/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 @@
                     "to the style more than once is invalid.", self, 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];
-
     if (otherLayer) {
         const mbgl::optional<std::string> belowLayerId{otherLayer.identifier.UTF8String};
         style.rawStyle->addLayer(std::move(_pendingLayer), belowLayerId);
@@ -54,10 +48,6 @@
 {
     if (self.rawLayer == style.rawStyle->getLayer(self.identifier.UTF8String)) {
         _pendingLayer = style.rawStyle->removeLayer(self.identifier.UTF8String);
-
-        // 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];
     }
 }
 
@@ -111,7 +101,14 @@
 {
     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
diff --git a/platform/darwin/src/MGLStyleLayerRetentionManager.mm b/platform/darwin/src/MGLStyleLayerRetentionManager.mm
new file mode 100644
index 0000000000..71ebd574ad
--- /dev/null
+++ b/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
diff --git a/platform/darwin/src/MGLStyleLayerRetentionManager_Private.h b/platform/darwin/src/MGLStyleLayerRetentionManager_Private.h
new file mode 100644
index 0000000000..0063d342a5
--- /dev/null
+++ b/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
+
diff --git a/platform/darwin/src/MGLStyleLayer_Private.h b/platform/darwin/src/MGLStyleLayer_Private.h
index 9bee013c3d..9c93f4c0c9 100644
--- a/platform/darwin/src/MGLStyleLayer_Private.h
+++ b/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
diff --git a/platform/darwin/src/MGLStyle_Private.h b/platform/darwin/src/MGLStyle_Private.h
index cfde78fd06..00985403f5 100644
--- a/platform/darwin/src/MGLStyle_Private.h
+++ b/platform/darwin/src/MGLStyle_Private.h
@@ -23,6 +23,7 @@ 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;
 
@@ -30,8 +31,6 @@ namespace mbgl {
 
 - (void)addToManagedLayers:(MGLStyleLayer*)layer;
 - (void)removeFromManagedLayers:(MGLStyleLayer*)layer;
-- (void)retainLayersUsedDuringRendering;
-- (void)releaseLayersUsedDuringRendering;
 @end
 
 @interface MGLStyle (MGLStreetsAdditions)