Skip to content

Conversation

@TimothyMakkison
Copy link
Contributor

@TimothyMakkison TimothyMakkison commented Feb 2, 2026

Make InterfaceCache stateless to prevent creating static references to INamedTypeSymbol and creating a memory leak

See #4644

@claude
Copy link
Contributor

claude bot commented Feb 2, 2026

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

This PR effectively addresses the critical memory leak issue by removing static caching that held references to stale Roslyn compilation objects. The approach is sound—trading a minor performance cost for eliminating unbounded memory growth is the right call for source generators, where correctness and resource efficiency are paramount.

The stateless design simplifies the code and removes cache invalidation concerns, making the implementation more maintainable going forward.

@thomhurst thomhurst enabled auto-merge (squash) February 2, 2026 23:05
@thomhurst thomhurst merged commit 39ba0e0 into thomhurst:main Feb 2, 2026
8 of 10 checks passed
@thomhurst
Copy link
Owner

Thanks for finding this issue!

@TimothyMakkison
Copy link
Contributor Author

Thanks for finding this issue!

No problem, easy mistake to made. I am a little worried that a smaller memory leak is happening as #4644 still shows a small increase each time we rerun the generator.

This might just be a quirk of the test or compilation, or it could be another leak. I don't see many likely culprits though 🤔

@thomhurst
Copy link
Owner

thomhurst commented Feb 3, 2026

Can we run it through dotnet-trace on a loop or something to measure memory?

@TimothyMakkison
Copy link
Contributor Author

TimothyMakkison commented Feb 3, 2026

Good idea, never used it before but it seems appropriate. I fruitlessly tried comparing dot memory snapshots, needless I'm no expert in these kinds of issues.

This might just be a quirk of the test or compilation, or it could be another leak. I don't see many likely culprits though

On the plus side, the leak (if it exists) seems fairly small. If it's proportionate to the size of the project then it'll be fairly benign in most real world smaller test projects

Again this might be a quirk of the test. Has anyone ever reported an ide memory usage issue?

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.

2 participants