Skip to content

Commit 1040790

Browse files
committed
fix(profiling): invalidate stale Echion frames on Python 3.14
1 parent 052e524 commit 1040790

9 files changed

Lines changed: 202 additions & 10 deletions

File tree

ddtrace/internal/datadog/profiling/stack/echion/echion/echion_sampler.h

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
#pragma once
22

3+
#include <algorithm>
34
#include <cstdint>
45
#include <optional>
56
#include <unordered_map>
67
#include <unordered_set>
8+
#include <utility>
9+
#include <vector>
710

811
#include <echion/cache.h>
912
#include <echion/frame.h>
@@ -56,6 +59,9 @@ class EchionSampler
5659
// Caches
5760
StringTable string_table_;
5861
LRUCache<uintptr_t, Frame> frame_cache_;
62+
#if PY_VERSION_HEX >= 0x030e0000
63+
std::vector<std::pair<int64_t, uint64_t>> code_object_generations_;
64+
#endif
5965

6066
// Stack renderer for outputting samples
6167
Datadog::StackRenderer renderer_;
@@ -107,6 +113,46 @@ class EchionSampler
107113
// Accessor for frame cache operations
108114
LRUCache<uintptr_t, Frame>& frame_cache() { return frame_cache_; }
109115

116+
void invalidate_frame_identity_cache()
117+
{
118+
frame_cache_.clear();
119+
asyncio_frame_cache_key_.reset();
120+
uvloop_frame_cache_key_.reset();
121+
}
122+
123+
#if PY_VERSION_HEX >= 0x030e0000
124+
bool update_code_object_generations(const std::vector<InterpreterInfo>& interpreters, bool snapshot_complete)
125+
{
126+
if (!snapshot_complete || interpreters.empty()) {
127+
invalidate_frame_identity_cache();
128+
code_object_generations_.clear();
129+
return false;
130+
}
131+
132+
bool generations_changed = interpreters.size() != code_object_generations_.size();
133+
if (!generations_changed) {
134+
for (const auto& interpreter : interpreters) {
135+
const std::pair<int64_t, uint64_t> generation{ interpreter.id, interpreter.code_object_generation };
136+
if (!std::binary_search(code_object_generations_.begin(), code_object_generations_.end(), generation)) {
137+
generations_changed = true;
138+
break;
139+
}
140+
}
141+
}
142+
143+
if (generations_changed) {
144+
invalidate_frame_identity_cache();
145+
code_object_generations_.clear();
146+
code_object_generations_.reserve(interpreters.size());
147+
for (const auto& interpreter : interpreters) {
148+
code_object_generations_.emplace_back(interpreter.id, interpreter.code_object_generation);
149+
}
150+
std::sort(code_object_generations_.begin(), code_object_generations_.end());
151+
}
152+
return true;
153+
}
154+
#endif
155+
110156
void postfork_child()
111157
{
112158
// Re-init mutexes (placement new to avoid UB)
@@ -121,6 +167,9 @@ class EchionSampler
121167
// because the Sampling Thread may have been modifying the cache when fork
122168
// took its snapshot. Traversing a corrupted list to free nodes would crash.
123169
frame_cache_.postfork_child();
170+
#if PY_VERSION_HEX >= 0x030e0000
171+
new (&code_object_generations_) std::vector<std::pair<int64_t, uint64_t>>();
172+
#endif
124173

125174
// Also use placement new for all containers touched by the sampling thread.
126175
// Using placement new means the existing containers are abandoned and

ddtrace/internal/datadog/profiling/stack/echion/echion/interp.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,10 @@ class InterpreterInfo
2727
int64_t id = 0;
2828
void* tstate_head = NULL;
2929
void* next = NULL;
30+
#if PY_VERSION_HEX >= 0x030e0000
31+
uint64_t code_object_generation = 0;
32+
#endif
3033
};
3134

32-
void
35+
[[nodiscard]] bool
3336
for_each_interp(_PyRuntimeState* runtime, const std::function<void(InterpreterInfo& interp)>& callback);

ddtrace/internal/datadog/profiling/stack/fuzz/fuzz_echion_interp.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
2828
runtime.interpreters.head = reinterpret_cast<PyInterpreterState*>(p0);
2929

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

3333
g_data = nullptr;
3434
g_size = 0;

ddtrace/internal/datadog/profiling/stack/include/sampler.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
#include "constants.hpp"
1212

13+
#include "echion/interp.h"
1314
#include "echion/task_name.h"
1415
#include "echion/timing.h"
1516

@@ -79,6 +80,7 @@ class Sampler
7980
microsecond_t max_sampling_period_us = g_max_sampling_period_us;
8081
unsigned int max_threads_per_sample = g_default_max_threads_per_sample;
8182
std::minstd_rand rng{ std::random_device{}() };
83+
std::vector<InterpreterInfo> interpreter_candidates;
8284
std::vector<PyThreadState> thread_candidates;
8385
void adapt_sampling_interval();
8486

ddtrace/internal/datadog/profiling/stack/src/echion/interp.cc

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
#include <echion/interp.h>
22

3-
void
3+
bool
44
for_each_interp(_PyRuntimeState* runtime, const std::function<void(InterpreterInfo& interp)>& callback)
55
{
6-
InterpreterInfo interpreter_info = { 0 };
6+
bool snapshot_complete = true;
77

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

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

25+
InterpreterInfo interpreter_info = { 0 };
26+
#if PY_VERSION_HEX >= 0x030e0000
27+
snapshot_complete &= !copy_type(interp_addr + offsetof(PyInterpreterState, _code_object_generation),
28+
interpreter_info.code_object_generation);
29+
#endif
30+
2531
// Always read next pointer first - we need it to advance
2632
if (copy_type(interp_addr + offsetof(PyInterpreterState, next), interpreter_info.next))
27-
break; // Can't read next, can't advance - stop iteration
33+
return false; // Can't read next, can't advance - stop iteration
2834

2935
if (copy_type(interp_addr + offsetof(PyInterpreterState, id), interpreter_info.id)) {
36+
snapshot_complete = false;
3037
interp_addr = reinterpret_cast<char*>(interpreter_info.next);
3138
continue;
3239
}
@@ -37,6 +44,7 @@ for_each_interp(_PyRuntimeState* runtime, const std::function<void(InterpreterIn
3744
if (copy_type(interp_addr + offsetof(PyInterpreterState, tstate_head), interpreter_info.tstate_head))
3845
#endif
3946
{
47+
snapshot_complete = false;
4048
interp_addr = reinterpret_cast<char*>(interpreter_info.next);
4149
continue;
4250
}
@@ -46,4 +54,6 @@ for_each_interp(_PyRuntimeState* runtime, const std::function<void(InterpreterIn
4654
// Move to next interpreter
4755
interp_addr = reinterpret_cast<char*>(interpreter_info.next);
4856
}
57+
58+
return snapshot_complete && interp_addr == NULL;
4959
}

ddtrace/internal/datadog/profiling/stack/src/sampler.cpp

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -258,26 +258,37 @@ Sampler::capture_samples(const microsecond_t wall_time_us)
258258
{
259259
auto* const runtime = &_PyRuntime;
260260

261+
interpreter_candidates.clear();
262+
const bool interpreter_snapshot_complete =
263+
for_each_interp(runtime, [&](InterpreterInfo& interp) { interpreter_candidates.push_back(interp); });
264+
#if PY_VERSION_HEX >= 0x030e0000
265+
if (!echion->update_code_object_generations(interpreter_candidates, interpreter_snapshot_complete)) {
266+
return;
267+
}
268+
#else
269+
(void)interpreter_snapshot_complete;
270+
#endif
271+
261272
// When max_threads_per_sample is set, we collect all threads first, then apply
262273
// reservoir sampling (Algorithm R) to select a uniform random subset, and only
263274
// sample the selected threads. This caps the O(n_threads) stack-unwinding cost.
264275
if (max_threads_per_sample == 0) {
265-
for_each_interp(runtime, [&](InterpreterInfo& interp) -> void {
276+
for (auto& interp : interpreter_candidates) {
266277
for_each_thread(*echion, interp, [&](PyThreadState* tstate, ThreadInfo& thread) {
267278
auto success = thread.sample(*echion, tstate, wall_time_us);
268279
if (success) {
269280
Sample::profile_borrow().stats().increment_sample_count();
270281
}
271282
});
272-
});
283+
}
273284
} else {
274285
thread_candidates.clear();
275286

276-
for_each_interp(runtime, [&](InterpreterInfo& interp) -> void {
287+
for (auto& interp : interpreter_candidates) {
277288
for_each_thread(*echion, interp, [&](PyThreadState* tstate, ThreadInfo& /*thread*/) {
278289
thread_candidates.push_back(*tstate);
279290
});
280-
});
291+
}
281292

282293
// Algorithm R: if we have more threads than the cap, select a uniform random subset.
283294
// Selected threads are placed in [0, sample_count). Overflow threads remain in

ddtrace/internal/datadog/profiling/stack/test/test_sampling_cycle_state.cpp

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,19 @@
55

66
#include <memory>
77

8+
#if PY_VERSION_HEX >= 0x030e0000
9+
namespace {
10+
InterpreterInfo
11+
interpreter(int64_t id, uint64_t generation)
12+
{
13+
InterpreterInfo info;
14+
info.id = id;
15+
info.code_object_generation = generation;
16+
return info;
17+
}
18+
} // namespace
19+
#endif
20+
821
TEST(SamplingCycleState, UnwindReplacesTaskAndGreenletStacksFromPriorCycle)
922
{
1023
EchionSampler echion;
@@ -23,3 +36,32 @@ TEST(SamplingCycleState, UnwindReplacesTaskAndGreenletStacksFromPriorCycle)
2336
EXPECT_TRUE(thread.current_tasks.empty());
2437
EXPECT_TRUE(thread.current_greenlets.empty());
2538
}
39+
40+
#if PY_VERSION_HEX >= 0x030e0000
41+
TEST(SamplingCycleState, CodeObjectGenerationInvalidatesFrameIdentityCache)
42+
{
43+
EchionSampler echion(2);
44+
constexpr Frame::Key key = 42;
45+
46+
ASSERT_TRUE(echion.update_code_object_generations({ interpreter(1, 1), interpreter(2, 1) }, true));
47+
echion.frame_cache().store(key, std::make_unique<Frame>(10));
48+
echion.asyncio_frame_cache_key() = key;
49+
echion.uvloop_frame_cache_key() = key;
50+
51+
EXPECT_TRUE(echion.update_code_object_generations({ interpreter(2, 1), interpreter(1, 1) }, true));
52+
EXPECT_TRUE(echion.frame_cache().lookup(key));
53+
54+
EXPECT_TRUE(echion.update_code_object_generations({ interpreter(1, 1), interpreter(2, 2) }, true));
55+
EXPECT_FALSE(echion.frame_cache().lookup(key));
56+
EXPECT_FALSE(echion.asyncio_frame_cache_key());
57+
EXPECT_FALSE(echion.uvloop_frame_cache_key());
58+
59+
echion.frame_cache().store(key, std::make_unique<Frame>(10));
60+
EXPECT_TRUE(echion.update_code_object_generations({ interpreter(1, 1), interpreter(3, 2) }, true));
61+
EXPECT_FALSE(echion.frame_cache().lookup(key));
62+
63+
echion.frame_cache().store(key, std::make_unique<Frame>(10));
64+
EXPECT_FALSE(echion.update_code_object_generations({ interpreter(1, 1), interpreter(3, 2) }, false));
65+
EXPECT_FALSE(echion.frame_cache().lookup(key));
66+
}
67+
#endif
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
profiling: On Python 3.14, prevents stack samples from being attributed to stale Python frames after code objects are replaced.

tests/profiling/collector/test_stack.py

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,77 @@ def foo() -> None:
150150
pprof_utils.assert_profile_has_sample(profile, samples=samples, expected_sample=expected_sample)
151151

152152

153+
@pytest.mark.skipif(sys.version_info < (3, 14), reason="requires CPython's code object generation")
154+
@pytest.mark.subprocess()
155+
def test_code_object_address_reuse_does_not_return_stale_frame() -> None:
156+
import gc
157+
import os
158+
from pathlib import Path
159+
import tempfile
160+
import time
161+
from types import FunctionType
162+
import weakref
163+
164+
import _interpreters
165+
166+
from ddtrace.internal.datadog.profiling import ddup
167+
from ddtrace.profiling.collector import stack
168+
from tests.profiling.collector import pprof_utils
169+
170+
interpreter = _interpreters.create("legacy")
171+
test_name = "test_code_object_address_reuse_does_not_return_stale_frame"
172+
tmp_path = Path(tempfile.mkdtemp(prefix=test_name))
173+
pprof_prefix = str(tmp_path / test_name)
174+
output_filename = pprof_prefix + "." + str(os.getpid())
175+
code_filename = "echion-code-reuse.py"
176+
177+
assert ddup.is_available
178+
ddup.config(env="test", service=test_name, version="my_version", output_filename=pprof_prefix)
179+
ddup.start()
180+
ddup.upload()
181+
182+
namespace = {"time": time}
183+
source = "def template(deadline):\n while time.monotonic() < deadline:\n pass\n"
184+
exec(compile(source, code_filename, "exec"), namespace)
185+
template_code = namespace["template"].__code__
186+
187+
def make_function(name: str):
188+
code = template_code.replace(co_name=name, co_qualname=name)
189+
return FunctionType(code, {"time": time})
190+
191+
old_name = "old_dynamic_function"
192+
old_function = make_function(old_name)
193+
194+
with stack.StackCollector():
195+
old_function(time.monotonic() + 0.3)
196+
ddup.upload()
197+
198+
old_address = id(old_function.__code__)
199+
old_code = weakref.ref(old_function.__code__)
200+
del old_function
201+
gc.collect()
202+
assert old_code() is None
203+
204+
replacement_name = "new_dynamic_function"
205+
replacement_function = make_function(replacement_name)
206+
assert id(replacement_function.__code__) == old_address
207+
replacement_function(time.monotonic() + 0.3)
208+
209+
ddup.upload()
210+
211+
profile = pprof_utils.parse_newest_profile(output_filename)
212+
samples = pprof_utils.get_samples_with_value_type(profile, "wall-time")
213+
sampled_names = {
214+
location.function_name
215+
for sample in samples
216+
for location in (pprof_utils.get_location_from_id(profile, location_id) for location_id in sample.location_id)
217+
if location.filename == code_filename
218+
}
219+
assert replacement_name in sampled_names
220+
assert old_name not in sampled_names
221+
_interpreters.destroy(interpreter)
222+
223+
153224
def test_push_span(tmp_path: Path, tracer: Tracer) -> None:
154225
test_name = "test_push_span"
155226
pprof_prefix = str(tmp_path / test_name)

0 commit comments

Comments
 (0)