diff --git a/filament/backend/include/backend/Platform.h b/filament/backend/include/backend/Platform.h index b8befb4cee6..af18b1488a4 100644 --- a/filament/backend/include/backend/Platform.h +++ b/filament/backend/include/backend/Platform.h @@ -68,13 +68,6 @@ class UTILS_PUBLIC Platform { */ size_t handleArenaSize = 0; - /** - * This number of most-recently destroyed textures will be tracked for use-after-free. - * Throws an exception when a texture is freed but still bound to a SamplerGroup and used in - * a draw call. 0 disables completely. Currently only respected by the Metal backend. - */ - size_t textureUseAfterFreePoolSize = 0; - size_t metalUploadBufferSizeBytes = 512 * 1024; /** diff --git a/filament/backend/src/metal/MetalContext.h b/filament/backend/src/metal/MetalContext.h index 3f6cbaa1a95..ef1175fa688 100644 --- a/filament/backend/src/metal/MetalContext.h +++ b/filament/backend/src/metal/MetalContext.h @@ -118,9 +118,6 @@ class MetalDynamicOffsets { }; struct MetalContext { - explicit MetalContext(size_t metalFreedTextureListSize) - : texturesToDestroy(metalFreedTextureListSize) {} - MetalDriver* driver; id device = nullptr; id commandQueue = nullptr; @@ -187,14 +184,6 @@ struct MetalContext { // Keeps track of all alive textures. tsl::robin_set textures; - // This circular buffer implements delayed destruction for Metal texture handles. It keeps a - // handle to a fixed number of the most recently destroyed texture handles. When we're asked to - // destroy a texture handle, we free its texture memory, but keep the MetalTexture object alive, - // marking it as "terminated". If we later are asked to use that texture, we can check its - // terminated status and throw an Objective-C error instead of crashing, which is helpful for - // debugging use-after-free issues in release builds. - utils::FixedCircularBuffer> texturesToDestroy; - MetalBufferPool* bufferPool; MetalBumpAllocator* bumpAllocator; diff --git a/filament/backend/src/metal/MetalDriver.mm b/filament/backend/src/metal/MetalDriver.mm index f10df34039b..0bcb55aa10f 100644 --- a/filament/backend/src/metal/MetalDriver.mm +++ b/filament/backend/src/metal/MetalDriver.mm @@ -107,13 +107,13 @@ return ConcreteDispatcher::make(); } -MetalDriver::MetalDriver(MetalPlatform* platform, const Platform::DriverConfig& driverConfig) noexcept - : mPlatform(*platform), - mContext(new MetalContext(driverConfig.textureUseAfterFreePoolSize)), - mHandleAllocator("Handles", - driverConfig.handleArenaSize, - driverConfig.disableHandleUseAfterFreeCheck), - mStereoscopicType(driverConfig.stereoscopicType) { +MetalDriver::MetalDriver( + MetalPlatform* platform, const Platform::DriverConfig& driverConfig) noexcept + : mPlatform(*platform), + mContext(new MetalContext), + mHandleAllocator( + "Handles", driverConfig.handleArenaSize, driverConfig.disableHandleUseAfterFreeCheck), + mStereoscopicType(driverConfig.stereoscopicType) { mContext->driver = this; TrackedMetalBuffer::setPlatform(platform); @@ -808,15 +808,7 @@ auto* metalTexture = handle_cast(th); mContext->textures.erase(metalTexture); - // Free memory from the texture and mark it as freed. - metalTexture->terminate(); - - // Add this texture handle to our texturesToDestroy queue to be destroyed later. - if (auto handleToFree = mContext->texturesToDestroy.push(th)) { - // If texturesToDestroy is full, then .push evicts the oldest texture handle in the - // queue (or simply th, if use-after-free detection is disabled). - destruct_handle(handleToFree.value()); - } + destruct_handle(th); } void MetalDriver::destroyRenderTarget(Handle rth) { @@ -866,12 +858,6 @@ } void MetalDriver::terminate() { - // Terminate any outstanding MetalTextures. - while (!mContext->texturesToDestroy.empty()) { - Handle toDestroy = mContext->texturesToDestroy.pop(); - destruct_handle(toDestroy); - } - // finish() will flush the pending command buffer and will ensure all GPU work has finished. // This must be done before calling bufferPool->reset() to ensure no buffers are in flight. finish(); diff --git a/filament/backend/src/metal/MetalHandles.h b/filament/backend/src/metal/MetalHandles.h index 4466f45cee7..8e2f7e253d0 100644 --- a/filament/backend/src/metal/MetalHandles.h +++ b/filament/backend/src/metal/MetalHandles.h @@ -267,26 +267,6 @@ class MetalTexture : public HwTexture { MTLPixelFormat devicePixelFormat; - // Frees memory associated with this texture and marks it as "terminated". - // Used to track "use after free" scenario. - void terminate() noexcept; - bool isTerminated() const noexcept { return terminated; } - inline void checkUseAfterFree(const char* samplerGroupDebugName, size_t textureIndex) const { - if (UTILS_LIKELY(!isTerminated())) { - return; - } - NSString* reason = - [NSString stringWithFormat: - @"Filament Metal texture use after free, sampler group = " - @"%s, texture index = %zu", - samplerGroupDebugName, textureIndex]; - NSException* useAfterFreeException = - [NSException exceptionWithName:@"MetalTextureUseAfterFree" - reason:reason - userInfo:nil]; - [useAfterFreeException raise]; - } - private: void loadSlice(uint32_t level, MTLRegion region, uint32_t byteOffset, uint32_t slice, PixelBufferDescriptor const& data) noexcept; @@ -303,8 +283,6 @@ class MetalTexture : public HwTexture { // Filament swizzling only affects texture reads, so this should not be used when the texture is // bound as a render target attachment. id swizzledTextureView = nil; - - bool terminated = false; }; class MetalRenderTarget : public HwRenderTarget { diff --git a/filament/backend/src/metal/MetalHandles.mm b/filament/backend/src/metal/MetalHandles.mm index c61504d3c7c..c451264efac 100644 --- a/filament/backend/src/metal/MetalHandles.mm +++ b/filament/backend/src/metal/MetalHandles.mm @@ -648,14 +648,6 @@ static void func(void* user) { texture = externalImage->getMtlTexture(); } -void MetalTexture::terminate() noexcept { - texture = nil; - swizzledTextureView = nil; - msaaSidecar = nil; - externalImage = nullptr; - terminated = true; -} - id MetalTexture::getMtlTextureForRead() const noexcept { return swizzledTextureView ? swizzledTextureView : texture; } diff --git a/filament/include/filament/Engine.h b/filament/include/filament/Engine.h index 641c9846eda..2686f25ea1c 100644 --- a/filament/include/filament/Engine.h +++ b/filament/include/filament/Engine.h @@ -288,16 +288,6 @@ class UTILS_PUBLIC Engine { */ uint32_t jobSystemThreadCount = 0; - /* - * Number of most-recently destroyed textures to track for use-after-free. - * - * This will cause the backend to throw an exception when a texture is freed but still bound - * to a SamplerGroup and used in a draw call. 0 disables completely. - * - * Currently only respected by the Metal backend. - */ - size_t textureUseAfterFreePoolSize = 0; - /** * When uploading vertex or index data, the Filament Metal backend copies data * into a shared staging area before transferring it to the GPU. This setting controls diff --git a/filament/src/details/Engine.cpp b/filament/src/details/Engine.cpp index c6598409538..b1aaa5c9f5f 100644 --- a/filament/src/details/Engine.cpp +++ b/filament/src/details/Engine.cpp @@ -103,7 +103,6 @@ Engine* FEngine::create(Engine::Builder const& builder) { } DriverConfig const driverConfig{ .handleArenaSize = instance->getRequestedDriverHandleArenaSize(), - .textureUseAfterFreePoolSize = instance->getConfig().textureUseAfterFreePoolSize, .metalUploadBufferSizeBytes = instance->getConfig().metalUploadBufferSizeBytes, .disableParallelShaderCompile = instance->getConfig().disableParallelShaderCompile, .disableHandleUseAfterFreeCheck = instance->getConfig().disableHandleUseAfterFreeCheck, @@ -701,7 +700,6 @@ int FEngine::loop() { DriverConfig const driverConfig { .handleArenaSize = getRequestedDriverHandleArenaSize(), - .textureUseAfterFreePoolSize = mConfig.textureUseAfterFreePoolSize, .metalUploadBufferSizeBytes = mConfig.metalUploadBufferSizeBytes, .disableParallelShaderCompile = mConfig.disableParallelShaderCompile, .disableHandleUseAfterFreeCheck = mConfig.disableHandleUseAfterFreeCheck,