fix(sdk): run process-dependent resource detectors synchronously in fork handlers (#5789) by chiragjangiir · Pull Request #5790 · open-telemetry/opentelemetry-python

chiragjangiir · 2026-10-16T15:01:40Z

Description

Background & Root Cause

In opentelemetry-sdk >= 1.44, providers (TracerProvider, MeterProvider, LoggerProvider) register os.register_at_fork(after_in_child=...) hooks to refresh process-dependent resources via _get_process_dependent_resource().

Previously, _get_process_dependent_resource() delegated to get_aggregated_resources(), which spawns a concurrent.futures.ThreadPoolExecutor.

In CPython's concurrent.futures.thread, fork hooks are registered upon import:

before: acquires _global_shutdown_lock
after_in_child: calls _global_shutdown_lock._at_fork_reinit()

Fork callbacks run after_in_child hooks in order of registration. If a provider is instantiated before concurrent.futures.thread is imported (e.g. applications passing custom resources before importing thread pools or web frameworks):

Parent enters fork; concurrent.futures.thread acquires _global_shutdown_lock.
Child process starts; provider's after_in_child callback executes before concurrent.futures.thread's after_in_child callback.
get_aggregated_resources() creates a ThreadPoolExecutor and calls submit(), which attempts to acquire _global_shutdown_lock.
Because the lock was acquired in the parent and has not been reinitialized yet, the child process deadlocks.
In processes using subprocess.Popen(..., preexec_fn=...), the parent hangs indefinitely waiting for errpipe_read.

Solution

Execute process-dependent detectors synchronously in _get_process_dependent_resource(). The process-dependent detectors (ProcessResourceDetector and ServiceInstanceIdResourceDetector) perform fast, local in-memory lookups (< 0.1 ms). Running them synchronously eliminates thread pool creation and thread lock acquisition in after_in_child while strictly preserving:

Detector ordering
Attribute merging sequence
Error handling and raise_on_error semantics
Warning log format

Verification

Added regression test test_tracer_provider_fork_with_preexec_fn_does_not_deadlock in opentelemetry-sdk/tests/trace/test_trace.py that reliably reproduces the deadlock without the fix and passes with the fix.
Verified test suite passes locally:
- pytest opentelemetry-sdk/tests/trace/test_trace.py -k test_tracer_provider_fork_with_preexec_fn_does_not_deadlock (PASSED)
- pytest opentelemetry-sdk/tests/resources opentelemetry-sdk/tests/trace (372 passed, 2 skipped, 58 subtests passed)
- ruff check and ruff format --check (All passed)
Added changelog entry in .changelog/5789.fixed.

Type of change

Bug fix (non-breaking change which fixes an issue)

Checklist:

Follows project styleguide and changelog guidelines
Added regression unit test with safe subprocess cleanup
Ran relevant tests and linting

…ork handlers (open-telemetry#5789) Refresh process-dependent resources synchronously inside _get_process_dependent_resource() rather than delegating to get_aggregated_resources(). Delegating to get_aggregated_resources() spawns a ThreadPoolExecutor in the fork handler's after_in_child hook. If concurrent.futures.thread was imported after the provider, the executor's submit() call deadlocks trying to acquire _global_shutdown_lock before concurrent.futures.thread's after_in_child hook has a chance to reinitialize the lock. Running the detectors synchronously eliminates thread pool worker creation and lock acquisition in the newly forked child, preserving detector order, exception handling, raise_on_error semantics, and attribute merging without deadlock hazards. Fixes open-telemetry#5789.

linux-foundation-easycla · 2026-10-16T15:01:50Z

The committers listed above are authorized under a signed CLA.

✅ login: chiragjangiir / name: Chirag Jangir (a731d3f)

opentelemetry-pr-dashboard · 2026-10-16T15:02:21Z

Pull request dashboard status

Waiting on maintainers · refreshed 2026-10-23 21:29 UTC

Merge when ready.

Status above doesn't look right?

Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

Copilot

🟡 Changes recommended

Synchronous aggregation still creates a thread pool when the initial resource is omitted.

1 open issue

Avoid thread pool creation when synchronous aggregation omits initial_resource · New

What changed in this PR

Addresses #5789 by running process-dependent resource detectors synchronously during SDK fork callbacks.

Changes:

Adds synchronous resource aggregation and uses it for fork-time refreshes.
Adds aggregation tests and a subprocess deadlock regression test.
Documents the fix in the changelog.

File	Description
opentelemetry-sdk/tests/trace/test_trace.py	Adds fork deadlock regression coverage with subprocess cleanup.
opentelemetry-sdk/tests/resources/test_resources.py	Tests synchronous merging, error handling, and thread-pool avoidance.
opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py	Adds synchronous detection for process-dependent resource refreshes.
.changelog/5789.fixed	Records the fork deadlock fix.

_{Review effort: Balanced}

herin049 · 2026-10-18T03:20:29Z

    """
    detectors_merged_resource = initial_resource or Resource.create()

+    if not use_threads:


This still just duplicates the code, we should adapt this to be a true generalization.

…us mode on default creation Refactor get_aggregated_resources() to unify detector result resolution and error handling across both threaded and synchronous execution paths, eliminating duplicated loops. Additionally, pass `use_threads` to Resource.create() when initial_resource is omitted so synchronous aggregation does not spawn a thread pool.

chiragjangiir · 2026-10-18T04:28:37Z

Thanks for the feedback @herin049. I've refactored get_aggregated_resources() to unify detector execution and error handling into a single shared loop, eliminating the duplicated logic between the synchronous and threaded paths.

I also addressed the case where initial_resource is omitted: Resource.create() now accepts and forwards use_threads, ensuring that calls such as get_aggregated_resources([], use_threads=False) execute entirely synchronously without spawning an implicit thread pool. All unit tests, the thread-pool avoidance assertions, and the fork-deadlock regression tests pass.

Copilot

🟢 Approval recommended

The change removes the reported lock dependency, preserves default threaded behavior, and includes focused regression coverage with no unresolved findings.

No open issues 🎉

1 resolved since last review

Avoid thread pool creation when synchronous aggregation omits initial_resource

_{Review effort: Balanced}

herin049 · 2026-10-19T16:02:35Z

    detectors: Sequence["ResourceDetector"],
    initial_resource: Resource | None = None,
    timeout: int = 5,
+    use_threads: bool = True,


Lets make this a keyword only, private argument.

Suggested change

use_threads: bool = True,

*,

_use_thread_pool: bool = True,

herin049 · 2026-10-19T16:03:58Z

    def create(
        attributes: Attributes | None = None,
        schema_url: str | None = None,
+        use_threads: bool = True,


Suggested change

use_threads: bool = True,

*,

_use_thread_pool: bool = True,

herin049 · 2026-10-19T16:04:04Z

        Args:
            attributes: Optional zero or more key-value pairs.
            schema_url: Optional URL pointing to the schema
+            use_threads: Optional flag to run detectors in a thread pool or synchronously


Suggested change

use_threads: Optional flag to run detectors in a thread pool or synchronously

herin049 · 2026-10-19T16:04:15Z

    :param initial_resource: Static resource. This has highest priority
-    :param timeout: Number of seconds to wait for each detector to return
+    :param timeout: Number of seconds to wait for each detector to return when running in a thread pool
+    :param use_threads: True to run detectors in a thread pool, False to run synchronously (timeouts do not apply in synchronous mode)


Suggested change

:param use_threads: True to run detectors in a thread pool, False to run synchronously (timeouts do not apply in synchronous mode)

herin049 · 2026-10-19T16:04:44Z

+            # A detector that exceeds `timeout` is skipped, but shutdown still joins
+            # its worker so no threads outlive this call. A detector that blocks
+            # indefinitely therefore blocks this call. `cancel_futures=True` drops
+            # detectors that have not started yet if a `raise_on_error` detector raises.


These comments don't add much value.

Suggested change

# A detector that exceeds `timeout` is skipped, but shutdown still joins

# its worker so no threads outlive this call. A detector that blocks

# indefinitely therefore blocks this call. `cancel_futures=True` drops

# detectors that have not started yet if a `raise_on_error` detector raises.

- Make thread pool configuration private keyword-only (_use_thread_pool) in get_aggregated_resources() and Resource.create() per review feedback - Use context manager for subprocess.Popen in fork deadlock regression test to resolve pylint R1732 - Rename changelog fragment to 5790.fixed to match pull request number

herin049 · 2026-10-20T15:25:25Z

+        "requires os.fork and os.register_at_fork",
+    )
+    def test_tracer_provider_fork_with_preexec_fn_does_not_deadlock(self):
+        code = (


Can we clean this up to follow the pattern used by the other fork-based tests?

chiragjangiir · 2026-10-20T16:19:01Z

Thanks for the feedback @herin049! I've moved the fork-test runner into a standalone helper script under opentelemetry-sdk/tests/trace/scripts/tracer_provider_fork_preexec.py and updated the test to invoke it using sys.executable. I've also addressed the ruff format lint issue in resources/__init__.py. The relevant fork and resource tests pass locally. Please let me know if you'd like any further changes.

chiragjangiir · 2026-10-21T05:37:41Z

Hi @herin049,

The recent CI failure on lint / opentelemetry-sdk was caused by Pylint flagging the new standalone test helper (W1509 and R1732). I've resolved this in commit 0f33eb8 by wrapping the subprocess in a context manager and adding the Pylint suppression directive.

Pylint now rates the module at 10.00/10 and the fork deadlock regression tests pass locally.

Since opentelemetry-sdk 1.44.0, a TracerProvider registers an os.fork() handler, which runs a ThreadPoolExecutor in the child. concurrent.futures.thread registers fork handlers as well: it acquires its global lock before the fork and resets it in the child. The child handlers run in registration order. When concurrent.futures.thread is imported after the TracerProvider was created, the TracerProvider handler submits to the executor while the lock is still held, and the child deadlocks. This happens in liveproxyd: the first fork of the daemonization runs the TracerProvider handler, which imports concurrent.futures.thread. The second fork then deadlocks. The process keeps the stdout of "omd start" open, so "omd start" never finishes. Until now, the OTLP exporter module imported concurrent.futures.thread through grpc before the TracerProvider was created. Importing the exporter only when needed would expose the deadlock. This is an upstream bug: open-telemetry/opentelemetry-python#5789 The fix open-telemetry/opentelemetry-python#5790 is approved, but not yet released. Until then, cmk.trace imports concurrent.futures.thread itself, so its fork handlers are always registered first. This is the only place which creates a TracerProvider. The import can be removed once we use an SDK release with the fix. The test covers the daemonizing double fork and stays. Change-Id: I32ed037a3b84148eaab57e7a64f656ed5424d5cf Jira-Ref: CMK-40299

chiragjangiir requested a review from a team as a code owner October 16, 2026 15:01

otelbot-python Bot added this to Python PR digest Oct 16, 2026

opentelemetry-pr-dashboard Bot mentioned this pull request Oct 16, 2026

Pull Request Dashboard #5550

Open

herin049 requested changes Oct 16, 2026

View reviewed changes

Comment thread opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py Outdated

github-project-automation Bot moved this to Reviewed PRs that need fixes in Python PR digest Oct 16, 2026

fix(sdk): reuse resource aggregation in fork handlers

fe0214e

herin049 requested a balanced review from Copilot October 18, 2026 03:15

Copilot started reviewing on behalf of herin049 October 18, 2026 03:16

View session

Copilot AI reviewed Oct 18, 2026

View reviewed changes

Comment thread opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py Outdated

herin049 reviewed Oct 18, 2026

View reviewed changes

Merge branch 'main' into fix/issue-5789-fork-handler-deadlock

27687aa

herin049 requested a balanced review from Copilot October 19, 2026 15:56

Copilot started reviewing on behalf of herin049 October 19, 2026 15:56

View session

Copilot AI reviewed Oct 19, 2026

View reviewed changes

herin049 requested changes Oct 19, 2026

View reviewed changes

herin049 reviewed Oct 20, 2026

View reviewed changes

test(trace): move fork preexec test logic to scripts helper and reformat

bcb1626

herin049 approved these changes Oct 20, 2026

View reviewed changes

fix(sdk): address pylint W1509 and R1732 in fork test helper

0f33eb8

Merge branch 'main' into fix/issue-5789-fork-handler-deadlock

bcb618a

Conversation

chiragjangiir commented Oct 16, 2026

Description

Background & Root Cause

Solution

Verification

Type of change

Checklist:

Uh oh!

linux-foundation-easycla Bot commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

opentelemetry-pr-dashboard Bot commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request dashboard status

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

herin049 Oct 18, 2026

Choose a reason for hiding this comment

Uh oh!

chiragjangiir commented Oct 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

herin049 Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

herin049 Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

herin049 Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

herin049 Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

herin049 Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

herin049 Oct 20, 2026

Choose a reason for hiding this comment

Uh oh!

chiragjangiir commented Oct 20, 2026

Uh oh!

chiragjangiir commented Oct 21, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

linux-foundation-easycla Bot commented Oct 16, 2026 •

edited

Loading

opentelemetry-pr-dashboard Bot commented Oct 16, 2026 •

edited

Loading