Fix use-after-free of a std::mutex in PresentCallable. (#7934)

If a user is using `setFrameScheduledCallback()`, managing the provided PresentCallable during engine shutdown is tricky -- we'll likely get a final frame scheduled when we flush the engine's work queue, but the PresentCallable will schedule the final CAMetalDrawable to be released on main thread afterwards, even if we call `present(false)` to skip it. If the swap chain is destroyed before that main queue block gets executed, the mutex presenting that drawable will no longer exist, causing a crash.

To make things easier, store the std::mutex in a shared_ptr, so that a PresentCallable can safely outlive the FSwapChain instance that created it and clean itself up afterwards.

Alternatives considered, all of which seem unfortunate:
* Require users to clear out the callback before shutting down the engine, so that any final drawables are immediately discarded instead of using PresentCallable
* Require users to split up the engine teardown across two main queue blocks, ensuring that the PresentCallable cleanup executes before swapchains are destroyed
* Drop the PresentCallable on the ground and leak the memory

filament/backend/src/metal/MetalHandles.h

@@ -109,7 +109,7 @@ class MetalSwapChain : public HwSwapChain {
     NSUInteger headlessWidth = 0;
     NSUInteger headlessHeight = 0;
     CAMetalLayer* layer = nullptr;
-    std::mutex layerDrawableMutex;
+    std::shared_ptr<std::mutex> layerDrawableMutex;
     MetalExternalImage externalImage;
     SwapChainType type;
 

filament/backend/src/metal/MetalHandles.mm

@@ -73,6 +73,7 @@ static inline MTLTextureUsage getMetalTextureUsage(TextureUsage usage) {
     : context(context),
       depthStencilFormat(decideDepthStencilFormat(flags)),
       layer(nativeWindow),
+      layerDrawableMutex(std::make_shared<std::mutex>()),
       externalImage(context),
       type(SwapChainType::CAMETALLAYER) {
 
@@ -179,7 +180,7 @@ static inline MTLTextureUsage getMetalTextureUsage(TextureUsage usage) {
     // calling -nextDrawable, or when releasing the last known reference
     // to any CAMetalDrawable returned from a previous -nextDrawable.
     {
-        std::lock_guard<std::mutex> lock(layerDrawableMutex);
+        std::lock_guard<std::mutex> lock(*layerDrawableMutex);
         drawable = [layer nextDrawable];
     }
 
@@ -188,8 +189,10 @@ static inline MTLTextureUsage getMetalTextureUsage(TextureUsage usage) {
 }
 
 void MetalSwapChain::releaseDrawable() {
-    std::lock_guard<std::mutex> lock(layerDrawableMutex);
-    drawable = nil;
+    if (drawable) {
+        std::lock_guard<std::mutex> lock(*layerDrawableMutex);
+        drawable = nil;
+    }
 }
 
 id<MTLTexture> MetalSwapChain::acquireDepthTexture() {
@@ -265,7 +268,7 @@ static inline MTLTextureUsage getMetalTextureUsage(TextureUsage usage) {
     PresentDrawableData& operator=(const PresentDrawableData&) = delete;
 
     static PresentDrawableData* create(id<CAMetalDrawable> drawable,
-            std::mutex* drawableMutex, MetalDriver* driver) {
+            std::shared_ptr<std::mutex> drawableMutex, MetalDriver* driver) {
         assert_invariant(drawableMutex);
         assert_invariant(driver);
         return new PresentDrawableData(drawable, drawableMutex, driver);
@@ -287,22 +290,22 @@ static void maybePresentAndDestroyAsync(PresentDrawableData* that, bool shouldPr
     }
 
 private:
-    PresentDrawableData(id<CAMetalDrawable> drawable, std::mutex* drawableMutex,
+    PresentDrawableData(id<CAMetalDrawable> drawable, std::shared_ptr<std::mutex> drawableMutex,
             MetalDriver* driver)
         : mDrawable(drawable), mDrawableMutex(drawableMutex), mDriver(driver) {}
 
     static void cleanupAndDestroy(PresentDrawableData *that) {
-        {
+        if (that->mDrawable) {
             std::lock_guard<std::mutex> lock(*(that->mDrawableMutex));
             that->mDrawable = nil;
         }
-        that->mDrawableMutex = nullptr;
+        that->mDrawableMutex.reset();
         that->mDriver = nullptr;
         delete that;
     }
 
     id<CAMetalDrawable> mDrawable;
-    std::mutex* mDrawableMutex = nullptr;
+    std::shared_ptr<std::mutex> mDrawableMutex;
     MetalDriver* mDriver = nullptr;
 };
 
@@ -320,7 +323,7 @@ void presentDrawable(bool presentFrame, void* user) {
 
     struct Callback {
         Callback(std::shared_ptr<FrameScheduledCallback> callback, id<CAMetalDrawable> drawable,
-                std::mutex* drawableMutex, MetalDriver* driver)
+                 std::shared_ptr<std::mutex> drawableMutex, MetalDriver* driver)
             : f(callback), data(PresentDrawableData::create(drawable, drawableMutex, driver)) {}
         std::shared_ptr<FrameScheduledCallback> f;
         // PresentDrawableData* is destroyed by maybePresentAndDestroyAsync() later.
@@ -337,7 +340,7 @@ static void func(void* user) {
     // This callback pointer will be captured by the block. Even if the scheduled handler is never
     // called, the unique_ptr will still ensure we don't leak memory.
     __block auto callback = std::make_unique<Callback>(
-        frameScheduled.callback, drawable, &layerDrawableMutex, context.driver);
+        frameScheduled.callback, drawable, layerDrawableMutex, context.driver);
 
     backend::CallbackHandler* handler = frameScheduled.handler;
     MetalDriver* driver = context.driver;