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

Signed-off-by: Larry Gritz <lg@larrygritz.com>

Author: lgritz

src/include/OpenImageIO/imageio.h

@@ -3968,6 +3968,15 @@ 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.
+///
 /// EXAMPLES:
 /// ```
 ///     // Setting single simple values simply:

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<float> IB_total_open_time;
 extern std::atomic<float> 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<std::string>&
 font_dirs();
 OIIO_API const std::vector<std::string>&

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) {

src/libutil/ustring.cpp

@@ -8,10 +8,29 @@
 #include <OpenImageIO/dassert.h>
 #include <OpenImageIO/export.h>
 #include <OpenImageIO/strutil.h>
+#include <OpenImageIO/sysutil.h>
 #include <OpenImageIO/thread.h>
 #include <OpenImageIO/unordered_map_concurrent.h>
 #include <OpenImageIO/ustring.h>
 
+
+
+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,26 @@ template<unsigned BASE_CAPACITY, unsigned POOL_SIZE> 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 also the unique_ptr arrays all_pools and
+            // large_allocs will naturally free their contents after this
+            // destructor body ends.
+            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 +222,16 @@ template<unsigned BASE_CAPACITY, unsigned POOL_SIZE> 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 +245,7 @@ template<unsigned BASE_CAPACITY, unsigned POOL_SIZE> struct TableRepMap {
     char* pool;
     size_t pool_offset = 0;
     size_t memory_usage;
+    std::vector<char*> pool_allocs;
 #ifdef USTRING_TRACK_NUM_LOOKUPS
     size_t num_lookups = 0;
 #endif