diff --git a/src/include/OpenImageIO/imageio.h b/src/include/OpenImageIO/imageio.h index 82fa598969..a7decea696 100644 --- a/src/include/OpenImageIO/imageio.h +++ b/src/include/OpenImageIO/imageio.h @@ -3968,6 +3968,16 @@ OIIO_API std::string geterror(bool clear = true); /// enable globally in an environment where security is a higher priority /// than being tolerant of partially broken image files. /// +/// - `ustring:cleanup` (int: 0) +/// +/// If nonzero, upon exit, do a thorough (and possibly expensive) teardown +/// of ustring internal resources to ensure that there are no apparent +/// memory leaks. This is only desirable in certain debugging situations. +/// Ordinarily, it is better to finish as quickly as possible, so the +/// default of 0 skips a time consuming and pointless teardown of the +/// ustring internal allocations when the app exits. Note that this can +/// also be enabled with the `OIIO_USTRING_CLEANUP` environment variable. +/// /// EXAMPLES: /// ``` /// // Setting single simple values simply: diff --git a/src/include/imageio_pvt.h b/src/include/imageio_pvt.h index c490a0a42e..eb1aebb798 100644 --- a/src/include/imageio_pvt.h +++ b/src/include/imageio_pvt.h @@ -53,7 +53,11 @@ extern atomic_ll IB_local_mem_current; extern atomic_ll IB_local_mem_peak; extern std::atomic IB_total_open_time; extern std::atomic IB_total_image_read_time; -extern OIIO_UTIL_API int oiio_use_tbb; // This lives in libOpenImageIO_Util + +// These live in libOpenImageIO_Util +extern OIIO_UTIL_API int oiio_use_tbb; +extern OIIO_UTIL_API int oiio_ustring_cleanup; + OIIO_API const std::vector& font_dirs(); OIIO_API const std::vector& diff --git a/src/libOpenImageIO/imageio.cpp b/src/libOpenImageIO/imageio.cpp index 95a430cfe9..265aee896a 100644 --- a/src/libOpenImageIO/imageio.cpp +++ b/src/libOpenImageIO/imageio.cpp @@ -373,6 +373,10 @@ attribute(string_view name, TypeDesc type, const void* val) // Things below here need to buarded by the attrib_mutex std::lock_guard lock(attrib_mutex); + if (name == "debug" && type == TypeInt) { + oiio_print_debug = *(const int*)val; + return true; + } if (name == "read_chunk" && type == TypeInt) { oiio_read_chunk = *(const int*)val; return true; @@ -447,10 +451,6 @@ attribute(string_view name, TypeDesc type, const void* val) oiio_use_tbb = *(const int*)val; return true; } - if (name == "debug" && type == TypeInt) { - oiio_print_debug = *(const int*)val; - return true; - } if (name == "log_times" && type == TypeInt) { oiio_log_times = *(const int*)val; return true; @@ -471,6 +471,10 @@ attribute(string_view name, TypeDesc type, const void* val) oiio_try_all_readers = *(const int*)val; return true; } + if (name == "ustring:cleanup" && type == TypeInt) { + oiio_ustring_cleanup = *(const int*)val; + return true; + } return false; } @@ -511,6 +515,10 @@ getattribute(string_view name, TypeDesc type, void* val) // Things below here need to buarded by the attrib_mutex std::lock_guard lock(attrib_mutex); + if (name == "debug" && type == TypeInt) { + *(int*)val = oiio_print_debug; + return true; + } if (name == "read_chunk" && type == TypeInt) { *(int*)val = oiio_read_chunk; return true; @@ -649,8 +657,8 @@ getattribute(string_view name, TypeDesc type, void* val) *(int*)val = oiio_use_tbb; return true; } - if (name == "debug" && type == TypeInt) { - *(int*)val = oiio_print_debug; + if (name == "ustring:cleanup" && type == TypeInt) { + *(int*)val = oiio_ustring_cleanup; return true; } if (name == "log_times" && type == TypeInt) { diff --git a/src/libutil/ustring.cpp b/src/libutil/ustring.cpp index 043ce43dfb..ec255bf1dc 100644 --- a/src/libutil/ustring.cpp +++ b/src/libutil/ustring.cpp @@ -8,10 +8,29 @@ #include #include #include +#include #include #include #include + + +OIIO_NAMESPACE_BEGIN +namespace pvt { + +// If nonzero, the ustring table will be freed at process exit. This is off by +// default because cleanup is unnecessary (the OS reclaims the memory) and can +// add measurable time at exit for large tables. Enable it when using valgrind +// or other leak detectors to suppress false positives. Settable via +// OIIO::attribute("ustring:cleanup",1) or the OIIO_USTRING_CLEANUP +// environment variable. +OIIO_UTIL_API int oiio_ustring_cleanup = Strutil::stoi( + Sysutil::getenv("OIIO_USTRING_CLEANUP")); + +} // namespace pvt +OIIO_NAMESPACE_END + + OIIO_NAMESPACE_3_1_BEGIN // Use rw spin locks @@ -43,10 +62,25 @@ template struct TableRepMap { , memory_usage(sizeof(*this) + POOL_SIZE + sizeof(ustring::TableRep*) * BASE_CAPACITY) { + pool_allocs.push_back(pool); } ~TableRepMap() - { /* just let memory leak */ + { + if (OIIO::pvt::oiio_ustring_cleanup) { + // If requested, take the time to properly destroy all the entries + // and everything we malloced. + ustring_write_lock_t lock(mutex); + for (size_t i = 0; i <= mask; ++i) { + if (entries[i]) + entries[i]->~TableRep(); + } + free(entries); + for (auto p : pool_allocs) + free(p); + } else { + /* just let memory leak */ + } } size_t get_memory_usage() @@ -187,13 +221,16 @@ template struct TableRepMap { if (len >= POOL_SIZE) { memory_usage += len; - return (char*)malloc(len); // no need to try and use the pool + char* r = (char*)malloc(len); // no need to try and use the pool + pool_allocs.push_back(r); + return r; } if (pool_offset + len > POOL_SIZE) { // NOTE: old pool will leak - this is ok because ustrings cannot be freed memory_usage += POOL_SIZE; pool = (char*)malloc(POOL_SIZE); pool_offset = 0; + pool_allocs.push_back(pool); } char* result = pool + pool_offset; pool_offset += len; @@ -207,6 +244,7 @@ template struct TableRepMap { char* pool; size_t pool_offset = 0; size_t memory_usage; + std::vector pool_allocs; #ifdef USTRING_TRACK_NUM_LOOKUPS size_t num_lookups = 0; #endif