Fixes to optimize resource objects retained by descriptors beyond their lifetimes.

For a resource object that can be retained by descriptors beyond its lifetime,
release memory resources when the object is destroyed by the app. This includes
objects of type MVKBuffer, MVKBufferView, MVKImageView, and MVKSampler.

When the app destroys an MVKBuffer, also detach from the MVKDeviceMemory,
to fix a potential race condition when the app updates the descriptor on
one thread while also freeing the MVKDeviceMemory on another thread.

MVKImageView guard against detached planes while in descriptor.

Add comment to clarify how destroy() is called from release().

Docs/Whats_New.md

@@ -25,6 +25,7 @@ Released TBD
 - Fix deletion of GPU counter `MTLFence` while it is being used by `MTLCommandBuffer`.
 - Remove limit on `VkPhysicalDeviceLimits::maxSamplerAllocationCount` when not using Metal argument buffers.
 - Avoid adjusting SRGB clear color values by half-ULP on GPUs that round float clear colors down.
+- Fixes to optimize resource objects retained by descriptors beyond their lifetimes.
 - `MoltenVKShaderConverter` tool defaults to the highest MSL version supported on runtime OS.
 - Update *glslang* version, to use `python3` in *glslang* scripts, to replace missing `python` on *macOS 12.3*.
 - Update to latest SPIRV-Cross:

MoltenVK/MoltenVK/GPUObjects/MVKBuffer.h

@@ -87,6 +87,8 @@ public:
 
 	~MVKBuffer() override;
 
+	void destroy() override;
+
 protected:
 	friend class MVKDeviceMemory;
 
@@ -99,6 +101,7 @@ protected:
 	VkResult flushToDevice(VkDeviceSize offset, VkDeviceSize size);
 	VkResult pullFromDevice(VkDeviceSize offset, VkDeviceSize size);
 	void initExternalMemory(VkExternalMemoryHandleTypeFlags handleTypes);
+	void detachMemory();
 
 	VkBufferUsageFlags _usage;
 	bool _isHostCoherentTexelBuffer = false;
@@ -133,8 +136,11 @@ public:
 
     ~MVKBufferView() override;
 
+	void destroy() override;
+
 protected:
 	void propagateDebugName() override;
+	void detachMemory();
 
     MVKBuffer* _buffer;
     NSUInteger _offset;

MoltenVK/MoltenVK/GPUObjects/MVKBuffer.mm

@@ -241,10 +241,33 @@ void MVKBuffer::initExternalMemory(VkExternalMemoryHandleTypeFlags handleTypes)
 	}
 }
 
+// Memory detached in destructor too, as a fail-safe.
 MVKBuffer::~MVKBuffer() {
+	detachMemory();
+}
+
+// Overridden to detach from the resource memory when the app destroys this object.
+// This object can be retained in a descriptor after the app destroys it, even
+// though the descriptor can't use it. But doing so retains usuable resource memory.
+// In addition, a potential race condition exists if the app updates the descriptor
+// on one thread at the same time it is destroying the buffer and freeing the device
+// memory on another thread. The race condition occurs when the device memory calls
+// back to this buffer to unbind from it. By detaching from the device memory here,
+// (when the app destroys the buffer), even if this buffer is retained by a descriptor,
+// when the device memory is freed by the app, it won't try to call back here to unbind.
+void MVKBuffer::destroy() {
+	detachMemory();
+	MVKResource::destroy();
+}
+
+// Potentially called twice, from destroy() and destructor, so ensure everything is nulled out.
+void MVKBuffer::detachMemory() {
 	if (_deviceMemory) { _deviceMemory->removeBuffer(this); }
-	if (_mtlBuffer) { [_mtlBuffer release]; }
-    if (_mtlBufferCache) { [_mtlBufferCache release]; }
+	_deviceMemory = nullptr;
+	[_mtlBuffer release];
+	_mtlBuffer = nil;
+	[_mtlBufferCache release];
+	_mtlBufferCache = nil;
 }
 
 
@@ -342,8 +365,21 @@ MVKBufferView::MVKBufferView(MVKDevice* device, const VkBufferViewCreateInfo* pC
     }
 }
 
+// Memory detached in destructor too, as a fail-safe.
 MVKBufferView::~MVKBufferView() {
-    [_mtlTexture release];
-    _mtlTexture = nil;
+	detachMemory();
 }
 
+// Overridden to detach from the resource memory when the app destroys this object.
+// This object can be retained in a descriptor after the app destroys it, even
+// though the descriptor can't use it. But doing so retains usuable resource memory.
+void MVKBufferView::destroy() {
+	detachMemory();
+	MVKVulkanAPIDeviceObject::destroy();
+}
+
+// Potentially called twice, from destroy() and destructor, so ensure everything is nulled out.
+void MVKBufferView::detachMemory() {
+	[_mtlTexture release];
+	_mtlTexture = nil;
+}

MoltenVK/MoltenVK/GPUObjects/MVKImage.h

@@ -565,14 +565,14 @@ public:
 
 #pragma mark Metal
 
-	/** Returns the Metal texture underlying this image view. */
-	id<MTLTexture> getMTLTexture(uint8_t planeIndex = 0) { return _planes[planeIndex]->getMTLTexture(); }
+	/** Returns the Metal texture underlying this image view.  */
+	id<MTLTexture> getMTLTexture(uint8_t planeIndex = 0) { return planeIndex < _planes.size() ? _planes[planeIndex]->getMTLTexture() : nil; }	// Guard against destroyed instance retained in a descriptor.
 
 	/** Returns the Metal pixel format of this image view. */
-	MTLPixelFormat getMTLPixelFormat(uint8_t planeIndex = 0) { return _planes[planeIndex]->_mtlPixFmt; }
+	MTLPixelFormat getMTLPixelFormat(uint8_t planeIndex = 0) { return planeIndex < _planes.size() ? _planes[planeIndex]->_mtlPixFmt : MTLPixelFormatInvalid; }	// Guard against destroyed instance retained in a descriptor.
     
     /** Returns the packed component swizzle of this image view. */
-    uint32_t getPackedSwizzle() { return _planes[0]->getPackedSwizzle(); }
+    uint32_t getPackedSwizzle() { return _planes.empty() ? 0 : _planes[0]->getPackedSwizzle(); }	// Guard against destroyed instance retained in a descriptor.
     
     /** Returns the number of planes of this image view. */
     uint8_t getPlaneCount() { return _planes.size(); }
@@ -599,10 +599,13 @@ public:
 
 	~MVKImageView();
 
+	void destroy() override;
+
 protected:
     friend MVKImageViewPlane;
     
 	void propagateDebugName() override;
+	void detachMemory();
 
     MVKImage* _image;
     MVKSmallVector<MVKImageViewPlane*, 3> _planes;
@@ -686,10 +689,13 @@ public:
 
 	~MVKSampler() override;
 
+	void destroy() override;
+
 protected:
 	void propagateDebugName() override {}
 	MTLSamplerDescriptor* newMTLSamplerDescriptor(const VkSamplerCreateInfo* pCreateInfo);
 	void initConstExprSampler(const VkSamplerCreateInfo* pCreateInfo);
+	void detachMemory();
 
 	id<MTLSamplerState> _mtlSamplerState;
 	SPIRV_CROSS_NAMESPACE::MSLConstexprSampler _constExprSampler;

MoltenVK/MoltenVK/GPUObjects/MVKImage.mm

@@ -1847,7 +1847,21 @@ MVKImageView::MVKImageView(MVKDevice* device, const VkImageViewCreateInfo* pCrea
     }
 }
 
+// Memory detached in destructor too, as a fail-safe.
 MVKImageView::~MVKImageView() {
+	detachMemory();
+}
+
+// Overridden to detach from the resource memory when the app destroys this object.
+// This object can be retained in a descriptor after the app destroys it, even
+// though the descriptor can't use it. But doing so retains usuable resource memory.
+void MVKImageView::destroy() {
+	detachMemory();
+	MVKVulkanAPIDeviceObject::destroy();
+}
+
+// Potentially called twice, from destroy() and destructor, so ensure everything is nulled out.
+void MVKImageView::detachMemory() {
 	mvkDestroyContainerContents(_planes);
 }
 
@@ -2101,8 +2115,23 @@ void MVKSampler::initConstExprSampler(const VkSamplerCreateInfo* pCreateInfo) {
     }
 }
 
+// Memory detached in destructor too, as a fail-safe.
 MVKSampler::~MVKSampler() {
+	detachMemory();
+}
+
+// Overridden to detach from the resource memory when the app destroys this object.
+// This object can be retained in a descriptor after the app destroys it, even
+// though the descriptor can't use it. But doing so retains usuable resource memory.
+void MVKSampler::destroy() {
+	detachMemory();
+	MVKVulkanAPIDeviceObject::destroy();
+}
+
+// Potentially called twice, from destroy() and destructor, so ensure everything is nulled out.
+void MVKSampler::detachMemory() {
 	@synchronized (getMTLDevice()) {
 		[_mtlSamplerState release];
+		_mtlSamplerState = nil;
 	}
 }

MoltenVK/MoltenVK/Utility/MVKBaseObject.h

@@ -135,6 +135,9 @@ public:
 	 * Once all references have been released, this object is free to be deleted.
 	 * If the destroy() function has already been called on this instance by the time
 	 * this function is called, this instance will be deleted.
+	 *
+	 * Note that the destroy() function is called on the BaseClass.
+	 * Releasing will not call any overridden destroy() function in a descendant class.
 	 */
 	void release() { if (--_refCount == 0) { BaseClass::destroy(); } }