-
Notifications
You must be signed in to change notification settings - Fork 541
fix(profiling): reject inconsistent Python 3.14+ asyncio task lists #19798
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
taegyunkim
wants to merge
9
commits into
main
Choose a base branch
from
taegyun/prof-echion-asyncio-task-traversal
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
369097c
test(profiling): reproduce asyncio task list migration
taegyunkim 7474aea
fix(profiling): reject inconsistent asyncio task lists
taegyunkim 6560b44
docs(profiling): add asyncio traversal release note
taegyunkim e7c8caf
docs(profiling): clarify asyncio traversal invariants
taegyunkim 4ed5281
Update ddtrace/internal/datadog/profiling/stack/src/echion/threads.cc
taegyunkim 50a43c3
Update ddtrace/internal/datadog/profiling/stack/src/echion/threads.cc
taegyunkim 1d27718
Merge branch 'main' into taegyun/prof-echion-asyncio-task-traversal
taegyunkim 0e3726c
style(profiling): fix Echion formatting
taegyunkim dc5090f
test(profiling): verify task traversal rollback
taegyunkim File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
144 changes: 144 additions & 0 deletions
144
ddtrace/internal/datadog/profiling/stack/test/test_task_traversal.cpp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,144 @@ | ||
| #include "echion/echion_sampler.h" | ||
| #include "echion/threads.h" | ||
|
|
||
| #include <gtest/gtest.h> | ||
|
|
||
| class ThreadInfoTaskTraversalTest : public ::testing::Test | ||
| { | ||
| protected: | ||
| #if PY_VERSION_HEX >= 0x030e0000 | ||
| // Keep the production traversal private while allowing deterministic linked-list topologies in this test. | ||
| static Result<void> traverse(ThreadInfo& thread, | ||
| EchionSampler& echion, | ||
| uintptr_t head, | ||
| std::vector<TaskInfo::Ptr>& tasks) | ||
| { | ||
| return thread.get_tasks_from_linked_list(echion, head, tasks); | ||
| } | ||
|
|
||
| static Result<std::vector<TaskInfo::Ptr>> get_all_tasks(ThreadInfo& thread, | ||
| EchionSampler& echion, | ||
| PyThreadState* tstate) | ||
| { | ||
| return thread.get_all_tasks(echion, tstate); | ||
| } | ||
| #endif | ||
| }; | ||
|
|
||
| #if PY_VERSION_HEX >= 0x030e0000 | ||
| TEST_F(ThreadInfoTaskTraversalTest, RejectsTaskMovedToAnotherList) | ||
| { | ||
| // A real asyncio.Task ensures TaskInfo::create follows the same coroutine and name-reading path as production. | ||
| Py_Initialize(); | ||
| PyObject* globals = PyDict_New(); | ||
| ASSERT_NE(globals, nullptr); | ||
| ASSERT_EQ(PyDict_SetItemString(globals, "__builtins__", PyEval_GetBuiltins()), 0); | ||
|
|
||
| PyObject* result = PyRun_String(R"( | ||
| import asyncio | ||
| loop = asyncio.new_event_loop() | ||
| async def wait_forever(): | ||
| await asyncio.Event().wait() | ||
| valid_task = loop.create_task(wait_forever()) | ||
| task = loop.create_task(wait_forever()) | ||
| )", | ||
| Py_file_input, | ||
| globals, | ||
| globals); | ||
| ASSERT_NE(result, nullptr); | ||
| Py_DECREF(result); | ||
|
|
||
| auto* loop = PyDict_GetItemString(globals, "loop"); | ||
| auto* valid_task = reinterpret_cast<TaskObj*>(PyDict_GetItemString(globals, "valid_task")); | ||
| auto* task = reinterpret_cast<TaskObj*>(PyDict_GetItemString(globals, "task")); | ||
| ASSERT_NE(loop, nullptr); | ||
| ASSERT_NE(valid_task, nullptr); | ||
| ASSERT_NE(task, nullptr); | ||
|
|
||
| EchionSampler echion; | ||
| #if defined PL_LINUX | ||
| ThreadInfo thread(1, 1, "test-thread", CLOCK_THREAD_CPUTIME_ID); | ||
| #elif defined PL_DARWIN | ||
| ThreadInfo thread(1, 1, "test-thread", mach_thread_self()); | ||
| #endif | ||
| thread.asyncio_loop = reinterpret_cast<uintptr_t>(loop); | ||
|
|
||
| // Seed the output to verify a failed source preserves tasks previously found by another source. | ||
| std::vector<TaskInfo::Ptr> tasks; | ||
| auto maybe_task = TaskInfo::create(echion, task); | ||
| ASSERT_TRUE(maybe_task); | ||
| tasks.push_back(std::move(*maybe_task)); | ||
| TaskInfo* sentinel = tasks.front().get(); | ||
|
|
||
| // Model Echion reading A and V from A <-> V <-> T before CPython moves T under head B. Reading T afterward | ||
| // produces this mixed-time view: | ||
| // | ||
| // copied nodes: A -> V -> T | ||
| // live task: B <-> T | ||
| // | ||
| // Traversal appends V before T.prev != V reveals the malformed edge and requires source-local rollback. | ||
| const llist_node original_valid_task_node = valid_task->task_node; | ||
| const llist_node original_task_node = task->task_node; | ||
| llist_node expected_head{}; | ||
| llist_node moved_head{}; | ||
| expected_head.next = &valid_task->task_node; | ||
| expected_head.prev = &task->task_node; | ||
| valid_task->task_node.prev = &expected_head; | ||
| valid_task->task_node.next = &task->task_node; | ||
| moved_head.next = moved_head.prev = &task->task_node; | ||
| task->task_node.next = task->task_node.prev = &moved_head; | ||
|
|
||
| result = nullptr; | ||
| auto traversal = traverse(thread, echion, reinterpret_cast<uintptr_t>(&expected_head), tasks); | ||
|
|
||
| // Reject the malformed source and roll back only the entries it appended. | ||
| EXPECT_FALSE(traversal); | ||
| EXPECT_EQ(tasks.size(), 1); | ||
|
taegyunkim marked this conversation as resolved.
|
||
| if (!tasks.empty()) { | ||
| EXPECT_EQ(tasks.front().get(), sentinel); | ||
| } | ||
|
|
||
| valid_task->task_node = original_valid_task_node; | ||
| task->task_node = original_task_node; | ||
| tasks.clear(); | ||
|
|
||
| // Expose the same Task through a valid thread list and the eager-task set. Cross-source discovery must still | ||
| // return one TaskInfo because downstream accounting and wall-time scaling operate on this result. | ||
| PyObject* eager_tasks = PySet_New(nullptr); | ||
| ASSERT_NE(eager_tasks, nullptr); | ||
| ASSERT_EQ(PySet_Add(eager_tasks, reinterpret_cast<PyObject*>(task)), 0); | ||
| echion.init_asyncio(nullptr, eager_tasks); | ||
|
|
||
| _PyThreadStateImpl remote_tstate{}; | ||
| remote_tstate.asyncio_tasks_head.next = remote_tstate.asyncio_tasks_head.prev = &task->task_node; | ||
| task->task_node.next = task->task_node.prev = &remote_tstate.asyncio_tasks_head; | ||
| thread.tstate_addr = reinterpret_cast<uintptr_t>(&remote_tstate); | ||
| PyThreadState local_tstate{}; | ||
|
|
||
| auto all_tasks = get_all_tasks(thread, echion, &local_tstate); | ||
|
|
||
| // Restore CPython's real links before cancellation or object destruction can inspect them. | ||
| task->task_node = original_task_node; | ||
| Py_DECREF(eager_tasks); | ||
| ASSERT_TRUE(all_tasks); | ||
| EXPECT_EQ(all_tasks->size(), 1); | ||
|
|
||
| // Process cancellation and close the loop so the real Task does not remain pending at process exit. | ||
| result = PyRun_String(R"( | ||
| for pending in (valid_task, task): | ||
| pending.cancel() | ||
| for pending in (valid_task, task): | ||
| try: | ||
| loop.run_until_complete(pending) | ||
| except asyncio.CancelledError: | ||
| pass | ||
| loop.close() | ||
| )", | ||
| Py_file_input, | ||
| globals, | ||
| globals); | ||
| EXPECT_NE(result, nullptr); | ||
| Py_XDECREF(result); | ||
| Py_DECREF(globals); | ||
| } | ||
| #endif | ||
4 changes: 4 additions & 0 deletions
4
releasenotes/notes/fix-profiler-asyncio-task-traversal-754a147329fc90cd.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| --- | ||
| fixes: | ||
| - | | ||
| profiling: Fixes an issue where asyncio tasks can be duplicated in profiles on Python 3.14, causing inflated task counts and wall time and increased profiler CPU usage. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit