[Python] Add ASan/LSan coverage for bindings and C++ integration (#1047)

* [Python] Add ASan/LSan coverage for bindings and C++ integration (#1027)

Add a coordinated sanitizer build mode, -Dtsfile.sanitizer=<checks>, that
instruments libtsfile and every Cython extension with the same checks. The
C++ library gets -fsanitize on compile and on the shared-library link and
keeps debug info and frame pointers; setup.py builds each extension with
-O1 -g -fno-omit-frame-pointer, links the sanitizer runtime, and verifies
with nm/readelf that every artifact is instrumented. Sanitizer builds never
produce a wheel.

The sanitizer compile flags must be re-applied after the bundled third-party
build resets CMAKE_CXX_FLAGS; previously they were discarded and only the
link step saw them. The C++-only ASan mode keeps its historical static-runtime
setup so an instrumented libtsfile cannot mix runtimes with the test binary.

Fix ownership defects the new checks expose:
- free STRING/TEXT value buffers and use the length-aware binary getter for
  BLOB so embedded NUL bytes survive;
- free the writer-side schema structs, not only their members;
- free result-set metadata and schema arrays on every path;
- free the property array on the C wrapper's bad_alloc path;
- add tsfile_free_buffer so bindings release buffers with the allocating
  module's CRT.

Add subprocess ownership scenarios (tree/table/dataset/Arrow reads, value
types and NULLs, early termination, exception cleanup, repeated open/close,
reader/result lifetime, file-object adapter), two positive controls, narrow
suppressions, and a Linux CI job. Leak reports are attributed by allocation
stack so only leaks reaching TsFile modules fail the run. Document build and
run commands, runtime preloading (libasan before libstdc++), symbolization,
and the suppression policy.

* [Python] Fix sanitizer workflow gaps and reader schema-array ownership

- Skip the ordinary pytest execution in the with-sanitizer Maven profile:
  the instrumented extensions abort on import unless the sanitizer runtime
  is preloaded, which Maven's test execution does not do, so
  `mvn verify -Dtsfile.sanitizer=address` failed before running a test.
- Document the complete ASan/LSan command in python/CLAUDE.md, including
  TSFILE_SANITIZER (without it test_sanitizer_ownership.py skips silently)
  and the ASAN/LSAN options used by CI.
- Add tsfile_free_table_schema_array()/tsfile_free_device_schema_array()
  so bindings release reader-owned schema arrays with libtsfile's
  allocator, and release partial conversion results when a schema
  conversion raises mid-loop.
- Point the C API value-getter docs at tsfile_free_buffer() and document
  the binary getter's NULL-cell behavior.
- Note the nm/binutils wheel requirement and the test/verify limitation in
  python/SANITIZER.md.

Author: ColinLeeo

.github/workflows/unit-test-python.yml

@@ -128,3 +128,84 @@ jobs:
           name: tsfile-${{ runner.os }}-py${{ matrix.python-version }}-whl
           path: python/dist/tsfile-*.whl
           retention-days: 1
+
+  # AddressSanitizer/LeakSanitizer coverage for the complete Python -> Cython
+  # -> C wrapper -> C++ chain. The build instruments libtsfile and every
+  # extension with the same compiler and check set; the scenarios run in fresh
+  # interpreters with leak detection on. See python/SANITIZER.md.
+  python-sanitizer:
+    runs-on: ubuntu-24.04
+    timeout-minutes: 60
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v7
+
+      - name: Set up Python
+        uses: actions/setup-python@v7
+        with:
+          python-version: "3.11"
+
+      - name: Set up JDK 17
+        uses: actions/setup-java@v6.0.1
+        with:
+          distribution: corretto
+          java-version: 17
+
+      - name: Cache Maven packages
+        uses: actions/cache@v6
+        with:
+          path: ~/.m2
+          key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }}
+          restore-keys: ${{ runner.os }}-m2-
+
+      - name: Install dependencies
+        shell: bash
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y uuid-dev binutils
+
+      - name: Build with ASan/LSan
+        shell: bash
+        run: |
+          # One compiler (and therefore one sanitizer runtime) for CMake,
+          # distutils and the preload below.
+          export CC=gcc CXX=g++
+          ./mvnw -P with-python \
+            -Dtsfile.sanitizer=address \
+            -Dbuild.type=RelWithDebInfo \
+            -Dbuild.test=OFF \
+            -Dspotless.skip=true \
+            clean test-compile
+
+      - name: Verify sanitizer instrumentation
+        shell: bash
+        run: |
+          python/venv/bin/python python/check_sanitizer_build.py python/tsfile
+
+      - name: Run ownership scenarios under ASan/LSan
+        shell: bash
+        run: |
+          set -o pipefail
+          # Suppressions must stay scoped to third-party/interpreter frames.
+          if grep -v '^#' python/tests/sanitizer/lsan_suppressions.txt \
+              | grep -Eqi 'tsfile|dataset/_merge|dataset/_index'; then
+            echo "::error::suppression file must not match TsFile modules"
+            exit 1
+          fi
+          ASAN_LIB="$(gcc -print-file-name=libasan.so)"
+          LIBSTDCXX="$(gcc -print-file-name=libstdc++.so.6)"
+          if [ ! -f "$ASAN_LIB" ] || [ ! -f "$LIBSTDCXX" ]; then
+            echo "::error::AddressSanitizer or libstdc++ runtime not found for gcc"
+            exit 1
+          fi
+          # The pytest runner itself does not load tsfile; each scenario child
+          # re-enables leak detection (see test_sanitizer_ownership.py).
+          #
+          # libasan must stay first (runtime-first check); libstdc++ follows so
+          # ASan's __cxa_throw interceptor can resolve the real symbol. Without
+          # it, any C++ exception aborts with "CHECK failed ... __cxa_throw".
+          export TSFILE_SANITIZER=address
+          export ASAN_OPTIONS="detect_leaks=0:abort_on_error=1:halt_on_error=1"
+          export LSAN_OPTIONS="suppressions=$PWD/python/tests/sanitizer/lsan_suppressions.txt"
+          LD_PRELOAD="$ASAN_LIB:$LIBSTDCXX" python/venv/bin/python -m pytest \
+            python/tests/test_sanitizer_ownership.py -v

cpp/CLAUDE.md

@@ -33,6 +33,8 @@ bash build.sh
 bash build.sh -t=Debug
 bash build.sh -t=RelWithDebInfo
 bash build.sh -a=ON              # Enable AddressSanitizer
+bash build.sh -a=ON --sanitizer-shared-runtime=ON \
+  --sanitizer-checks=address     # For the Python bindings (see python/SANITIZER.md)
 bash build.sh -c=ON              # Enable code coverage
 
 # Disable optional compression libraries
@@ -69,6 +71,8 @@ All `ON` by default unless noted:
 | `ENABLE_LZMA2` | LZMA2 compression (`OFF` by default; bundled XZ requires CMake 3.20+) |
 | `ENABLE_THREADS` | Multi-threaded read/write via pthreads |
 | `ENABLE_ASAN` | AddressSanitizer (`OFF` by default) |
+| `TSFILE_SANITIZER_CHECKS` | `-fsanitize=` list used by `ENABLE_ASAN` (default `address,undefined`) |
+| `TSFILE_SANITIZER_SHARED_RUNTIME` | Link the sanitizer runtime dynamically into shared libraries; required when Python bindings preload it (`OFF` by default) |
 | `ENABLE_SIMD` | SIMD acceleration |
 
 ## Source Structure

cpp/CMakeLists.txt

@@ -155,6 +155,20 @@ if (ENABLE_MEM_STAT)
 endif ()
 
 
+# Sanitizer configuration. Declared before the build-type block because the
+# Release flags below must know whether LTO may be used.
+option(ENABLE_ASAN "Enable Address Sanitizer" OFF)
+set(TSFILE_SANITIZER_CHECKS "address,undefined"
+        CACHE STRING
+        "Sanitizer checks passed to -fsanitize= when ENABLE_ASAN is ON (for example 'address' or 'address,undefined')")
+# Python bindings preload the sanitizer runtime, so instrumented shared
+# libraries must not embed a private copy of it. Keep this OFF for the C++ test
+# builds that use -static-libasan, and turn it ON for the coordinated
+# C++/Python binding build.
+option(TSFILE_SANITIZER_SHARED_RUNTIME
+        "Link the sanitizer runtime dynamically into shared libraries (for preloaded Python bindings)"
+        OFF)
+
 if (NOT CMAKE_BUILD_TYPE)
     set(CMAKE_BUILD_TYPE "Release" CACHE STRING "Choose the type of build." FORCE)
 endif ()
@@ -212,7 +226,14 @@ else ()
             # (see TSFILE_ENABLE_NATIVE_ARCH above): the default target for the
             # deployment platform is the portable baseline, and on macOS arm64
             # it still includes +crc so snappy's fast path stays available.
-            if (MINGW OR WIN32)
+            if (ENABLE_ASAN AND TSFILE_SANITIZER_SHARED_RUNTIME)
+                # LTO loses the stack frames sanitizer reports need, and its
+                # additional optimization pipeline complicates symbolization.
+                # Only the binding mode changes optimization: the C++ test mode
+                # keeps its historical flags so its static runtime stays in
+                # sync with how libtsfile is linked.
+                set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -O3")
+            elseif (MINGW OR WIN32)
                 set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -O3")
             elseif (TSFILE_ENABLE_NATIVE_ARCH)
                 set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -O3 -march=native -flto")
@@ -229,11 +250,9 @@ else ()
 endif ()
 message("CMAKE DEBUG: CMAKE_CXX_FLAGS=${CMAKE_CXX_FLAGS}")
 
-# disable asan by default.
-option(ENABLE_ASAN "Enable Address Sanitizer" OFF)
-
 if (ENABLE_ASAN)
-    message("Address Sanitizer is enabled.")
+    message("Address Sanitizer is enabled: checks=${TSFILE_SANITIZER_CHECKS}, "
+            "shared_runtime=${TSFILE_SANITIZER_SHARED_RUNTIME}")
     if (MSVC)
         # MSVC ships AddressSanitizer; it requires Visual Studio 2019 16.9 or
         # newer (MSVC_VERSION >= 1928). Only the address sanitizer is available
@@ -259,13 +278,32 @@ if (ENABLE_ASAN)
         endforeach ()
         add_link_options(/INCREMENTAL:NO /DEBUG)
     elseif (NOT WIN32)
-        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address,undefined -fno-omit-frame-pointer")
-
-        if (NOT APPLE)
+        # Compile flags here only survive until add_subdirectory(third_party)
+        # resets CMAKE_CXX_FLAGS. The binding mode re-applies them after that
+        # call; the C++ test mode keeps its historical behavior. Link flags set
+        # here persist.
+        set(CMAKE_CXX_FLAGS
+                "${CMAKE_CXX_FLAGS} -fsanitize=${TSFILE_SANITIZER_CHECKS} -fno-omit-frame-pointer")
+
+        if (TSFILE_SANITIZER_SHARED_RUNTIME)
+            # The runtime stays dynamic so one preloaded copy serves Python, the
+            # extensions and libtsfile. Debug info is required for useful
+            # reports in every build type.
+            set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g")
+            set(CMAKE_SHARED_LINKER_FLAGS
+                    "${CMAKE_SHARED_LINKER_FLAGS} -fsanitize=${TSFILE_SANITIZER_CHECKS}")
+            set(CMAKE_EXE_LINKER_FLAGS
+                    "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=${TSFILE_SANITIZER_CHECKS}")
+        elseif (NOT APPLE)
             set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -static-libasan")
-            set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=address,undefined -static-libasan -static-libubsan")
+            set(CMAKE_EXE_LINKER_FLAGS
+                    "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=${TSFILE_SANITIZER_CHECKS} -static-libasan")
+            if (TSFILE_SANITIZER_CHECKS MATCHES "undefined")
+                set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -static-libubsan")
+            endif ()
         else ()
-            set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=address,undefined")
+            set(CMAKE_EXE_LINKER_FLAGS
+                    "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=${TSFILE_SANITIZER_CHECKS}")
         endif ()
     else ()
         message(WARNING
@@ -397,6 +435,16 @@ else ()
 endif ()
 add_subdirectory(third_party)
 
+# third_party intentionally rebuilds with a minimal CMAKE_CXX_FLAGS so bundled
+# projects do not inherit TsFile's warning flags. The binding mode re-applies
+# the sanitizer instrumentation for the TsFile targets that follow. The C++
+# test mode keeps its historical static-runtime setup, so re-applying here
+# would mix a dynamic runtime in libtsfile with the executable's static one.
+if (ENABLE_ASAN AND TSFILE_SANITIZER_SHARED_RUNTIME AND NOT MSVC AND NOT WIN32)
+    set(CMAKE_CXX_FLAGS
+            "${CMAKE_CXX_FLAGS} -fsanitize=${TSFILE_SANITIZER_CHECKS} -fno-omit-frame-pointer -g")
+endif ()
+
 # CMake 3.11 cannot call target_link_libraries() on object-library targets, but
 # directory-level link_libraries() initializes both their usage requirements
 # and the final library's link dependencies. Apply every enabled project

cpp/build.sh

@@ -28,6 +28,9 @@ enable_cov=0
 debug_se=0
 run_cov_only=0
 enable_antlr4=ON
+enable_asan=OFF
+sanitizer_checks=address,undefined
+sanitizer_shared_runtime=OFF
 
 enable_snappy=ON
 enable_lz4=ON
@@ -56,6 +59,13 @@ Commands:
 Options:
   -t=<type>, -t <type>   Build type: Debug, Release, RelWithDebInfo, MinSizeRel.
   -a=<ON|OFF>            Enable or disable AddressSanitizer.
+  --sanitizer-checks=<list>
+                         Checks passed to -fsanitize= when ASan is enabled
+                         (default: address,undefined).
+  --sanitizer-shared-runtime=<ON|OFF>
+                         Link the sanitizer runtime dynamically into shared
+                         libraries; required when building the Python bindings,
+                         which preload libasan (default: OFF).
   -c=<ON|OFF>            Enable or disable code coverage.
   --build-shared=<ON|OFF>
                          Build libtsfile as a shared library (default: ON).
@@ -89,6 +99,8 @@ function print_config()
   echo "use_cpp11=$use_cpp11"
   echo "enable_cov=$enable_cov"
   echo "enable_asan=$enable_asan"
+  echo "sanitizer_checks=$sanitizer_checks"
+  echo "sanitizer_shared_runtime=$sanitizer_shared_runtime"
   echo "enable_antlr4=$enable_antlr4"
   echo "enable_snappy=$enable_snappy"
   echo "enable_lz4=$enable_lz4"
@@ -125,6 +137,10 @@ parse_options()
     -a)
       shift
       enable_asan=$(get_key_value "$1");;
+    --sanitizer-checks=*)
+      sanitizer_checks=$(get_key_value "$1");;
+    --sanitizer-shared-runtime=*)
+      sanitizer_shared_runtime=$(get_key_value "$1");;
     -c=*)
       enable_cov=$(get_key_value "$1");;
     -c)
@@ -223,6 +239,9 @@ cmake ../../                           \
   -DCMAKE_BUILD_TYPE=$build_type       \
   -DUSE_CPP11=$use_cpp11               \
   -DENABLE_COV=$enable_cov             \
+  -DENABLE_ASAN=$enable_asan           \
+  -DTSFILE_SANITIZER_CHECKS=$sanitizer_checks \
+  -DTSFILE_SANITIZER_SHARED_RUNTIME=$sanitizer_shared_runtime \
   -DDEBUG_SE=$debug_se                 \
   -DENABLE_ANTLR4=$enable_antlr4       \
   -DBUILD_TSFILE_ONLY=$build_tsfile_only \

cpp/pom.xml

@@ -34,6 +34,14 @@
         <coverage.enabled>OFF</coverage.enabled>
         <build.type>Release</build.type>
         <enable.asan>OFF</enable.asan>
+        <!--
+            Sanitizer details used when enable.asan is ON. The binding build
+            (-Dtsfile.sanitizer=address) activates the with-sanitizer profile,
+            which selects the address-only check set and a dynamically linked
+            runtime that Python can preload.
+        -->
+        <tsfile.sanitizer.checks>address,undefined</tsfile.sanitizer.checks>
+        <tsfile.sanitizer.shared.runtime>OFF</tsfile.sanitizer.shared.runtime>
         <build.test>ON</build.test>
         <tsfile.build.shared>ON</tsfile.build.shared>
         <tsfile.dependency.source>BUNDLED</tsfile.dependency.source>
@@ -110,6 +118,8 @@
                                 <option>-DCOV_ENABLED=${coverage.enabled}</option>
                                 <option>-DCMAKE_BUILD_TYPE=${build.type}</option>
                                 <option>-DENABLE_ASAN=${enable.asan}</option>
+                                <option>-DTSFILE_SANITIZER_CHECKS=${tsfile.sanitizer.checks}</option>
+                                <option>-DTSFILE_SANITIZER_SHARED_RUNTIME=${tsfile.sanitizer.shared.runtime}</option>
                                 <option>-DBUILD_TEST=${build.test}</option>
                                 <option>-DTSFILE_BUILD_SHARED=${tsfile.build.shared}</option>
                                 <option>-DTSFILE_DEPENDENCY_SOURCE=${tsfile.dependency.source}</option>
@@ -291,6 +301,25 @@
                 </plugins>
             </build>
         </profile>
+        <profile>
+            <id>with-sanitizer</id>
+            <!--
+                Activated by -Dtsfile.sanitizer=<checks> (for example
+                -Dtsfile.sanitizer=address). Selects ASan, keeps the sanitizer
+                runtime dynamic, and lets setup.py instrument every Python
+                extension with the same check set.
+            -->
+            <activation>
+                <property>
+                    <name>tsfile.sanitizer</name>
+                </property>
+            </activation>
+            <properties>
+                <enable.asan>ON</enable.asan>
+                <tsfile.sanitizer.checks>${tsfile.sanitizer}</tsfile.sanitizer.checks>
+                <tsfile.sanitizer.shared.runtime>ON</tsfile.sanitizer.shared.runtime>
+            </properties>
+        </profile>
         <profile>
             <id>with-code-coverage</id>
             <properties>

cpp/src/cwrapper/tsfile_cwrapper.cc

@@ -1173,6 +1173,14 @@ static void free_device_schema_array(DeviceSchema* schemas, size_t size) {
     free(schemas);
 }
 
+void tsfile_free_table_schema_array(TableSchema* schemas, uint32_t length) {
+    free_table_schema_array(schemas, length);
+}
+
+void tsfile_free_device_schema_array(DeviceSchema* schemas, uint32_t length) {
+    free_device_schema_array(schemas, length);
+}
+
 ERRNO tsfile_reader_get_table_schema_checked(TsFileReader reader,
                                              const char* table_name,
                                              TableSchema* out_schema) {
@@ -2170,6 +2178,8 @@ ERRNO tsfile_reader_get_tsfile_properties(TsFileReader reader,
         return common::E_INVALID_ARG;
     }
 
+    TsFileProperty* properties = nullptr;
+    uint32_t property_index = 0;
     try {
         auto* r = static_cast<storage::TsFileReader*>(reader);
         storage::TsFileProperties cpp_properties = r->get_tsfile_properties();
@@ -2183,14 +2193,13 @@ ERRNO tsfile_reader_get_tsfile_properties(TsFileReader reader,
             return common::E_OK;
         }
 
-        auto* properties = static_cast<TsFileProperty*>(
+        properties = static_cast<TsFileProperty*>(
             malloc(sizeof(TsFileProperty) * cpp_properties.size()));
         if (properties == nullptr) {
             return common::E_OOM;
         }
         memset(properties, 0, sizeof(TsFileProperty) * cpp_properties.size());
 
-        uint32_t property_index = 0;
         for (const auto& cpp_property : cpp_properties) {
             TsFileProperty& property = properties[property_index];
             if (cpp_property.first.size() >
@@ -2241,6 +2250,8 @@ ERRNO tsfile_reader_get_tsfile_properties(TsFileReader reader,
         *out_length = static_cast<uint32_t>(cpp_properties.size());
         return common::E_OK;
     } catch (const std::bad_alloc&) {
+        // get_tsfile_properties() can throw after the array was allocated.
+        tsfile_free_tsfile_properties(properties, property_index + 1);
         return common::E_OOM;
     }
 }
@@ -2296,6 +2307,8 @@ void free_table_schema(TableSchema schema) {
 }
 void free_column_schema(ColumnSchema schema) { free(schema.column_name); }
 
+void tsfile_free_buffer(void* buffer) { free(buffer); }
+
 void free_write_file(WriteFile* write_file) {
     auto f = static_cast<storage::WriteFile*>(*write_file);
     delete f;