Hotfix/3.14.4

.github/workflows/run_tests.yaml

@@ -22,7 +22,7 @@ jobs:
     steps:
 
       - name: Checkout the repo 
-        uses: actions/checkout@v4
+        uses: actions/checkout@v7
         with:
           submodules: recursive
 
@@ -103,18 +103,71 @@ jobs:
 #        if: ${{ ! success() }}
 #        uses: mxschmitt/action-tmate@v3
 
+  RunTestsSanitizers:
+
+    # A separate job rather than part of the RunTests matrix: the sanitizers roughly double
+    # the build time and need a different ctest invocation, and there is no reason to slow
+    # down all five platforms.  One Linux runner is enough -- the memory errors these catch
+    # are platform-independent.
+    runs-on: ubuntu-24.04
+
+    steps:
+
+      - name: Checkout the repo
+        uses: actions/checkout@v7
+        with:
+          submodules: recursive
+
+      - name: Install dependencies
+        run: |
+          sudo apt-get update
+          DEBIAN_FRONTEND=noninteractive sudo apt-get install -y \
+            libboost-all-dev \
+            rapidjson-dev \
+            libyaml-cpp-dev
+
+      - name: Configure
+        # The python bindings are off because running the sanitizers through the python
+        # interpreter requires preloading the sanitizer runtime, and adds no coverage here.
+        # CMAKE_BUILD_TYPE is deliberately left unset so that it defaults to DEBUG: NDEBUG
+        # must stay undefined, because LTRACE and LDEBUG compile to nothing when it is set.
+        run: |
+          mkdir build
+          cd build
+          cmake .. -DScarab_ENABLE_TESTING=TRUE -DScarab_ENABLE_SANITIZERS=TRUE -DScarab_BUILD_PYTHON=FALSE
+
+      - name: Build
+        run: |
+          cd build
+          make -j2 install
+
+      - name: Run tests
+        # abort_on_error makes a sanitizer report fail the test rather than only printing.
+        # Leak detection is off: this job targets memory-safety errors, and the existing
+        # tests have not been audited for leaks.
+        # Container-overflow detection is off because it gives false positives unless the
+        # whole standard library is also built with the sanitizer; param_array's deque of
+        # unique_ptrs trips it.  See
+        # https://github.com/google/sanitizers/wiki/AddressSanitizerContainerOverflow
+        env:
+          ASAN_OPTIONS: abort_on_error=1:detect_leaks=0:detect_container_overflow=0
+          UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=1
+        run: |
+          cd build
+          ctest --output-on-failure
+
   Release:
 
     runs-on: ubuntu-22.04
 
     if: ${{ github.event_name == 'push' && contains(github.ref, 'refs/tags/') }}
 
-    needs: [RunTests]
+    needs: [RunTests, RunTestsSanitizers]
 
     steps:
 
       - name: Checkout the repo 
-        uses: actions/checkout@v4
+        uses: actions/checkout@v7
         with:
           submodules: recursive
 

CMakeLists.txt

@@ -44,6 +44,10 @@ set( Scarab_LOGGER_DEFAULT_THRESHOLD "eTrace" CACHE STRING "Default threshold fo
 
 option( Scarab_ENABLE_LOGGER_DEBUG "Flag to enable debug printing for the logger itself" FALSE )
 
+# Applies to the test programs only, so that the installed library's ABI is unchanged.
+# The sanitizer runtime is interposed process-wide, so errors inside the library are still caught.
+option( Scarab_ENABLE_SANITIZERS "Flag to build the test programs with the address and UB sanitizers" FALSE )
+
 if( (Scarab_BUILD_CODEC_JSON OR Scarab_BUILD_CODEC_YAML OR Scarab_BUILD_AUTHENTICATION OR Scarab_BUILD_CLI) AND NOT Scarab_BUILD_PARAM )
     message( FATAL_ERROR "Invalid combination of build options.  Building the Codecs, Authentication, and CLI requires Param.  If you want these elements, turn on Param.  If you want Param off, turn these elements off." )
 endif()

VERSION

@@ -1 +1 @@
-3 14 3
+3 14 4

changelog.md

@@ -10,6 +10,23 @@ Types of changes: Added, Changed, Deprecated, Removed, Fixed, Security
 ## [Unreleased]
 
 
+## [3.14.4] - 2026-09-04
+
+### Added
+
+- Static destruction test for indexed_factory and indexed_registrar
+- Scarab_ENABLE_SANITIZERS CMake option to build the test programs with the address and UB sanitizers, and a CI job that uses it
+
+### Changed
+
+- Updated actions versions (actions/checkout to v7)
+
+### Fixed
+
+- Removed the trace logging from `indexed_factory::remove_class()`, which caused a heap-use-after-free during static destruction: `remove_class()` is called from `~indexed_registrar()`, at which point the static logger it created may already have been destroyed
+- Corrected misleading comments in `indexed_factory::register_class()` claiming a local (non-static) logger was used
+
+
 ## [3.14.3] - 2026-08-21
 
 ### Fixed

library/utility/indexed_factory.hh

@@ -211,7 +211,8 @@ namespace scarab
     template< class XIndexType, class XBaseType, typename ... XArgs >
     void indexed_factory< XIndexType, XBaseType, XArgs... >::register_class( const XIndexType& a_index, const base_registrar< XBaseType, XArgs... >* a_registrar )
     {
-        // A local (non-static) logger is created inside this function to avoid static initialization order problems
+        // A function-local static logger is used so that it is initialized on first use,
+        // avoiding static initialization order problems.
         LOGGER( slog_ind_factory_reg, "indexed_factory_register");
 
         std::unique_lock< std::mutex > t_lock( this->f_factory_mutex );
@@ -234,17 +235,8 @@ namespace scarab
     template< class XIndexType, class XBaseType, typename ... XArgs >
     void indexed_factory< XIndexType, XBaseType, XArgs... >::remove_class(const XIndexType& a_index )
     {
-        // A local (non-static) logger is created inside this function to avoid static destruction problems
-        LOGGER( slog_ind_factory_rem, "indexed_factory_remove");
-        LTRACE( slog_ind_factory_rem, "Removing indexed_factory for class " << a_index << " from " << this );
-        /*
-#ifndef NDEBUG
-        if( ELevel::eTrace >= f_global_threshold )
-        {
-            std::cout << "Removing indexed_factory for class " << a_index << " from " << this << std::endl;
-        }
-#endif
-        */
+        // No logging here: remove_class() is called from ~indexed_registrar() during static
+        // destruction, when a static logger may already have been destroyed.
         FactoryIt iter = fMap->find( a_index );
         if( iter != fMap->end() ) fMap->erase( iter );
         return;
@@ -352,7 +344,8 @@ namespace scarab
     template< class XIndexType, class XBaseType >
     void indexed_factory< XIndexType, XBaseType, void >::register_class( const XIndexType& a_index, const base_registrar< XBaseType >* a_registrar )
     {
-        // A local (non-static) logger is created inside this function to avoid static initialization order problems
+        // A function-local static logger is used so that it is initialized on first use,
+        // avoiding static initialization order problems.
         LOGGER( slog_ind_factory_reg, "indexed_factory_register");
 
         std::unique_lock< std::mutex > t_lock( this->f_factory_mutex );
@@ -376,15 +369,8 @@ namespace scarab
     template< class XIndexType, class XBaseType >
     void indexed_factory< XIndexType, XBaseType, void >::remove_class(const XIndexType& a_index )
     {
-        // A local (non-static) logger is created inside this function to avoid static destruction problems
-        LOGGER( slog_ind_factory_rem, "indexed_factory_remove");
-        LTRACE( slog_ind_factory_rem, "Removing indexed_factory for class " << a_index << " from " << this );
-//#ifndef NDEBUG
-//        if( ELevel::eTrace >= f_global_threshold )
-//        {
-//            std::cout << "Removing indexed_factory for class " << a_index << " from " << this << std::endl;
-//        }
-//#endif
+        // No logging here: remove_class() is called from ~indexed_registrar() during static
+        // destruction, when a static logger may already have been destroyed.
         FactoryIt iter = fMap->find( a_index );
         if( iter != fMap->end() ) fMap->erase( iter );
         return;

testing/CMakeLists.txt

@@ -99,13 +99,20 @@ set( testing_LIB_DEPENDENCIES
     Scarab
 )
 
-pbuilder_executable( 
+pbuilder_executable(
     EXECUTABLE run_tests
     SOURCES ${testing_SOURCES}
     PROJECT_LIBRARIES ${testing_LIB_DEPENDENCIES}
     PRIVATE_EXTERNAL_LIBRARIES Catch2::Catch2
 )
 
+# LINK_FLAGS is used rather than target_link_options because the latter requires CMake 3.13,
+# and cmake_minimum_required is 3.12.
+if( Scarab_ENABLE_SANITIZERS )
+    target_compile_options( run_tests PRIVATE -fsanitize=address,undefined -g )
+    set_property( TARGET run_tests APPEND_STRING PROPERTY LINK_FLAGS " -fsanitize=address,undefined" )
+endif()
+
 ##########
 
 # Other tests

testing/applications/CMakeLists.txt

@@ -12,6 +12,7 @@ set( testing_applications_SOURCES
     test_raise_sigint.cc
     test_raise_sigquit.cc
     test_raise_sigterm.cc
+    test_static_destruction.cc
     test_static_initialization.cc
     test_unhandled_exception.cc
 )
@@ -20,6 +21,16 @@ pbuilder_executables( SOURCES ${testing_applications_SOURCES}
         TARGETS_VAR programs
         PROJECT_LIBRARIES ${testing_LIB_DEPENDENCIES}
 )
+
+# LINK_FLAGS is used rather than target_link_options because the latter requires CMake 3.13,
+# and cmake_minimum_required is 3.12.
+if( Scarab_ENABLE_SANITIZERS )
+    foreach( program ${programs} )
+        target_compile_options( ${program} PRIVATE -fsanitize=address,undefined -g )
+        set_property( TARGET ${program} APPEND_STRING PROPERTY LINK_FLAGS " -fsanitize=address,undefined" )
+    endforeach()
+endif()
+
 set( programs ${programs} PARENT_SCOPE )
 
 if( Scarab_ENABLE_TESTING )
@@ -33,6 +44,7 @@ if( Scarab_ENABLE_TESTING )
     add_test( NAME raise_sigint COMMAND test_raise_sigint WORKING_DIRECTORY ${BIN_INSTALL_DIR} )
     add_test( NAME raise_sigquit COMMAND test_raise_sigquit WORKING_DIRECTORY ${BIN_INSTALL_DIR} )
     add_test( NAME raise_sigterm COMMAND test_raise_sigterm WORKING_DIRECTORY ${BIN_INSTALL_DIR} )
+    add_test( NAME static_destruction COMMAND test_static_destruction WORKING_DIRECTORY ${BIN_INSTALL_DIR} )
     add_test( NAME static_initialization COMMAND test_static_initialization WORKING_DIRECTORY ${BIN_INSTALL_DIR} )
     add_test( NAME unhandled_exception COMMAND test_unhandled_exception WORKING_DIRECTORY ${BIN_INSTALL_DIR} )
 

testing/applications/test_static_destruction.cc

@@ -0,0 +1,50 @@
+/*
+ * test_static_destruction.cc
+ *
+ *  Created on: Sep 4, 2026
+ *      Author: N.S. Oblath
+ *
+ *  Use:
+ *    > bin/test_static_destruction
+ *
+ *  Regression test for a heap-use-after-free during static destruction.
+ *
+ *  A registrar un-registers itself in its destructor, calling indexed_factory::remove_class().
+ *  For a static registrar that happens during static destruction, so remove_class() must not
+ *  log: a logger is itself a static object and may already have been destroyed.
+ *
+ *  Two registrars are required to reproduce the failure.  Statics are destroyed in reverse
+ *  order of construction, so a logger created inside remove_class() during the destruction
+ *  phase is destroyed almost immediately afterward -- the first registrar creates it, and
+ *  the second then writes into freed memory.  With only one registrar nothing follows it,
+ *  and the bug does not appear.
+ *
+ *  This program exits successfully either way unless it is built with the sanitizers
+ *  (Scarab_ENABLE_SANITIZERS=TRUE), which is what turns the invalid write into a failure.
+ *  It must also be built without NDEBUG, since LTRACE compiles to nothing when NDEBUG is set.
+ */
+
+#include "factory.hh"
+
+#include <string>
+
+namespace scarab
+{
+    struct test_base
+    {
+        virtual ~test_base() {}
+    };
+
+    struct test_derived_1 : test_base {};
+    struct test_derived_2 : test_base {};
+}
+
+// Both registrars must be static and use the same factory instantiation so that the
+// second one's destructor runs after the first one has already created the logger.
+static scarab::registrar< scarab::test_base, scarab::test_derived_1 > s_reg_1( "test_derived_1" );
+static scarab::registrar< scarab::test_base, scarab::test_derived_2 > s_reg_2( "test_derived_2" );
+
+int main(int , char ** )
+{
+    return( EXIT_SUCCESS );
+}