test(profiling): regression test for #16661

.gitlab/templates/build-base-venvs.yml

@@ -11,6 +11,7 @@ build_base_venvs:
     DD_PROFILING_NATIVE_TESTS: '1'
     DD_USE_SCCACHE: '1'
     DD_FAST_BUILD: '1'
+    DD_PROFILING_MEMALLOC_ASSERT_ON_REENTRY: '1'
   rules:
     - if: '$CI_COMMIT_REF_NAME == "main"'
       variables:

ddtrace/internal/settings/profiling.py

@@ -247,7 +247,9 @@ def __init__(self, *args: t.Any, **kwargs: t.Any) -> None:
         "enable_asserts",
         default=False,
         help_type="Boolean",
-        help="Whether to enable debug assertions in the profiler code",
+        help="Whether to enable debug assertions in the profiler code. "
+        "When set at build time, also enables compile-time asserts in native extensions "
+        "(e.g. abort on reentrant allocator hook calls in _memalloc).",
     )
 
     sample_pool_capacity = DDConfig.v(

ddtrace/profiling/collector/CMakeLists.txt

@@ -140,6 +140,13 @@ if(DEFINED NATIVE_EXTENSION_LOCATION)
     endif()
 endif()
 
+# Enable allocator-hook reentry assertions in test builds. setup.py turns this on when
+# DD_PROFILING_MEMALLOC_ASSERT_ON_REENTRY is set in the build environment.
+if(MEMALLOC_ASSERT_ON_REENTRY)
+    message(STATUS "MEMALLOC_ASSERT_ON_REENTRY enabled: will abort on reentrant allocator hook calls")
+    target_compile_definitions(${FULL_EXTENSION_NAME} PRIVATE MEMALLOC_ASSERT_ON_REENTRY)
+endif()
+
 # Add NDEBUG flag for release builds
 if(CMAKE_BUILD_TYPE STREQUAL "Release"
    OR CMAKE_BUILD_TYPE STREQUAL "RelWithDebInfo"

ddtrace/profiling/collector/_memalloc.cpp

@@ -42,8 +42,17 @@ memalloc_free(void* ctx, void* ptr)
     if (ptr == NULL)
         return;
 
-    memalloc_heap_untrack_no_cpython(ptr);
+#ifdef MEMALLOC_ASSERT_ON_REENTRY
+    /* Abort in test builds if we're re-entering from the malloc hook.
+     * In production we can't abort or skip untrack (skipping would leak
+     * heap tracker entries), so we just let it proceed — direct struct
+     * access frame walking avoids calling CPython APIs that could free. */
+    if (_MEMALLOC_ON_THREAD) {
+        _memalloc_abort_reentry("free");
+    }
+#endif
 
+    memalloc_heap_untrack_no_cpython(ptr);
     alloc->free(alloc->ctx, ptr);
 }
 

ddtrace/profiling/collector/_memalloc_heap.cpp

@@ -360,7 +360,8 @@ memalloc_heap_track_invokes_cpython(uint16_t max_nframe, void* ptr, size_t size,
         return;
     }
 
-    /* Avoid loops */
+    /* Skip tracking if we're already inside the malloc hook on this thread.
+     * Reentrant tracking would corrupt the heap tracker's data structures. */
     memalloc_reentrant_guard_t guard;
     if (!guard) {
         return;

ddtrace/profiling/collector/_memalloc_reentrant.h

@@ -1,5 +1,7 @@
 #pragma once
 
+#include <stdbool.h>
+
 // Thread-local storage macro for Unix (GCC/Clang)
 // NB - we explicitly specify global-dynamic on Unix because the others are problematic.
 // See e.g. https://fuchsia.dev/fuchsia-src/development/kernel/threads/tls for
@@ -10,13 +12,34 @@
 // sees we're building a shared library. But we've been bit by issues related
 // to this before, and it doesn't hurt to explicitly declare the model here.
 #define MEMALLOC_TLS __attribute__((tls_model("global-dynamic"))) __thread
+
+/* True while the malloc allocator hook is running on this thread.
+ * Used to prevent reentrant heap tracking (which would corrupt the heap
+ * tracker's data structures) and to detect reentrant calls in assert builds. */
 extern MEMALLOC_TLS bool _MEMALLOC_ON_THREAD;
 
-/* RAII guard for reentrancy protection. Automatically acquires the guard in the
- * constructor and releases it in the destructor.
+#ifdef MEMALLOC_ASSERT_ON_REENTRY
+#include <stdio.h>
+#include <stdlib.h>
+
+static inline void
+_memalloc_abort_reentry(const char* inner_op)
+{
+    fprintf(stderr, "[memalloc] FATAL: reentrant allocator hook detected: malloc -> %s\n", inner_op);
+    abort();
+}
+#endif /* MEMALLOC_ASSERT_ON_REENTRY */
+
+/* RAII guard for reentrancy protection. Sets _MEMALLOC_ON_THREAD in the
+ * constructor and clears it in the destructor.
+ *
+ * If _MEMALLOC_ON_THREAD is already set (reentrant call), the guard does
+ * not acquire:
+ *  - In assert builds, it aborts immediately for early detection.
+ *  - In production builds, callers check acquired() / operator bool() to
+ *    skip heap tracking and avoid data structure corruption.
  *
- * Ordinarily, a process-wide semaphore would require a CAS, but since this is
- * thread-local we can just set it.  */
+ * Since this is thread-local, no CAS or atomic is needed.  */
 class memalloc_reentrant_guard_t
 {
   public:
@@ -26,6 +49,10 @@ class memalloc_reentrant_guard_t
         if (!_MEMALLOC_ON_THREAD) {
             _MEMALLOC_ON_THREAD = true;
             acquired_ = true;
+        } else {
+#ifdef MEMALLOC_ASSERT_ON_REENTRY
+            _memalloc_abort_reentry("malloc");
+#endif
         }
     }
 

docker-compose.yml

@@ -201,6 +201,7 @@ services:
           CYTHON_CACHE_DIR: "/home/bits/.cache/cython"
           DD_USE_SCCACHE: "1"
           SCCACHE_DIR: "/home/bits/.cache/sccache"
+          DD_PROFILING_MEMALLOC_ASSERT_ON_REENTRY: "1"
         network_mode: host
         userns_mode: host
         working_dir: /home/bits/project/

docs/spelling_wordlist.txt

@@ -194,6 +194,7 @@ loopback
 macOS
 mako
 mariadb
+memalloc
 memcached
 metadata
 microservices
@@ -266,6 +267,7 @@ pytest-bdd
 PyTorch
 quickstart
 ratelimit
+reentrant
 redis
 rediscluster
 renderer

riotfile.py

@@ -3384,6 +3384,7 @@ def select_pys(min_version: str = MIN_PYTHON_VERSION, max_version: str = MAX_PYT
             command="python -m tests.profiling.run pytest -v --no-cov --capture=no --benchmark-disable --ignore='tests/profiling/collector/test_memalloc.py' --ignore='tests/profiling/test_memalloc_fork.py' {cmdargs} tests/profiling",  # noqa: E501
             env={
                 "DD_PROFILING_ENABLE_ASSERTS": "1",
+                "DD_PROFILING_MEMALLOC_ASSERT_ON_REENTRY": "1",
                 "CPUCOUNT": "12",
                 "PYTHONWARNINGS": "ignore::UserWarning:gevent.events",
             },
@@ -3596,6 +3597,9 @@ def select_pys(min_version: str = MIN_PYTHON_VERSION, max_version: str = MAX_PYT
                     name="profile-memalloc",
                     command="python -m tests.profiling.run pytest -v --no-cov --capture=no --benchmark-disable {cmdargs} tests/profiling/collector/test_memalloc.py tests/profiling/test_memalloc_fork.py",  # noqa: E501
                     pys=select_pys(),
+                    env={
+                        "DD_PROFILING_MEMALLOC_ASSERT_ON_REENTRY": "1",
+                    },
                     pkgs={
                         "protobuf": latest,
                     },

setup.py

@@ -1190,10 +1190,14 @@ def get_exts_for(name):
     if CURRENT_OS in ("Linux", "Darwin") and is_64_bit_python():
         # Memory profiler now uses CMake to support Abseil dependency
         MEMALLOC_DIR = HERE / "ddtrace" / "profiling" / "collector"
+        memalloc_cmake_args = []
+        if os.environ.get("DD_PROFILING_MEMALLOC_ASSERT_ON_REENTRY", "0") not in ("0", ""):
+            memalloc_cmake_args.append("-DMEMALLOC_ASSERT_ON_REENTRY=ON")
         ext_modules.append(
             CMakeExtension(
                 "ddtrace.profiling.collector._memalloc",
                 source_dir=MEMALLOC_DIR,
+                cmake_args=memalloc_cmake_args,
                 optional=False,
             )
         )