Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
#pragma once

#include <algorithm>
#include <cstdint>
#include <optional>
#include <random>
#include <unordered_map>
#include <unordered_set>
#include <utility>
#include <vector>

#include <echion/cache.h>
#include <echion/frame.h>
Expand Down Expand Up @@ -65,6 +68,9 @@ class EchionSampler
// Caches
StringTable string_table_;
LRUCache<uintptr_t, Frame> frame_cache_;
#if PY_VERSION_HEX >= 0x030e0000
std::vector<std::pair<int64_t, uint64_t>> code_object_generations_;
#endif

// Stack renderer for outputting samples
Datadog::StackRenderer renderer_;
Expand Down Expand Up @@ -121,6 +127,46 @@ class EchionSampler
// Accessor for frame cache operations
LRUCache<uintptr_t, Frame>& frame_cache() { return frame_cache_; }

void invalidate_frame_identity_cache()
{
frame_cache_.clear();
asyncio_frame_cache_key_.reset();
uvloop_frame_cache_key_.reset();
}

#if PY_VERSION_HEX >= 0x030e0000
bool update_code_object_generations(const std::vector<InterpreterInfo>& interpreters, bool snapshot_complete)
{
if (!snapshot_complete || interpreters.empty()) {
invalidate_frame_identity_cache();
code_object_generations_.clear();
return false;
}

bool generations_changed = interpreters.size() != code_object_generations_.size();
if (!generations_changed) {
for (const auto& interpreter : interpreters) {
const std::pair<int64_t, uint64_t> generation{ interpreter.id, interpreter.code_object_generation };
if (!std::binary_search(code_object_generations_.begin(), code_object_generations_.end(), generation)) {
generations_changed = true;
break;
}
}
}

if (generations_changed) {
invalidate_frame_identity_cache();
code_object_generations_.clear();
code_object_generations_.reserve(interpreters.size());
for (const auto& interpreter : interpreters) {
code_object_generations_.emplace_back(interpreter.id, interpreter.code_object_generation);
}
std::sort(code_object_generations_.begin(), code_object_generations_.end());
}
return true;
}
#endif

void postfork_child()
{
// Re-init mutexes (placement new to avoid UB)
Expand All @@ -135,6 +181,9 @@ class EchionSampler
// because the Sampling Thread may have been modifying the cache when fork
// took its snapshot. Traversing a corrupted list to free nodes would crash.
frame_cache_.postfork_child();
#if PY_VERSION_HEX >= 0x030e0000
new (&code_object_generations_) std::vector<std::pair<int64_t, uint64_t>>();
#endif

// Also use placement new for all containers touched by the sampling thread.
// Using placement new means the existing containers are abandoned and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ class InterpreterInfo
int64_t id = 0;
void* tstate_head = NULL;
void* next = NULL;
#if PY_VERSION_HEX >= 0x030e0000
uint64_t code_object_generation = 0;
#endif
};

void
[[nodiscard]] bool
for_each_interp(_PyRuntimeState* runtime, const std::function<void(InterpreterInfo& interp)>& callback);
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
runtime.interpreters.head = reinterpret_cast<PyInterpreterState*>(p0);

size_t interp_count = 0;
for_each_interp(&runtime, [&interp_count](InterpreterInfo&) { interp_count++; });
(void)for_each_interp(&runtime, [&interp_count](InterpreterInfo&) { interp_count++; });

g_data = nullptr;
g_size = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "constants.hpp"

#include "echion/interp.h"
#include "echion/task_name.h"
#include "echion/timing.h"

Expand Down Expand Up @@ -79,6 +80,7 @@ class Sampler
microsecond_t max_sampling_period_us = g_max_sampling_period_us;
unsigned int max_threads_per_sample = g_default_max_threads_per_sample;
std::minstd_rand rng{ std::random_device{}() };
std::vector<InterpreterInfo> interpreter_candidates;
Comment thread
taegyunkim marked this conversation as resolved.
Comment thread
taegyunkim marked this conversation as resolved.
std::vector<PyThreadState> thread_candidates;
void adapt_sampling_interval();

Expand Down
18 changes: 14 additions & 4 deletions ddtrace/internal/datadog/profiling/stack/src/echion/interp.cc
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
#include <echion/interp.h>

void
bool
for_each_interp(_PyRuntimeState* runtime, const std::function<void(InterpreterInfo& interp)>& callback)
{
InterpreterInfo interpreter_info = { 0 };
bool snapshot_complete = true;

// Limit interpreter iteration to prevent infinite loops from cycles or corrupted memory.
// This limit is based on CPython's tachyon profiler (256) and should be more than
Expand All @@ -18,15 +18,22 @@ for_each_interp(_PyRuntimeState* runtime, const std::function<void(InterpreterIn

// Cycle detection: if we didn't advance from previous iteration, we're stuck
if (prev_interp_addr != nullptr && interp_addr == prev_interp_addr) {
break; // Cycle detected or failed to advance
return false; // Cycle detected or failed to advance
}
prev_interp_addr = interp_addr;

InterpreterInfo interpreter_info = { 0 };
#if PY_VERSION_HEX >= 0x030e0000
snapshot_complete &= !copy_type(interp_addr + offsetof(PyInterpreterState, _code_object_generation),
interpreter_info.code_object_generation);
#endif

// Always read next pointer first - we need it to advance
if (copy_type(interp_addr + offsetof(PyInterpreterState, next), interpreter_info.next))
break; // Can't read next, can't advance - stop iteration
return false; // Can't read next, can't advance - stop iteration

if (copy_type(interp_addr + offsetof(PyInterpreterState, id), interpreter_info.id)) {
snapshot_complete = false;
interp_addr = reinterpret_cast<char*>(interpreter_info.next);
continue;
}
Expand All @@ -37,6 +44,7 @@ for_each_interp(_PyRuntimeState* runtime, const std::function<void(InterpreterIn
if (copy_type(interp_addr + offsetof(PyInterpreterState, tstate_head), interpreter_info.tstate_head))
#endif
{
snapshot_complete = false;
interp_addr = reinterpret_cast<char*>(interpreter_info.next);
continue;
}
Expand All @@ -46,4 +54,6 @@ for_each_interp(_PyRuntimeState* runtime, const std::function<void(InterpreterIn
// Move to next interpreter
interp_addr = reinterpret_cast<char*>(interpreter_info.next);
}

return snapshot_complete && interp_addr == NULL;
}
26 changes: 22 additions & 4 deletions ddtrace/internal/datadog/profiling/stack/src/sampler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -258,26 +258,39 @@ Sampler::capture_samples(const microsecond_t wall_time_us)
{
auto* const runtime = &_PyRuntime;

interpreter_candidates.clear();
const bool interpreter_snapshot_complete =
for_each_interp(runtime, [&](InterpreterInfo& interp) { interpreter_candidates.push_back(interp); });
Comment thread
taegyunkim marked this conversation as resolved.
#if PY_VERSION_HEX >= 0x030e0000
// This lock-free snapshot can race with code destruction during the sampling cycle. In that case, the current
// cycle may use stale frame metadata; the next cycle observes the generation change and clears the cache.
if (!echion->update_code_object_generations(interpreter_candidates, interpreter_snapshot_complete)) {
return;
}
#else
(void)interpreter_snapshot_complete;
#endif

// When max_threads_per_sample is set, we collect all threads first, then apply
// reservoir sampling (Algorithm R) to select a uniform random subset, and only
// sample the selected threads. This caps the O(n_threads) stack-unwinding cost.
if (max_threads_per_sample == 0) {
for_each_interp(runtime, [&](InterpreterInfo& interp) -> void {
for (auto& interp : interpreter_candidates) {
for_each_thread(*echion, interp, [&](PyThreadState* tstate, ThreadInfo& thread) {
auto success = thread.sample(*echion, tstate, wall_time_us);
if (success) {
Sample::profile_borrow().stats().increment_sample_count();
}
});
});
}
} else {
thread_candidates.clear();

for_each_interp(runtime, [&](InterpreterInfo& interp) -> void {
for (auto& interp : interpreter_candidates) {
for_each_thread(*echion, interp, [&](PyThreadState* tstate, ThreadInfo& /*thread*/) {
thread_candidates.push_back(*tstate);
});
});
}

// Algorithm R: if we have more threads than the cap, select a uniform random subset.
// Selected threads are placed in [0, sample_count). Overflow threads remain in
Expand Down Expand Up @@ -593,6 +606,11 @@ Sampler::postfork_child()
new (&pause_mutex_) std::mutex();
new (&pause_cv_) std::condition_variable();

// The parent sampling thread may have been mutating these vectors when fork took its snapshot. Abandon their
// inherited storage instead of traversing potentially inconsistent state in clear() or push_back().
new (&interpreter_candidates) std::vector<InterpreterInfo>();
new (&thread_candidates) std::vector<PyThreadState>();

// Clear stale echion state (mutexes, maps) from parent process
if (echion) {
echion->postfork_child();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,19 @@

#include <memory>

#if PY_VERSION_HEX >= 0x030e0000
namespace {
InterpreterInfo
interpreter(int64_t id, uint64_t generation)
{
InterpreterInfo info;
info.id = id;
info.code_object_generation = generation;
return info;
}
} // namespace
#endif

#if defined PL_LINUX
TEST(ThreadInfoCreate, IgnoresNonPthreadPythonThreadId)
{
Expand Down Expand Up @@ -38,6 +51,35 @@ TEST(SamplingCycleState, UnwindReplacesTaskAndGreenletStacksFromPriorCycle)
EXPECT_TRUE(thread.current_greenlets.empty());
}

#if PY_VERSION_HEX >= 0x030e0000
TEST(SamplingCycleState, CodeObjectGenerationInvalidatesFrameIdentityCache)
{
EchionSampler echion(2);
constexpr Frame::Key key = 42;

ASSERT_TRUE(echion.update_code_object_generations({ interpreter(1, 1), interpreter(2, 1) }, true));
echion.frame_cache().store(key, std::make_unique<Frame>(10));
echion.asyncio_frame_cache_key() = key;
echion.uvloop_frame_cache_key() = key;

EXPECT_TRUE(echion.update_code_object_generations({ interpreter(2, 1), interpreter(1, 1) }, true));
EXPECT_TRUE(echion.frame_cache().lookup(key));

EXPECT_TRUE(echion.update_code_object_generations({ interpreter(1, 1), interpreter(2, 2) }, true));
EXPECT_FALSE(echion.frame_cache().lookup(key));
EXPECT_FALSE(echion.asyncio_frame_cache_key());
EXPECT_FALSE(echion.uvloop_frame_cache_key());

echion.frame_cache().store(key, std::make_unique<Frame>(10));
EXPECT_TRUE(echion.update_code_object_generations({ interpreter(1, 1), interpreter(3, 2) }, true));
EXPECT_FALSE(echion.frame_cache().lookup(key));

echion.frame_cache().store(key, std::make_unique<Frame>(10));
EXPECT_FALSE(echion.update_code_object_generations({ interpreter(1, 1), interpreter(3, 2) }, false));
EXPECT_FALSE(echion.frame_cache().lookup(key));
}
#endif

TEST(SamplingCycleState, GreenletSwitchPreservesLinkedParentFrame)
{
constexpr GreenletInfo::ID child_id = 101;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
fixes:
- |
profiling: On Python 3.14, prevents stack samples from being attributed to stale Python frames after code objects are replaced.
67 changes: 67 additions & 0 deletions tests/profiling/collector/test_stack.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,73 @@ def foo() -> None:
pprof_utils.assert_profile_has_sample(profile, samples=samples, expected_sample=expected_sample)


@pytest.mark.skipif(sys.version_info < (3, 14), reason="requires CPython's code object generation")
@pytest.mark.subprocess()
def test_code_object_address_reuse_does_not_return_stale_frame() -> None:
import gc
import os
from pathlib import Path
import tempfile
import time
from types import FunctionType
import weakref

from ddtrace.internal.datadog.profiling import ddup
from ddtrace.profiling.collector import stack
from tests.profiling.collector import pprof_utils

test_name = "test_code_object_address_reuse_does_not_return_stale_frame"
tmp_path = Path(tempfile.mkdtemp(prefix=test_name))
pprof_prefix = str(tmp_path / test_name)
output_filename = pprof_prefix + "." + str(os.getpid())
code_filename = "echion-code-reuse.py"

assert ddup.is_available
ddup.config(env="test", service=test_name, version="my_version", output_filename=pprof_prefix)
ddup.start()
ddup.upload()

namespace = {"time": time}
source = "def template(deadline):\n while time.monotonic() < deadline:\n pass\n"
exec(compile(source, code_filename, "exec"), namespace)
template_code = namespace["template"].__code__

def make_function(name: str):
code = template_code.replace(co_name=name, co_qualname=name)
return FunctionType(code, {"time": time})

old_name = "old_dynamic_function"
old_function = make_function(old_name)

with stack.StackCollector():
old_function(time.monotonic() + 0.3)
ddup.upload()

old_address = id(old_function.__code__)
old_code = weakref.ref(old_function.__code__)
del old_function
gc.collect()
assert old_code() is None

replacement_name = "new_dynamic_function"
replacement_function = make_function(replacement_name)
assert id(replacement_function.__code__) == old_address
replacement_function(time.monotonic() + 0.3)

ddup.upload()

profile = pprof_utils.parse_newest_profile(output_filename)
samples = pprof_utils.get_samples_with_value_type(profile, "wall-time")
sampled_names = {
location.function_name
for sample in samples
for location in (pprof_utils.get_location_from_id(profile, location_id) for location_id in sample.location_id)
if location.filename == code_filename
}
assert replacement_name in sampled_names
assert old_name not in sampled_names


def test_push_span(tmp_path: Path, tracer: Tracer) -> None:
test_name = "test_push_span"
pprof_prefix = str(tmp_path / test_name)
Expand Down
Loading