Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a memory leak with Parley fonts #115

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DavidHusicka
Copy link
Contributor

Using a self-referencing struct and bunch of unsafe, we are able to extend the lifetime of the font names and their array without leaking them. These are necessary for a TextStyle which is part of Parley. This creates a shim called "OwnedTextStyle" that owns the data.

Using a self-referencing struct and bunch of unsafe, we are able to extend the lifetime of the font names and their array without leaking them. These are necessary for a TextStyle which is part of Parley. This creates a shim called "OwnedTextStyle" that owns the data.
@nicoburns
Copy link
Collaborator

Using a self-referencing struct and bunch of unsafe, we are able to extend the lifetime of the font names and their array without leaking them. These are necessary for a TextStyle which is part of Parley. This creates a shim called "OwnedTextStyle" that owns the data.

Hmm... all the unsafe code with tricky lifetimes doesn't seem ideal. How about instead of a self-referencing struct, the names are stored in a document-global HashSet (or similar- perhaps a HashSet wouldn't work). There are unlikely to be many unique font names used within a single Document so this shouldn't use too much memory.

Or even a short-lived datastructure (that is passed in to the stylo_to_parley function so it is high enough up the stack for the lifetimes to work. I believe the FamilyNames style only needs to live until it is used (the reference is not stored).

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