Skip to content

fix(profiling): never enable fast copy memory if interpreter is embedded [backport 4.12] - #19927

Open
KowalskiThomas wants to merge 1 commit into
4.12from
kowalski/backport-19920-to-4.12
Open

fix(profiling): never enable fast copy memory if interpreter is embedded [backport 4.12]#19927
KowalskiThomas wants to merge 1 commit into
4.12from
kowalski/backport-19920-to-4.12

Conversation

@KowalskiThomas

Copy link
Copy Markdown
Collaborator

Backport of PR #19920 to branch 4.12.

Description

This PR changes the logic for the enablement of fast copy memory (signal handler-based) to never install signal handlers / try to use fast copy memory when the Python interpreter is running embedded in another application.

The reason we originally want to do that is we received crash reports originating from the Datadog Agent where running Python instrumentation in the Agent caused the Agent to crash. It makes sense generally speaking to be as quiet as possible, and as non-dangerous as possible, when running within another process. Hence this change.

Notes on conflict resolution

The 4.12 branch's ddtrace/internal/settings/profiling.py has diverged structurally from main (it lacks the native heap / exception-profiling availability checks and the max_frames/gc_enabled config options present on main). The equivalent change was applied by hand:

  • Added the _is_python_embedded() helper and its imports (os, sys).
  • Inserted the config.stack.fast_copy override in the same style as the other availability-override blocks in this file (after the stack_is_available check, before tag enrichment), since 4.12 has no later report_configuration(config) call to place it before.
  • vm.cc and the new test file/CMakeLists.txt entries were unchanged from main and applied as-is.
  • The Python unit tests and the release note were copied over unchanged.

Note: source PR #19920 was not yet merged at the time this backport was prepared (approved and mergeable); this backport is created from its current head commit.

@KowalskiThomas
KowalskiThomas requested review from a team as code owners August 28, 2026 14:50
@cit-pr-commenter-54b7da

Copy link
Copy Markdown

Codeowners resolved as

ddtrace/internal/datadog/profiling/stack/src/echion/vm.cc               @DataDog/profiling-python
ddtrace/internal/datadog/profiling/stack/test/CMakeLists.txt            @DataDog/profiling-python
ddtrace/internal/datadog/profiling/stack/test/test_embedded_fast_copy.cpp  @DataDog/profiling-python
ddtrace/internal/settings/profiling.py                                  @DataDog/profiling-python
ddtrace/internal/settings/profiling.pyi                                 @DataDog/profiling-python
releasenotes/notes/profiling-embedded-interpreter-fast-copy-memory-48b01a71fc45447f.yaml  @DataDog/apm-python
tests/profiling/test_profiling_config.py                                @DataDog/profiling-python

@cit-pr-commenter-54b7da

cit-pr-commenter-54b7da Bot commented Aug 28, 2026

Copy link
Copy Markdown

Circular import analysis

⚠️ Existing circular imports

There are 61 circular imports that already exist on the base branch and have not been changed by this PR.

Show existing cycles (showing 5 of 61 shortest)
ddtrace.internal.datastreams -> ddtrace.internal.datastreams.botocore -> ddtrace.internal.datastreams
ddtrace.internal.datastreams -> ddtrace.internal.datastreams.kafka -> ddtrace.internal.datastreams
ddtrace.internal.datastreams -> ddtrace.internal.datastreams.google_cloud_pubsub -> ddtrace.internal.datastreams
ddtrace.internal.datastreams -> ddtrace.internal.datastreams.aiokafka -> ddtrace.internal.datastreams
ddtrace.internal.datastreams -> ddtrace.internal.datastreams.kombu -> ddtrace.internal.datastreams

To see all cycles, download the cycles-base.json and cycles-pr.json artifacts from this CI job and run:

uv run --script scripts/import-analysis/cycles.py compare cycles-base.json cycles-pr.json

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f70c1b6626

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ddtrace/internal/datadog/profiling/stack/src/echion/vm.cc
Comment thread ddtrace/internal/settings/profiling.py
@datadog-prod-us1-4

datadog-prod-us1-4 Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Pipelines  Tests

⚠️ Warnings

Your PR has failed checks. Please review the issues below and take necessary action before merging.

🚦 13 Pipeline jobs failed

DataDog/apm-reliability/dd-trace-py | build linux serverless: [amd64, cp315-cp315, v113741491-d2b8243-musllinux_1_2_x86_64, 1] — 🔧 Needs a code fix, caused by this PR

View more details · View in GitLab

DataDog/apm-reliability/dd-trace-py | build linux serverless: [arm64, cp315-cp315, v113741589-d2b8243-musllinux_1_2_aarch64, 1] — 🔧 Needs a code fix, caused by this PR

View more details · View in GitLab

DataDog/apm-reliability/dd-trace-py | build linux: [amd64, cp315-cp315, v113741491-d2b8243-musllinux_1_2_x86_64] — 🔧 Needs a code fix, caused by this PR

View more details · View in GitLab

View all 13 failed jobs.

ℹ️ Info

No other issues found (see more)

🧪 All tests passed
❄️ No new flaky tests detected

Useful? React with 👍 / 👎

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 1b3741d | Docs | View more details | Give us feedback!

@KowalskiThomas
KowalskiThomas enabled auto-merge (squash) August 28, 2026 15:22
Fast memory copy (safe_memcpy) relies on installing SIGSEGV/SIGBUS signal
handlers to recover from faults. When Python is embedded in another
process (e.g. the Datadog Agent), those handlers can displace signal
handlers owned by the host process and cause crashes.

This change detects whether the interpreter is embedded (by checking
whether the process executable looks like a Python binary) and, if so,
skips installing the signal handlers and disables fast copy on both the
native (vm.cc) and Python config (profiling.py) sides.
@KowalskiThomas
KowalskiThomas force-pushed the kowalski/backport-19920-to-4.12 branch from f70c1b6 to 1b3741d Compare August 29, 2026 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants