Skip to content

Commit 478d90a

Browse files
authored
fix(profiling): memalloc clean restart after fork [backport #5586 to 1.12] (#5614)
Backport of #5586 to 1.12 This change ensures that the memalloc module of the profiler is restarted cleanly after a fork. This fixes a regression introduced by a previous change whereby error messages would be logged about failed attempts to restart the memalloc module. ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines) are followed. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] PR description includes explicit acknowledgement/acceptance of the performance implications of this PR as reported in the benchmarks PR comment. ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment.
1 parent 3bb826c commit 478d90a

File tree

3 files changed

+27
-1
lines changed

3 files changed

+27
-1
lines changed

ddtrace/profiling/collector/memalloc.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,14 @@ def _start_service(self):
105105
if _memalloc is None:
106106
raise collector.CollectorUnavailable
107107

108-
_memalloc.start(self.max_nframe, self._max_events, self.heap_sample_size)
108+
try:
109+
_memalloc.start(self.max_nframe, self._max_events, self.heap_sample_size)
110+
except RuntimeError:
111+
# This happens on fork because we don't call the shutdown hook since
112+
# the thread responsible for doing so is not running in the child
113+
# process. Therefore we stop and restart the collector instead.
114+
_memalloc.stop()
115+
_memalloc.start(self.max_nframe, self._max_events, self.heap_sample_size)
109116

110117
super(MemoryCollector, self)._start_service()
111118

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
fixes:
3+
- |
4+
profiling: Fixed a regression in the memory collector that caused it to fail
5+
to cleanly re-initialize after a fork, causing error messages to be logged.

tests/profiling/test_main.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,3 +110,17 @@ def test_multiprocessing(method, tmp_path, monkeypatch):
110110
pid, child_pid = list(s.strip() for s in stdout.decode().strip().split("\n"))
111111
utils.check_pprof_file(filename + "." + str(pid) + ".1")
112112
utils.check_pprof_file(filename + "." + str(child_pid) + ".1")
113+
114+
115+
@pytest.mark.subprocess(
116+
ddtrace_run=True,
117+
env=dict(DD_PROFILING_ENABLED="1"),
118+
err=lambda _: "RuntimeError: the memalloc module is already started" not in _,
119+
)
120+
def test_memalloc_no_init_error_on_fork():
121+
import os
122+
123+
pid = os.fork()
124+
if not pid:
125+
exit(0)
126+
os.waitpid(pid, 0)

0 commit comments

Comments
 (0)