Skip to content

Commit f46a123

Browse files
committed
feat: allow freeing of ustring table for debugging cases
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 Assisted-by: Claude Code / Sonnet 4.6 Signed-off-by: Larry Gritz <lg@larrygritz.com>
1 parent ddba92f commit f46a123

3 files changed

Lines changed: 93 additions & 9 deletions

File tree

src/include/imageio_pvt.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,11 @@ extern atomic_ll IB_local_mem_current;
5353
extern atomic_ll IB_local_mem_peak;
5454
extern std::atomic<float> IB_total_open_time;
5555
extern std::atomic<float> IB_total_image_read_time;
56-
extern OIIO_UTIL_API int oiio_use_tbb; // This lives in libOpenImageIO_Util
56+
57+
// These live in libOpenImageIO_Util
58+
extern OIIO_UTIL_API int oiio_use_tbb;
59+
extern OIIO_UTIL_API int oiio_ustring_cleanup;
60+
5761
OIIO_API const std::vector<std::string>&
5862
font_dirs();
5963
OIIO_API const std::vector<std::string>&

src/libOpenImageIO/imageio.cpp

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,10 @@ attribute(string_view name, TypeDesc type, const void* val)
373373

374374
// Things below here need to buarded by the attrib_mutex
375375
std::lock_guard lock(attrib_mutex);
376+
if (name == "debug" && type == TypeInt) {
377+
oiio_print_debug = *(const int*)val;
378+
return true;
379+
}
376380
if (name == "read_chunk" && type == TypeInt) {
377381
oiio_read_chunk = *(const int*)val;
378382
return true;
@@ -447,10 +451,6 @@ attribute(string_view name, TypeDesc type, const void* val)
447451
oiio_use_tbb = *(const int*)val;
448452
return true;
449453
}
450-
if (name == "debug" && type == TypeInt) {
451-
oiio_print_debug = *(const int*)val;
452-
return true;
453-
}
454454
if (name == "log_times" && type == TypeInt) {
455455
oiio_log_times = *(const int*)val;
456456
return true;
@@ -471,6 +471,10 @@ attribute(string_view name, TypeDesc type, const void* val)
471471
oiio_try_all_readers = *(const int*)val;
472472
return true;
473473
}
474+
if (name == "ustring:cleanup" && type == TypeInt) {
475+
oiio_ustring_cleanup = *(const int*)val;
476+
return true;
477+
}
474478

475479
return false;
476480
}
@@ -511,6 +515,10 @@ getattribute(string_view name, TypeDesc type, void* val)
511515

512516
// Things below here need to buarded by the attrib_mutex
513517
std::lock_guard lock(attrib_mutex);
518+
if (name == "debug" && type == TypeInt) {
519+
*(int*)val = oiio_print_debug;
520+
return true;
521+
}
514522
if (name == "read_chunk" && type == TypeInt) {
515523
*(int*)val = oiio_read_chunk;
516524
return true;
@@ -649,8 +657,8 @@ getattribute(string_view name, TypeDesc type, void* val)
649657
*(int*)val = oiio_use_tbb;
650658
return true;
651659
}
652-
if (name == "debug" && type == TypeInt) {
653-
*(int*)val = oiio_print_debug;
660+
if (name == "ustring:cleanup" && type == TypeInt) {
661+
*(int*)val = oiio_ustring_cleanup;
654662
return true;
655663
}
656664
if (name == "log_times" && type == TypeInt) {

src/libutil/ustring.cpp

Lines changed: 74 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,15 @@
22
// SPDX-License-Identifier: Apache-2.0
33
// https://github.com/AcademySoftwareFoundation/OpenImageIO
44

5+
#include <cstdlib>
56
#include <string>
67
#include <unordered_map>
8+
#include <vector>
79

810
#include <OpenImageIO/dassert.h>
911
#include <OpenImageIO/export.h>
1012
#include <OpenImageIO/strutil.h>
13+
#include <OpenImageIO/sysutil.h>
1114
#include <OpenImageIO/thread.h>
1215
#include <OpenImageIO/unordered_map_concurrent.h>
1316
#include <OpenImageIO/ustring.h>
@@ -43,12 +46,42 @@ template<unsigned BASE_CAPACITY, unsigned POOL_SIZE> struct TableRepMap {
4346
, memory_usage(sizeof(*this) + POOL_SIZE
4447
+ sizeof(ustring::TableRep*) * BASE_CAPACITY)
4548
{
49+
all_pools.push_back(pool);
4650
}
4751

4852
~TableRepMap()
4953
{ /* just let memory leak */
5054
}
5155

56+
void clear()
57+
{
58+
ustring_write_lock_t lock(mutex);
59+
// Destroy all TableRep objects. The destructor safely handles the
60+
// case where the internal std::string aliases the pool chars.
61+
for (size_t i = 0; i <= mask; ++i) {
62+
if (entries[i])
63+
entries[i]->~TableRep();
64+
}
65+
// Free all pool allocations and large individual allocations.
66+
for (char* p : all_pools)
67+
free(p);
68+
all_pools.clear();
69+
for (char* p : large_allocs)
70+
free(p);
71+
large_allocs.clear();
72+
free(entries);
73+
// Re-initialize to a fresh, usable state.
74+
mask = BASE_CAPACITY - 1;
75+
entries = static_cast<ustring::TableRep**>(
76+
calloc(BASE_CAPACITY, sizeof(ustring::TableRep*)));
77+
pool = static_cast<char*>(malloc(POOL_SIZE));
78+
pool_offset = 0;
79+
num_entries = 0;
80+
memory_usage = sizeof(*this) + POOL_SIZE
81+
+ sizeof(ustring::TableRep*) * BASE_CAPACITY;
82+
all_pools.push_back(pool);
83+
}
84+
5285
size_t get_memory_usage()
5386
{
5487
ustring_read_lock_t lock(mutex);
@@ -187,13 +220,15 @@ template<unsigned BASE_CAPACITY, unsigned POOL_SIZE> struct TableRepMap {
187220

188221
if (len >= POOL_SIZE) {
189222
memory_usage += len;
190-
return (char*)malloc(len); // no need to try and use the pool
223+
char* p = (char*)malloc(len);
224+
large_allocs.push_back(p);
225+
return p;
191226
}
192227
if (pool_offset + len > POOL_SIZE) {
193-
// NOTE: old pool will leak - this is ok because ustrings cannot be freed
194228
memory_usage += POOL_SIZE;
195229
pool = (char*)malloc(POOL_SIZE);
196230
pool_offset = 0;
231+
all_pools.push_back(pool);
197232
}
198233
char* result = pool + pool_offset;
199234
pool_offset += len;
@@ -207,6 +242,8 @@ template<unsigned BASE_CAPACITY, unsigned POOL_SIZE> struct TableRepMap {
207242
char* pool;
208243
size_t pool_offset = 0;
209244
size_t memory_usage;
245+
std::vector<char*> all_pools;
246+
std::vector<char*> large_allocs;
210247
#ifdef USTRING_TRACK_NUM_LOOKUPS
211248
size_t num_lookups = 0;
212249
#endif
@@ -249,6 +286,12 @@ struct UstringTable {
249286
return num;
250287
}
251288

289+
void clear()
290+
{
291+
for (auto& bin : bins)
292+
bin.clear();
293+
}
294+
252295
# ifdef USTRING_TRACK_NUM_LOOKUPS
253296
size_t get_num_lookups()
254297
{
@@ -678,3 +721,32 @@ ustring::memory()
678721
}
679722

680723
OIIO_NAMESPACE_3_1_END
724+
725+
726+
OIIO_NAMESPACE_BEGIN
727+
namespace pvt {
728+
729+
// If nonzero, the ustring table will be freed at process exit. This is off by
730+
// default because cleanup is unnecessary (the OS reclaims the memory) and can
731+
// add measurable time at exit for large tables. Enable it when using valgrind
732+
// or other leak detectors to suppress false positives. Settable via
733+
// OIIO::attribute("ustring:cleanup",1) or the OIIO_USTRING_CLEANUP
734+
// environment variable.
735+
OIIO_UTIL_API int oiio_ustring_cleanup = Strutil::stoi(
736+
Sysutil::getenv("OIIO_USTRING_CLEANUP"));
737+
738+
// Register an atexit handler once at startup. The handler checks the flag at
739+
// exit time, so it covers both the env-var path (flag set here) and the
740+
// OIIO::attribute() path (flag set later at runtime).
741+
static int ustring_cleanup_atexit_registered = []() {
742+
std::atexit([]() {
743+
if (pvt::oiio_ustring_cleanup) {
744+
v3_1::ustring_table().clear();
745+
v3_1::reverse_map().clear();
746+
}
747+
});
748+
return 0;
749+
}();
750+
751+
} // namespace pvt
752+
OIIO_NAMESPACE_END

0 commit comments

Comments
 (0)