fix(internal): fix crash at finalisation by KowalskiThomas · Pull Request #20081 · DataDog/dd-trace-py

KowalskiThomas · 2026-09-04T15:27:32Z

Description

This fixes a crash that can happen at interpreter finalisation before Python 3.14. Python 3.14 fixed this in python/cpython#87135 but previous versions are still sensitive to the issue. We fix it in the Python tracer by replicating python/cpython#105805, where threads trying to acquire the GIL at finalisation time are hanged rather than properly exited.

This happens because the exporter runs on a thread that is not joined at shutdown. (The PR fixes the bug for all native threads, not only the exporter.)

Error UnixSignal: Process terminated with SI_TKILL (SIGABRT)
#0   0x0000f4f997842038 __pthread_kill_implementation (nptl/nptl/pthread_kill.c:44)
#1   0x0000f4f9977fa83c __GI_raise (sysdeps/posix/raise.c:27)
#2   0x0000f4f9977e7134 __GI_abort (stdlib/stdlib/abort.c:81)
#3   0x0000f4f99784909c  
#4   0x0000f4f997841344 __pthread_exit (nptl/nptl/pthread_exit.c:30)
#5   0x0000ba94a70708a4 _PyThread_at_fork_reinit (/usr/src/python/Python/thread_pthread.h:727)
#6   0x0000ba94a71036b0 take_gil (/usr/src/python/Python/ceval_gil.c:452)
#7   0x0000ba94a7103adc PyEval_RestoreThread (/usr/src/python/Python/ceval_gil.c:710)
#8   0x0000f4f99660b768 _native::data_pipeline::TraceExporterPy::__pymethod_send__ 
#9   0x0000f4f9965d2a74 pyo3::impl_::trampoline::fastcall_cfunction_with_keywords::inner 
#10  0x0000f4f9965d2310 pyo3::impl_::trampoline::fastcall_cfunction_with_keywords 
#11  0x0000ba94a70ed1d8 _PyEval_EvalFrameDefault (/usr/src/python/Python/bytecodes.c:3123)
#12  0x0000000000000000 _PyObject_VectorcallTstate (/usr/src/python/Include/internal/pycore_call.h:93)
#13  0x0000ba94a70bc9cc _PyObject_VectorcallTstate (/usr/src/python/Include/internal/pycore_call.h:93)
#14  0x0000ba94a70bc9cc method_vectorcall (/usr/src/python/Objects/classobject.c:69)
#15  0x0000ba94a70bae50 _PyObject_VectorcallTstate (/usr/src/python/Include/internal/pycore_call.h:93)
#16  0x0000f4f996ef5184 std::thread::_State_impl<std::thread::_Invoker<std::tuple<_PeriodicThread_do_start(periodic_thread*, bool)::{lambda()#1}> > >::_M_run() 
#17  0x0000f4f996ef5e9c execute_native_thread_routine 
#18  0x0000f4f9978403c8 start_thread (nptl/nptl/pthread_create.c:442)

cit-pr-commenter-54b7da · 2026-09-04T15:28:27Z

Codeowners resolved as

Resolved from the full PR diff against main using the target branch CODEOWNERS file.
CODEOWNERS team requests not listed below are not required by the current file set.

releasenotes/notes/fix-exporter-gil-finalize-abort-fbb593d14b8c1493.yaml  @DataDog/apm-python
src/native/data_pipeline/mod.rs                                         @DataDog/apm-core-python
src/native/debugger.rs                                                  @DataDog/apm-core-python
src/native/gil.rs                                                       @DataDog/apm-core-python
src/native/http_client/client.rs                                        @DataDog/apm-core-python
src/native/lib.rs                                                       @DataDog/apm-core-python
src/native/remote_config.rs                                             @DataDog/apm-core-python
src/native/telemetry.rs                                                 @DataDog/apm-core-python
src/native/tracer_flare.rs                                              @DataDog/apm-core-python

cit-pr-commenter-54b7da · 2026-09-04T15:28:37Z

Circular import analysis

⚠️ Existing circular imports

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

ddtrace.errortracking._handled_exceptions.bytecode_injector -> ddtrace.errortracking._handled_exceptions.callbacks -> ddtrace.errortracking._handled_exceptions.collector -> ddtrace.errortracking._handled_exceptions.bytecode_reporting -> ddtrace.errortracking._handled_exceptions.bytecode_injector

cit-pr-commenter-54b7da · 2026-09-04T15:28:41Z

Dependency direction analysis

⚠️ Existing dependency direction violations

There are 163 dependency direction violations that already exist on the base branch and have not been changed by this PR.

Show existing violations (showing 5 of 163 highest severity)

ddtrace.internal.tracemethods -×-> ddtrace.trace  (internal-core -> product:tracing, score=121)
ddtrace.llmobs._integrations.anthropic -×-> ddtrace.trace  (product:llmobs -> product:tracing, score=119)
ddtrace.llmobs._integrations.vertexai -×-> ddtrace.trace  (product:llmobs -> product:tracing, score=119)
ddtrace.aiguard._api_client -×-> ddtrace.trace  (product:aiguard -> product:tracing, score=119)
ddtrace.llmobs._integrations.mcp -×-> ddtrace.trace  (product:llmobs -> product:tracing, score=119)

To see all violations, download the layers-base.json and layers-pr.json artifacts from this CI job and run:
uv run --script scripts/import-analysis/layers.py compare layers-base.json layers-pr.json

datadog-official · 2026-09-04T15:48:26Z

✅ All CI checks and tests passed.

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: 62222d5 | Docs | View more details | Give us feedback!}

pr-commenter · 2026-09-04T16:00:36Z

Benchmarks

Benchmark execution time: 2026-10-13 08:41:58

Comparing candidate commit 62222d5 in PR branch kowalski/fix-internal-fix-crash-at-finalisation with baseline commit f769eb0 in branch main.

📊 Benchmarking dashboard

Found 0 performance improvements and 6 performance regressions! Performance is the same for 547 metrics, 10 unstable metrics, 7 known flaky benchmarks, 17 flaky benchmarks without significant changes.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

🟩 = significantly better candidate vs. baseline
🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

scenario:httppropagationextract-empty_headers

🟥 execution_time [+143.918ns; +158.073ns] or [+21.315%; +23.412%]

scenario:httppropagationextract-wsgi_empty_headers

🟥 execution_time [+48.998ns; +61.531ns] or [+7.273%; +9.134%]

scenario:iastaspectsremodule-re_expand_aspect

🟥 execution_time [+74.434µs; +82.240µs] or [+11.696%; +12.922%]

scenario:msgpackencoderscenario-simple_one_span

🟥 execution_time [+512.729ns; +572.477ns] or [+13.262%; +14.807%]

scenario:otelspan-start

🟥 execution_time [+2.612ms; +3.474ms] or [+10.019%; +13.326%]

scenario:recursivecomputation-shallow

🟥 execution_time [+60.670µs; +64.559µs] or [+8.720%; +9.279%]

Unstable benchmarks

These benchmarks have a confidence interval too wide to call a change; treat them as noise rather than signal.

scenario:coreapiscenario-context_with_data_listeners

unstable execution_time [-813.683ns; +656.940ns] or [-7.824%; +6.317%]

scenario:coreapiscenario-core_dispatch_1_listener

unstable execution_time [-41.751ns; +36.917ns] or [-6.273%; +5.547%]

scenario:coreapiscenario-core_dispatch_50_listeners

unstable execution_time [-1960.005ns; +1857.175ns] or [-9.855%; +9.338%]

scenario:coreapiscenario-core_dispatch_exception_listeners

unstable execution_time [-1830.941ns; +2205.653ns] or [-9.421%; +11.349%]

scenario:coreapiscenario-core_dispatch_listeners

unstable execution_time [-380.098ns; +375.597ns] or [-8.927%; +8.821%]

scenario:coreapiscenario-core_dispatch_no_args_listeners

unstable execution_time [-251.285ns; +203.399ns] or [-9.349%; +7.567%]

scenario:coreapiscenario-core_dispatch_with_results_1_listener

unstable execution_time [-79.192ns; +105.685ns] or [-5.773%; +7.705%]

scenario:coreapiscenario-core_dispatch_with_results_50_listeners

unstable execution_time [-5522.614ns; +3814.366ns] or [-11.344%; +7.835%]

scenario:coreapiscenario-core_dispatch_with_results_listeners

unstable execution_time [-1177.258ns; +697.050ns] or [-11.461%; +6.786%]

scenario:packagesupdateimporteddependencies-import_many_stdlib_cached

unstable execution_time [-53060.057ns; +53744.523ns] or [-9.503%; +9.626%]

Known flaky benchmarks

These benchmarks are marked as flaky and will not trigger a failure. Modify FLAKY_BENCHMARKS_REGEX to control which benchmarks are marked as flaky.

scenario:httppropagationinject-ids_only

🟥 execution_time [+2.278µs; +2.407µs] or [+16.557%; +17.496%]

scenario:iastaspects-translate_aspect

🟥 execution_time [+111.711µs; +117.595µs] or [+16.405%; +17.269%]

scenario:iastaspectsospath-ospathbasename_aspect

🟥 execution_time [+149.695µs; +155.823µs] or [+38.368%; +39.939%]

scenario:iastaspectssplit-rsplit_aspect

🟥 execution_time [+41.641µs; +45.599µs] or [+26.958%; +29.520%]

scenario:span-start

🟥 execution_time [+1.400ms; +1.701ms] or [+12.637%; +15.357%]

scenario:telemetryaddmetric-1-count-metric-1-times

🟥 execution_time [+269.902ns; +304.188ns] or [+14.158%; +15.956%]

scenario:tracer-small

🟥 execution_time [+44.407µs; +46.080µs] or [+18.185%; +18.870%]

Known flaky benchmarks without significant changes:

scenario:errortrackingflasksqli-baseline
scenario:flasksimple-iast-get
scenario:iastaspects-casefold_aspect
scenario:iastaspects-casefold_noaspect
scenario:iastaspects-index_aspect
scenario:iastaspects-ljust_noaspect
scenario:iastaspects-lower_aspect
scenario:iastaspects-replace_aspect
scenario:iastaspects-rstrip_aspect
scenario:iastaspects-swapcase_aspect
scenario:iastaspects-title_noaspect
scenario:iastaspects-translate_noaspect
scenario:iastaspects-upper_noaspect
scenario:packagespackageforrootmodulemapping-cache_off
scenario:packagespackageforrootmodulemapping-cache_on
scenario:sethttpmeta-all-enabled
scenario:telemetryaddmetric-record-100-metrics

P403n1x87

LGTM modulo open conversations

VianneyRuhlmann

LGTM, maybe add a regression test crashing for py<3.14 but that might be a bit complicated to setup

chatgpt-codex-connector · 2026-09-22T14:05:20Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-13T08:15:46.254568Z	`62222d5`	New commits
🔒 Security Review	✅ Completed 2026-10-13T08:19:26.656961Z	`62222d5`	New commits

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3790fa4574

ℹ️ 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".

chatgpt-codex-connector · 2026-09-22T14:20:02Z

+    let _ = py; // The GIL is held on entry, which PyEval_SaveThread requires.
+    let save = unsafe { ffi::PyEval_SaveThread() };


Suspend PyO3 attachment state while releasing the GIL

Calling PyEval_SaveThread() directly leaves PyO3's thread-local attachment count indicating that this thread is still attached, unlike Python::detach. This is already unsafe for TraceExporterPy::send and the other byte-payload callers: their moved PyBackedBytes captures are destroyed when catch_unwind returns, before the GIL is restored, so PyO3 can perform the owner's Py_DECREF while believing the GIL is held. Preserve PyO3's suspension bookkeeping (or otherwise keep every Python-owned value alive until after restoration) to avoid refcount corruption or crashes during ordinary sends.

AGENTS.md reference: AGENTS.md:L135-L135

Useful? React with 👍 / 👎.

KowalskiThomas added the Tracing Distributed Tracing label Sep 4, 2026

VianneyRuhlmann reviewed Sep 7, 2026

View reviewed changes

Comment thread src/native/gil.rs

Comment thread src/native/gil.rs Outdated

Comment thread src/native/gil.rs Outdated

Comment thread src/native/gil.rs Outdated

P403n1x87 approved these changes Sep 16, 2026

View reviewed changes

KowalskiThomas requested a review from VianneyRuhlmann September 16, 2026 13:17

VianneyRuhlmann approved these changes Sep 22, 2026

View reviewed changes

Comment thread src/native/gil.rs Outdated

KowalskiThomas marked this pull request as ready for review September 22, 2026 14:05

KowalskiThomas requested review from a team as code owners September 22, 2026 14:05

KowalskiThomas requested review from ZStriker19 and juanjux and removed request for a team September 22, 2026 14:05

KowalskiThomas force-pushed the kowalski/fix-internal-fix-crash-at-finalisation branch from 3790fa4 to 524b2e7 Compare September 22, 2026 14:05

chatgpt-codex-connector Bot reviewed Sep 22, 2026

View reviewed changes

KowalskiThomas force-pushed the kowalski/fix-internal-fix-crash-at-finalisation branch from 259c4ff to 9af8e79 Compare September 23, 2026 12:49

KowalskiThomas added 5 commits October 13, 2026 10:11

fix(internal): fix crash at finalisation

9c31936

fix: fix the same crash in other threads

818567a

review: apply suggestions

9b02d19

review: record main thread after fork

a901eda

review: address codex comments

62222d5

KowalskiThomas force-pushed the kowalski/fix-internal-fix-crash-at-finalisation branch from 9af8e79 to 62222d5 Compare October 13, 2026 08:11

juanjux approved these changes Oct 19, 2026

View reviewed changes

		let _ = py; // The GIL is held on entry, which PyEval_SaveThread requires.
		let save = unsafe { ffi::PyEval_SaveThread() };

Conversation

KowalskiThomas commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

cit-pr-commenter-54b7da Bot commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codeowners resolved as

Uh oh!

cit-pr-commenter-54b7da Bot commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Circular import analysis

⚠️ Existing circular imports

Uh oh!

cit-pr-commenter-54b7da Bot commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependency direction analysis

⚠️ Existing dependency direction violations

Uh oh!

datadog-official Bot commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pr-commenter Bot commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

Explanation

More details about the CI and significant changes

scenario:httppropagationextract-empty_headers

scenario:httppropagationextract-wsgi_empty_headers

scenario:iastaspectsremodule-re_expand_aspect

scenario:msgpackencoderscenario-simple_one_span

scenario:otelspan-start

scenario:recursivecomputation-shallow

Unstable benchmarks

scenario:coreapiscenario-context_with_data_listeners

scenario:coreapiscenario-core_dispatch_1_listener

scenario:coreapiscenario-core_dispatch_50_listeners

scenario:coreapiscenario-core_dispatch_exception_listeners

scenario:coreapiscenario-core_dispatch_listeners

scenario:coreapiscenario-core_dispatch_no_args_listeners

scenario:coreapiscenario-core_dispatch_with_results_1_listener

scenario:coreapiscenario-core_dispatch_with_results_50_listeners

scenario:coreapiscenario-core_dispatch_with_results_listeners

scenario:packagesupdateimporteddependencies-import_many_stdlib_cached

Known flaky benchmarks

scenario:httppropagationinject-ids_only

scenario:iastaspects-translate_aspect

scenario:iastaspectsospath-ospathbasename_aspect

scenario:iastaspectssplit-rsplit_aspect

scenario:span-start

scenario:telemetryaddmetric-1-count-metric-1-times

scenario:tracer-small

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

P403n1x87 left a comment

Choose a reason for hiding this comment

Uh oh!

VianneyRuhlmann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

KowalskiThomas commented Sep 4, 2026 •

edited

Loading

cit-pr-commenter-54b7da Bot commented Sep 4, 2026 •

edited

Loading

cit-pr-commenter-54b7da Bot commented Sep 4, 2026 •

edited

Loading

cit-pr-commenter-54b7da Bot commented Sep 4, 2026 •

edited

Loading

datadog-official Bot commented Sep 4, 2026 •

edited

Loading

pr-commenter Bot commented Sep 4, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Sep 22, 2026 •

edited

Loading