Skip to content

feat: allow freeing of ustring table for debugging cases#5213

Open
lgritz wants to merge 1 commit into
AcademySoftwareFoundation:mainfrom
lgritz:lg-ustringcleanup2
Open

feat: allow freeing of ustring table for debugging cases#5213
lgritz wants to merge 1 commit into
AcademySoftwareFoundation:mainfrom
lgritz:lg-ustringcleanup2

Conversation

@lgritz
Copy link
Copy Markdown
Collaborator

@lgritz lgritz commented May 23, 2026

This patch optionally frees the contents of the ustring table as the process is exiting. It's triggered one of two ways:

  • OIIO::attribute("ustring:cleanup", 1);
  • Environment variable OIIO_USTRING_CLEANUP=1

If neither is used, the teardown won't happen, thus avoiding any pointless work when it should be exiting the process.

The purpose of this is that when using certain debugging tools, the ustring table (which only grows and never frees) looks like a memory leak, even though it's by design. Someimes, it's useful to make it not appear ot leak, and you are happy for a large table to spend time pointlessly freeing all the data.

Fixes #3913

@lgritz
Copy link
Copy Markdown
Collaborator Author

lgritz commented May 23, 2026

Another independent stab that is an alternate to #5193. This PR still establishes the same external API for requesting that the ustring table be cleaned up, but I change the absolute minimal amount of the existing code.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds an opt-in shutdown-time cleanup path for the global ustring table so that leak-detection tools (e.g. valgrind) don’t report the intentionally-retained ustring storage as leaks. The cleanup is enabled either via OIIO::attribute("ustring:cleanup", 1) or the OIIO_USTRING_CLEANUP environment variable.

Changes:

  • Introduce a global oiio_ustring_cleanup flag (env-var initialized) and add optional teardown logic to ustring table destruction.
  • Wire the new "ustring:cleanup" option into OIIO::attribute() / OIIO::getattribute().
  • Document the new attribute in the public API header.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
src/libutil/ustring.cpp Adds the cleanup flag and implements optional destruction/freeing of table entries and pools at exit.
src/libOpenImageIO/imageio.cpp Exposes "ustring:cleanup" through global attribute/getattribute plumbing.
src/include/OpenImageIO/imageio.h Documents the new global attribute.
src/include/imageio_pvt.h Declares the exported cleanup flag for cross-library access.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/libutil/ustring.cpp Outdated
Comment thread src/libutil/ustring.cpp Outdated
Comment thread src/include/OpenImageIO/imageio.h Outdated
Comment thread src/libOpenImageIO/imageio.cpp
This patch optionally frees the contents of the ustring table as the
process is exiting. It's triggered one of two ways:

- `OIIO::attribute("ustring:cleanup", 1);`
- Environment variable `OIIO_USTRING_CLEANUP=1`

If neither is used, the teardown won't happen, thus avoiding any
pointless work when it should be exiting the process.

The purpose of this is that when using certain debugging tools, the
ustring table (which only grows and never frees) looks like a memory
leak, even though it's by design. Someimes, it's useful to make it not
appear ot leak, and you are happy for a large table to spend time
pointlessly freeing all the data.

Fixes 3913

Signed-off-by: Larry Gritz <lg@larrygritz.com>
@lgritz lgritz force-pushed the lg-ustringcleanup2 branch from e2e216c to 6eb8e19 Compare May 23, 2026 05:34
@lgritz
Copy link
Copy Markdown
Collaborator Author

lgritz commented May 23, 2026

@fpsunflower Is this one more to your liking? It's more of a truly minimal modification to the existing ustring.cpp code.

And @jreichel-nvidia, so sorry, but could you also try this one and verify that it squashes all the detectable leaks?

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.

memory leak in ustring

2 participants