Skip to content

Commit

Permalink
Remove textureUseAfterFreePoolSize (#8163)
Browse files Browse the repository at this point in the history
  • Loading branch information
bejado authored Oct 4, 2024
1 parent 3bd4c45 commit 3b9fc8f
Show file tree
Hide file tree
Showing 7 changed files with 8 additions and 82 deletions.
7 changes: 0 additions & 7 deletions filament/backend/include/backend/Platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down
11 changes: 0 additions & 11 deletions filament/backend/src/metal/MetalContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,6 @@ class MetalDynamicOffsets {
};

struct MetalContext {
explicit MetalContext(size_t metalFreedTextureListSize)
: texturesToDestroy(metalFreedTextureListSize) {}

MetalDriver* driver;
id<MTLDevice> device = nullptr;
id<MTLCommandQueue> commandQueue = nullptr;
Expand Down Expand Up @@ -187,14 +184,6 @@ struct MetalContext {
// Keeps track of all alive textures.
tsl::robin_set<MetalTexture*> 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<Handle<HwTexture>> texturesToDestroy;

MetalBufferPool* bufferPool;
MetalBumpAllocator* bumpAllocator;

Expand Down
30 changes: 8 additions & 22 deletions filament/backend/src/metal/MetalDriver.mm
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,13 @@
return ConcreteDispatcher<MetalDriver>::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);
Expand Down Expand Up @@ -808,15 +808,7 @@
auto* metalTexture = handle_cast<MetalTexture>(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<MetalTexture>(handleToFree.value());
}
destruct_handle<MetalTexture>(th);
}

void MetalDriver::destroyRenderTarget(Handle<HwRenderTarget> rth) {
Expand Down Expand Up @@ -866,12 +858,6 @@
}

void MetalDriver::terminate() {
// Terminate any outstanding MetalTextures.
while (!mContext->texturesToDestroy.empty()) {
Handle<HwTexture> toDestroy = mContext->texturesToDestroy.pop();
destruct_handle<MetalTexture>(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();
Expand Down
22 changes: 0 additions & 22 deletions filament/backend/src/metal/MetalHandles.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<MTLTexture> swizzledTextureView = nil;

bool terminated = false;
};

class MetalRenderTarget : public HwRenderTarget {
Expand Down
8 changes: 0 additions & 8 deletions filament/backend/src/metal/MetalHandles.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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<MTLTexture> MetalTexture::getMtlTextureForRead() const noexcept {
return swizzledTextureView ? swizzledTextureView : texture;
}
Expand Down
10 changes: 0 additions & 10 deletions filament/include/filament/Engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions filament/src/details/Engine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -701,7 +700,6 @@ int FEngine::loop() {

DriverConfig const driverConfig {
.handleArenaSize = getRequestedDriverHandleArenaSize(),
.textureUseAfterFreePoolSize = mConfig.textureUseAfterFreePoolSize,
.metalUploadBufferSizeBytes = mConfig.metalUploadBufferSizeBytes,
.disableParallelShaderCompile = mConfig.disableParallelShaderCompile,
.disableHandleUseAfterFreeCheck = mConfig.disableHandleUseAfterFreeCheck,
Expand Down

0 comments on commit 3b9fc8f

Please sign in to comment.