Synchronize native transport fork teardown - #6127
Conversation
Fork hooks, sends, and transport teardown share the exporter and its runtime. Serialize `close` and finalization against active fork lifecycles and in-flight sends so hooks cannot disappear mid-fork or the exporter be released during a send. Use a blocking mock agent to exercise these races deterministically instead of relying on timing delays.
A failed fork or `:before` callback can leave earlier callbacks holding resources. Always run the matching `:parent` callbacks, and continue running cleanup callbacks after one fails before re-raising the first error. Defer finalizer hook removal when it interrupts an active fork so the runtime and lifecycle locks are restored first.
Capture one callback snapshot for the complete fork lifecycle and register callback triplets atomically. This prevents concurrent registration or removal from pairing a `:before` callback with the wrong `:parent` or `:child` cleanup. Track lock acquisition explicitly so a reentrant `close` cannot release locks owned by the surrounding fork lifecycle.
lloeki
added
the
AI Generated
lloeki
added
the
AI Generated
|
Thank you for updating Change log entry section 👏 Visited at: 2026-07-30 12:12:00 UTC |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ad246116c
ℹ️ 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".
| loop do | ||
| fork_mutex.lock | ||
| break if fork_state[:pending] == 0 |
There was a problem hiding this comment.
When GC invokes this finalizer on a thread other than the one holding fork_mutex for an active fork, the finalizer blocks here without setting fork_state[:finalize]. If the owner forks while it is waiting, that waiting thread disappears in the child; the child callback restores and unlocks the exporter but does not remove the hooks because :finalize remains false, and the already-started finalizer is not rerun there. The child consequently retains the exporter/runtime and its process-global hooks indefinitely, so finalization needs to be marked as pending before this potentially blocking acquisition.
AGENTS.md reference: AGENTS.md:L110-L110
Useful? React with 👍 / 👎.
| rescue Exception # rubocop:disable Lint/RescueException -- re-raised unchanged; we only need to run parent cleanup first | ||
| # The fork failed, so no child was created and we are still in the | ||
| # parent. The `:before` blocks already ran (and may hold resources, | ||
| # e.g. a locked mutex); run the `:parent` blocks so that state is | ||
| # restored, then re-raise. | ||
| AtForkMonkeyPatch.run_at_fork_blocks(:parent) | ||
| # The fork or a before-fork callback failed, so no child was | ||
| # created. Restore state set up by any earlier callbacks. | ||
| AtForkMonkeyPatch.run_at_fork_blocks(:parent, snapshot) |
There was a problem hiding this comment.
When a :before callback raises, this dispatches the entire :parent snapshot even though later :before callbacks were skipped at the first failure. A later callback triplet can therefore have its cleanup invoked without its setup—for example, attempting to unlock an unacquired mutex and replacing the original fork error with ThreadError, or resetting unrelated lifecycle state. Track which before callbacks completed and run only their matching cleanup handlers.
Useful? React with 👍 / 👎.
| def self.snapshot_at_fork_blocks | ||
| AT_FORK_REGISTRY_MUTEX.synchronize do |
There was a problem hiding this comment.
When an application calls fork directly from a Signal.trap handler, every patched fork now reaches this Mutex#synchronize; on MRI this raises ThreadError: can't be called from trap context before the real fork is attempted. A plain fork is otherwise supported from such a handler, and previously the callback arrays were read without locking, so initializing Datadog now breaks this valid process-management pattern even when the native transport is not in use. The snapshot path needs a trap-safe synchronization strategy.
Useful? React with 👍 / 👎.
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: b0c5772 | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-07-30 12:25:52 Comparing candidate commit b0c5772 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 48 metrics, 1 unstable metrics.
|
0ad2461 to
b0c5772
Compare
Strech
left a comment
Strech
left a comment
There was a problem hiding this comment.
It's a bit hard to read and follow tests, but I would give non-blocking 👍🏼
|
|
||
| def self.run_at_fork_blocks(stage) | ||
| blocks_for(stage).each(&:call) | ||
| def self.run_at_fork_blocks(stage, snapshot = nil) |
There was a problem hiding this comment.
Would you consider snapshot: nil kwarg alternative?
3 participants
AI-generated code disclosure: this PR was implemented with substantial AI assistance and manually reviewed and validated.
What does this PR do?
Makes native transport fork lifecycle dispatch atomic and failure-safe. It coordinates
closewith in-flight sends and forks, runs every completion callback before re-raising the first error, and prevents finalizer and reentrant-lock deadlocks.Motivation:
Native transport teardown can race with sends or fork handling, and exceptional callbacks can otherwise leave lifecycle work incomplete. This is the internal safety work tracked by APMSP-3823.
Change log entry
No.
Additional Notes:
How to test the change?