Skip to content

Fix get_descriptors memory leak - #746

Merged
mrobinson merged 1 commit into
servo:mainfrom
Anthony-Eid:fix-get-descriptors-memory-leak
Jan 15, 2026
Merged

Fix get_descriptors memory leak#746
mrobinson merged 1 commit into
servo:mainfrom
Anthony-Eid:fix-get-descriptors-memory-leak

Conversation

@Anthony-Eid

Copy link
Copy Markdown
Contributor

This function incorrectly assumed that
CTFontCollectionCreateMatchingFontDescriptors doesn't follow the Create rule which is false.

From Apple's docs

A retained reference to an array of normalized font descriptors
matching the collection definition.

I also confirmed using instruments that this was causing a memory leak in Zed.

@madsmtm madsmtm left a comment

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.

Introduced in 4914947 (2012).

Maybe it was a bug in the macOS version at the time?

@Anthony-Eid

Anthony-Eid commented Dec 17, 2025

Copy link
Copy Markdown
Contributor Author

Introduced in 4914947 (2012).

Maybe it was a bug in the macOS version at the time?

That would make sense. To be honest, I didn't check the file history. I just figured "bugs happen" and I should make a PR to fix it. The leak isn't the biggest deal for Zed since we cache the results, but I would still really appreciate it if this got merged.

Anthony-Eid added a commit to zed-industries/zed that referenced this pull request Dec 17, 2025
The below memory leaks were caused by failing to release reference
counted resources. I confirmed using instruments that my changes stopped
the leaks from occurring.

- System prompts 
- Screen capturing 
- loading font families

There were also two memory leaks I found from some of our dependencies
that I made PRs to fix
- RustAudio/coreaudio-rs#147
- servo/core-foundation-rs#746

Release Notes:

- N/A
HactarCE pushed a commit to zed-industries/zed that referenced this pull request Dec 17, 2025
The below memory leaks were caused by failing to release reference
counted resources. I confirmed using instruments that my changes stopped
the leaks from occurring.

- System prompts 
- Screen capturing 
- loading font families

There were also two memory leaks I found from some of our dependencies
that I made PRs to fix
- RustAudio/coreaudio-rs#147
- servo/core-foundation-rs#746

Release Notes:

- N/A
rtfeldman pushed a commit to zed-industries/zed that referenced this pull request Jan 5, 2026
The below memory leaks were caused by failing to release reference
counted resources. I confirmed using instruments that my changes stopped
the leaks from occurring.

- System prompts 
- Screen capturing 
- loading font families

There were also two memory leaks I found from some of our dependencies
that I made PRs to fix
- RustAudio/coreaudio-rs#147
- servo/core-foundation-rs#746

Release Notes:

- N/A
@Anthony-Eid

Copy link
Copy Markdown
Contributor Author

@madsmtm Is there anything else I can do to get this PR merged? Or should I just wait until a reviewer has time?

@jdm
jdm enabled auto-merge January 13, 2026 05:59
@atouchet

Copy link
Copy Markdown
Contributor

CI was fixed in #747. This might need another attempt.

@mrobinson

Copy link
Copy Markdown
Member

@Anthony-Eid Can you rebase your changes and push to this branch or give maintainers access to modify this pull request?

This function incorrectly assumed that
`CTFontCollectionCreateMatchingFontDescriptors` doesn't follow the
*Create* rule which is false.

From Apple's docs

> A retained reference to an array of normalized font descriptors
  matching the collection definition.

https://leopard-adc.pepas.com/documentation/Carbon/Reference/CTFontCollectionRef/Reference/reference.html#//apple_ref/c/func/CTFontCollectionCreateMatchingFontDescriptors

I also confirmed using instruments that this was causing a memory leak
in Zed.
auto-merge was automatically disabled January 15, 2026 20:33

Head branch was pushed to by a user without write access

@Anthony-Eid
Anthony-Eid force-pushed the fix-get-descriptors-memory-leak branch from 33915cd to 6b17d00 Compare January 15, 2026 20:33
@Anthony-Eid

Copy link
Copy Markdown
Contributor Author

@Anthony-Eid Can you rebase your changes and push to this branch or give maintainers access to modify this pull request?

done, please let me know if you need me to do anything else

@mrobinson
mrobinson enabled auto-merge January 15, 2026 21:37
@mrobinson
mrobinson added this pull request to the merge queue Jan 15, 2026
Merged via the queue into servo:main with commit 6f844cf Jan 15, 2026
7 of 8 checks passed
jasonsmithio pushed a commit to paddleboarddev/paddleboard that referenced this pull request May 31, 2026
The below memory leaks were caused by failing to release reference
counted resources. I confirmed using instruments that my changes stopped
the leaks from occurring.

- System prompts 
- Screen capturing 
- loading font families

There were also two memory leaks I found from some of our dependencies
that I made PRs to fix
- RustAudio/coreaudio-rs#147
- servo/core-foundation-rs#746

Release Notes:

- N/A
jonx pushed a commit to jonx/zed-aros that referenced this pull request Jul 17, 2026
The below memory leaks were caused by failing to release reference
counted resources. I confirmed using instruments that my changes stopped
the leaks from occurring.

- System prompts 
- Screen capturing 
- loading font families

There were also two memory leaks I found from some of our dependencies
that I made PRs to fix
- RustAudio/coreaudio-rs#147
- servo/core-foundation-rs#746

Release Notes:

- N/A
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.

5 participants