Reference cycle if UIView owns VkSurfaceKHR

[dmikis]

If we're to create a simple view like that:

#import <UIKit/UIKit.h>
#import <QuartzCore/CAMetalLayer.h>

#define VK_USE_PLATFORM_IOS_MVK
#include <vulkan/vulkan.h>

@interface YRTThinVulkanView: UIView
@end

@implementation YRTThinVulkanView {
    VkInstance _instance;
    VkSurfaceKHR _surface;
}

+ (Class)layerClass { return [CAMetalLayer class]; }

- (instancetype)initWithCoder:(NSCoder*)coder
{
    self = [super initWithCoder:coder];

    if (!self) return self;

    VkInstanceCreateInfo instanceCreateInfo;
    instanceCreateInfo.sType = VK_STRUCTURE_TYPE_INSTANCE_CREATE_INFO;
    instanceCreateInfo.pNext = nullptr;
    instanceCreateInfo.flags = 0;
    instanceCreateInfo.pApplicationInfo = nullptr;
    instanceCreateInfo.enabledLayerCount = instanceCreateInfo.enabledExtensionCount = 0;

    NSAssert(
        vkCreateInstance(&instanceCreateInfo, nullptr, &_instance) == VK_SUCCESS,
        @"Failed to create instance");

    VkIOSSurfaceCreateInfoMVK surfaceCreateInfo;
    surfaceCreateInfo.sType = VK_STRUCTURE_TYPE_IOS_SURFACE_CREATE_INFO_MVK;
    surfaceCreateInfo.pNext = nullptr;
    surfaceCreateInfo.flags = 0;
    surfaceCreateInfo.pView = (__bridge void*) self.layer;
    NSAssert(
        vkCreateIOSSurfaceMVK(_instance, &surfaceCreateInfo, nullptr, &_surface) == VK_SUCCESS,
        @"Failed to create surface");

    return self;
}

- (void)dealloc
{
    vkDestroySurfaceKHR(_instance, _surface, nullptr);
    vkDestroyInstance(_instance, nullptr);
}

@end

This view leaks "itself" because listener in MVKSurface takes a strong reference to it. In our framework the view is a root of the whole object hierarchy and with the view a whole bunch of objects are leaked.

[cdavis5e]

I wanted to use a weak reference originally, but we can't, because we aren't using ARC. So now I'm left with the problem of, "How do I know the view is (being) destroyed, so I shouldn't attempt to remove myself from the KVO list?"

[billhollings]

@dmikis

Presumably you are able to move the call to vkDestroySurfaceKHR() to somewhere outside the YRTThinVulkanView dealloc method?

Admittedly not quite as nicely self contained...but say something along the lines of openVulkan and closeVulkan methods?

[billhollings]

@cdavis5e

It looks like the view is only retained in the observer for the purpose of alerting it when the observer is destroyed when the surface is destroyed.

Hmmm...I wonder if we could opt to not retain the view in the observer...and when the surface is destroyed, instead of releasing it...mark the observer as obsolete...and then let the KVO environment continue to call it...which could trigger the obsolete observer to remove itself from the KVO...and release itself.

Kind of a deferred release of the obsolete observer. It would effectively constitute a small leak of just the observer object until the KVO was triggered. I suppose it might leak forever if the KVO was not triggered when the view was destroyed.

[dmikis]

I wonder if we could opt to not retain the view in the observer

I think that's may be a viable option: there's no correct situation when a surface outlives a view. Hence situation when _target pointer becomes invalid is improbable. I believe, if a surface for some reason outlives view it's created from, it'll break on next image acquire anyway.

If a surface's destroyed well before corresponding view, it safe to simply remove observer from the target anyway, without obsolescence flag.

I can try to test that tomorrow and create a pull request with the fix if it'll appear to work.

UPD. Somewhat side question: why MoltenVK doesn't use ARC?

[billhollings]

I can try to test that tomorrow and create a pull request with the fix if it'll appear to work.

Yes...please do so.

there's no correct situation when a surface outlives a view

Be careful. We need to be able to guarantee that. Not many apps will ensure the view will clean up the surface as yours does. It's quite possible for an app to be structured to release the view on the line before it destroys the surface...which would then cause the MVKBlockObserver::stopObserving() function to trigger a memory crash on it's first line with a reference to a now dead _target.

Why MoltenVK doesn't use ARC?

ARC is quite costly in that retain/release happens under the covers a lot more than one would expect. And the use of Obj-C in MoltenVK is a small portion of the overall MoltenVK code...so the decision was to treat the Obj-C objects in a similar manner to the C++ objects.

[dolphineye]

Hi,

It seems we have a similar issue in our case. We have a UIView calling the following code during initialization: https://github.com/DiligentGraphics/DiligentCore/blob/master/Graphics/GraphicsEngineVulkan/src/SwapChainVkImpl.cpp#L101

Unless we explicitly call a destroy() method that manually does all the require cleanup, our dealloc method wil never get called. Figures shown that after the call to vkCreateIOSSurfaceMVK, we have two more additional strong references to our UIView.

Is there anything specific we should be doing to fix that?

Thanks for helping! :)