Skip to content

fix Metal leaking resources #9373

Open
39ali wants to merge 1 commit intogfx-rs:trunkfrom
39ali:metal-leak
Open

fix Metal leaking resources #9373
39ali wants to merge 1 commit intogfx-rs:trunkfrom
39ali:metal-leak

Conversation

@39ali
Copy link
Copy Markdown
Contributor

@39ali 39ali commented Apr 5, 2026

Connections
should help fix #8018

Description
metal is leaking some strings and gpu device because metal resources hold a strong reference back to the MTLDevice

Testing
ran some custom tests that would keep creating/deleting resources

Squash or Rebase?
Squash

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy --tests. If applicable, add:
    • --target wasm32-unknown-unknown
  • Run cargo xtask test to run tests.
  • If this contains user-facing changes, add a CHANGELOG.md entry.

@ErichDonGubler ErichDonGubler self-assigned this Apr 7, 2026
@ErichDonGubler
Copy link
Copy Markdown
Member

@andyleiserson, could you please take this on, since you measured the original problem?

@39ali: You have merge conflicts, and need to rebase.

self.counters.buffers.sub(1);
autoreleasepool(|_| {
drop(buffer);
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain in more detail how you found this (and the similar handling for QueueShared) to be necessary? Is the idea that releasing these objects transfers ownership of some member(s) to the autorelease pool, rather than directly releasing them?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from my testings, it doesn't directly release them since there's no main release pool and that's because the code is not running inside Cocoa event loop so you have to manually autoreleasepool by yourself , there's probably a lot of places like this in the metal backend but this is what i currently found to be leaking

Copy link
Copy Markdown
Contributor

@andyleiserson andyleiserson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to omit creating autoreleasepools in Drop / destroy, unless there's compelling evidence it is necessary.

(For what it's worth, I tried running the fuzzing test case from #8018 with just the added autoreleasepools on non-destructor paths, and it did seem like the leak was resolved.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Metal resources appear to be leaking in fuzzing runs

3 participants