Merge pull request #652 from billhollings/master
Fix race condition between swapchain image destruction and presentation completion callback
diff --git a/Docs/Whats_New.md b/Docs/Whats_New.md
index 96ab18e..27cc42c 100644
--- a/Docs/Whats_New.md
+++ b/Docs/Whats_New.md
@@ -27,7 +27,8 @@
- Skip `SPIRV-Tools` build in Travis because Travis does not support the required Python 3.
- Separate `SPIRVToMSLConverterContext` into input config and output results.
- Fix pipeline cache lookups.
-- Doument that the functions in `vk_mvk_moltenvk.h` cannot be used with objects
+- Fix race condition between swapchain image destruction and presentation completion callback.
+- Document that the functions in `vk_mvk_moltenvk.h` cannot be used with objects
retrieved through the *Vulkan SDK Loader and Layers* framework.
- Update `VK_MVK_MOLTENVK_SPEC_VERSION` to 21.
- Update to latest SPIRV-Cross version:
diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm b/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm
index 96ebc14..cc977a5 100644
--- a/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm
+++ b/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm
@@ -1323,7 +1323,12 @@
// Signal the semaphore device-side.
_availabilitySignalers.front().first->encodeSignal(mtlCmdBuff);
}
- [mtlCmdBuff addCompletedHandler: ^(id<MTLCommandBuffer> mcb) { makeAvailable(); }];
+
+ retain(); // Ensure this image is not destroyed while awaiting MTLCommandBuffer completion
+ [mtlCmdBuff addCompletedHandler: ^(id<MTLCommandBuffer> mcb) {
+ makeAvailable();
+ release();
+ }];
} else {
[mtlDrawable present];
resetMetalSurface();
diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKVulkanAPIObject.h b/MoltenVK/MoltenVK/GPUObjects/MVKVulkanAPIObject.h
index 0a326dd..abf7863 100644
--- a/MoltenVK/MoltenVK/GPUObjects/MVKVulkanAPIObject.h
+++ b/MoltenVK/MoltenVK/GPUObjects/MVKVulkanAPIObject.h
@@ -21,7 +21,7 @@
#include "MVKBaseObject.h"
#include <vulkan/vk_icd.h>
#include <string>
-#include <mutex>
+#include <atomic>
#import <Foundation/NSString.h>
@@ -93,25 +93,18 @@
static MVKVulkanAPIObject* getMVKVulkanAPIObject(VkObjectType objType, uint64_t objectHandle);
/** Construct an empty instance. Declared here to support copy constructor. */
- MVKVulkanAPIObject() {}
+ MVKVulkanAPIObject() : _refCount(1) {}
- /**
- * Construct an instance from a copy. Default copy constructor disallowed due to mutex.
- * Copies start with fresh reference counts.
- */
- MVKVulkanAPIObject(const MVKVulkanAPIObject& other) {}
+ /** Default copy constructor disallowed due to mutex. Copy starts with fresh reference counts. */
+ MVKVulkanAPIObject(const MVKVulkanAPIObject& other) : _refCount(1) {}
~MVKVulkanAPIObject() override;
protected:
- bool decrementRetainCount();
- bool markDestroyed();
virtual void propogateDebugName() = 0;
+ std::atomic<uint32_t> _refCount;
NSString* _debugName = nil;
- std::mutex _refLock;
- unsigned _refCount = 0;
- bool _isDestroyed = false;
};
diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKVulkanAPIObject.mm b/MoltenVK/MoltenVK/GPUObjects/MVKVulkanAPIObject.mm
index 6fd294d..6a390b1 100644
--- a/MoltenVK/MoltenVK/GPUObjects/MVKVulkanAPIObject.mm
+++ b/MoltenVK/MoltenVK/GPUObjects/MVKVulkanAPIObject.mm
@@ -25,33 +25,15 @@
#pragma mark MVKVulkanAPIObject
void MVKVulkanAPIObject::retain() {
- lock_guard<mutex> lock(_refLock);
-
_refCount++;
}
void MVKVulkanAPIObject::release() {
- if (decrementRetainCount()) { destroy(); }
+ if (--_refCount == 0) { MVKConfigurableObject::destroy(); }
}
void MVKVulkanAPIObject::destroy() {
- if (markDestroyed()) { MVKConfigurableObject::destroy(); }
-}
-
-// Decrements the reference count, and returns whether it's time to destroy this object.
-bool MVKVulkanAPIObject::decrementRetainCount() {
- lock_guard<mutex> lock(_refLock);
-
- if (_refCount > 0) { _refCount--; }
- return (_isDestroyed && _refCount == 0);
-}
-
-// Marks this object as destroyed, and returns whether no references are left outstanding.
-bool MVKVulkanAPIObject::markDestroyed() {
- lock_guard<mutex> lock(_refLock);
-
- _isDestroyed = true;
- return _refCount == 0;
+ release();
}
VkResult MVKVulkanAPIObject::setDebugName(const char* pObjectName) {
