Skip to content

Commit beaceb3

Browse files
fix: add timeout handling for subprocess wait [backport 3.3] (#12904)
Backport b518cfd from #12806 to 3.3. After subprocess is opened, wait and kill if Timeout is exceeded to ensure zombie processes are killed. ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) https://datadoghq.atlassian.net/browse/APMLP-313 Co-authored-by: Quinna Halim <[email protected]>
1 parent d7a5b54 commit beaceb3

File tree

2 files changed

+37
-10
lines changed

2 files changed

+37
-10
lines changed

lib-injection/sources/sitecustomize.py

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ def gen_telemetry_payload(telemetry_events, ddtrace_version):
148148
}
149149

150150

151-
def send_telemetry(event):
151+
def _send_telemetry(event):
152152
event_json = json.dumps(event)
153153
_log("maybe sending telemetry to %s" % FORWARDER_EXECUTABLE, level="debug")
154154
if not FORWARDER_EXECUTABLE or not TELEMETRY_ENABLED:
@@ -161,15 +161,38 @@ def send_telemetry(event):
161161
stderr=subprocess.PIPE,
162162
universal_newlines=True,
163163
)
164-
if p.stdin:
165-
p.stdin.write(event_json)
166-
p.stdin.close()
167-
_log("wrote telemetry to %s" % FORWARDER_EXECUTABLE, level="debug")
168-
else:
169-
_log(
170-
"failed to write telemetry to %s, could not write to telemetry writer stdin" % FORWARDER_EXECUTABLE,
171-
level="error",
172-
)
164+
# Mimic Popen.__exit__ which was added in Python 3.3
165+
try:
166+
if p.stdin:
167+
p.stdin.write(event_json)
168+
_log("wrote telemetry to %s" % FORWARDER_EXECUTABLE, level="debug")
169+
else:
170+
_log(
171+
"failed to write telemetry to %s, could not write to telemetry writer stdin" % FORWARDER_EXECUTABLE,
172+
level="error",
173+
)
174+
finally:
175+
if p.stdin:
176+
p.stdin.close()
177+
if p.stderr:
178+
p.stderr.close()
179+
if p.stdout:
180+
p.stdout.close()
181+
182+
# backwards compatible `p.wait(1)`
183+
start = time.time()
184+
while p.poll() is None:
185+
if time.time() - start > 1:
186+
p.kill()
187+
break
188+
time.sleep(0.05)
189+
190+
191+
def send_telemetry(event):
192+
try:
193+
_send_telemetry(event)
194+
except Exception as e:
195+
_log("Failed to send telemetry: %s" % e, level="error")
173196

174197

175198
def _get_clib():
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
lib-injection: Avoid zombie process from telemetry sender on startup.

0 commit comments

Comments
 (0)