MGLOpenGLStyleLayer released when removed from style

[jmkiley]

Platform: iOS
Mapbox SDK version: v3.7.3 (also seeing in master)

Steps to trigger behavior

1. Create a subclass of MGLOpenGLStyleLayer.
2. Create a property on a view controller that belongs to that class, and add it to the map's style in mapView:didFinishLoadingStyle:,
3. Repeatedly remove and add that layer.

Expected behavior

For the custom layer to be removed and added repeatedly.

Actual behavior

The custom layer is added and removed as expected a couple of times. Then I receive a bad access exception:

* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x0000000112118b05 libobjc.A.dylib`objc_msgSend + 69
  * frame #1: 0x000000010f5f2a1e Mapbox GL`-[MBXViewController toggleLayer](self=0x00007ffe7ad039d0, _cmd="toggleLayer") at MBXViewController.m:1912
    frame #2: 0x0000000111c5d144 Foundation`__NSFireTimer + 83
    frame #3: 0x0000000112633964 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ + 20
    frame #4: 0x00000001126335f3 CoreFoundation`__CFRunLoopDoTimer + 1075
    frame #5: 0x000000011263317a CoreFoundation`__CFRunLoopDoTimers + 250
    frame #6: 0x000000011262af01 CoreFoundation`__CFRunLoopRun + 2065
    frame #7: 0x000000011262a494 CoreFoundation`CFRunLoopRunSpecific + 420
    frame #8: 0x000000011718ba6f GraphicsServices`GSEventRunModal + 161
    frame #9: 0x000000011326bf34 UIKit`UIApplicationMain + 159
    frame #10: 0x000000010f5d699f Mapbox GL`main(argc=1, argv=0x00007ffee0629548) at main.m:8
    frame #11: 0x00000001160be68d libdyld.dylib`start + 1
    frame #12: 0x00000001160be68d libdyld.dylib`start + 1

Message with a weak reference for the property:

*** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: 'The style layer (null) cannot be added to the style. Make sure the style layer was created as a member of a concrete subclass of MGLStyleLayer.'
*** First throw call stack:
(
	0   CoreFoundation                      0x0000000104e1c34b __exceptionPreprocess + 171
	1   libobjc.A.dylib                     0x000000010487d21e objc_exception_throw + 48
	2   CoreFoundation                      0x0000000104e85265 +[NSException raise:format:] + 197
	3   Mapbox                              0x0000000102211f2d -[MGLStyle addLayer:] + 141
	4   Mapbox GL                           0x0000000101d82d1d -[MBXViewController toggleLayer] + 397
	5   Foundation                          0x00000001043d8144 __NSFireTimer + 83
	6   CoreFoundation                      0x0000000104dae964 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ + 20
	7   CoreFoundation                      0x0000000104dae5f3 __CFRunLoopDoTimer + 1075
	8   CoreFoundation                      0x0000000104dae17a __CFRunLoopDoTimers + 250
	9   CoreFoundation                      0x0000000104da5f01 __CFRunLoopRun + 2065
	10  CoreFoundation                      0x0000000104da5494 CFRunLoopRunSpecific + 420
	11  GraphicsServices                    0x0000000109867a6f GSEventRunModal + 161
	12  UIKit                               0x00000001059e6f34 UIApplicationMain + 159
	13  Mapbox GL                           0x0000000101d6634f main + 111
	14  libdyld.dylib                       0x000000010883968d start + 1
	15  ???                                 0x0000000000000001 0x0 + 1
)
libc++abi.dylib: terminating with uncaught exception of type NSException

The Zombies profiling tool gives me:
An Objective-C message was sent to a deallocated ‘LimeGreenStyleLayer’ object (zombie) at address: 0x600000c72fc0.

I am able to avoid this exception by reinitializing the custom layer prior to adding it to the map's style. I do not see this behavior with other MGLStyleLayers. Branch that includes a repro in iosapp.

cc @asheemmamoowala @akitchen @lilykaiser

[jfirebaugh]

Previously: #10755, #10765.

[1ec5]

For what it’s worth, #10771 is the opposite issue (an overretain).

[asheemmamoowala]

@akitchen thanks for setting up the test framework - it helps speed up debugging of this issue in a big way!

For style layer types other than MGLOpenGLStyleLayer:
The ownership is quite simple: User code owns the style layer, and core has a weak reference to it. The weak reference helps reuse the style layer object in [MGLStyle layerFromMBGLLayer]. Other than that, there is no relation between the peer and the core layer objects.

For MGLOpenGLStyleLayer, core is passed a additional pointer to the peer layer object as part of the construction of the core layer in the context parameter. This pointer is used in callbacks to notify the MGLOpenGLStyleLayer subclass of events in core. Since #10765, this context pointer is owned by the core layer, and released when the finish callback is invoked.
There is retain/release mismatch, which can easily be reproduced in the following scenarios:

1. When the layer is not added to a map, and the finish callback is never invoked (over-retain)
2. When the layer is removed from/re-added to the same style (over-release)

One solution to this is to revert #10765 and add matching CFRetain/CFRelease calls to the prepare and finish callbacks. The style would own the style layer, and we would have an additional ownership for the rendering lifecycle. I'm not sure that manual retain/release is OK in this case, so maybe there's a better container that can own the layers to match the lifetime.

[friedbunny]

Fixed by #11553 and ios-v4.0.0-beta.3.