Skip to content

Commit f49bd41

Browse files
fix: address incompatibilities between ddtrace and the builtin multiprocessing library [backport 1.18] (#6756)
Backport 68becb8 from #6566 to 1.18. ## Description - Resolves an import error raised in `ddtrace/appsec/iast/taint_sinks/_base.py` when module unloading is enabled. - Removes inlined import in `ddtrace/internal/remoteconfig/_connectors.py`. This will allow us to unload all ddtrace references to multiprocessing library when ddtrace-run is used. - One nice side-effect of this PR is that it makes gevent compatible with iast (I hope 🤞) ## Motivation Resolves: #6511 [multiprocess.set_start_method(...)](https://docs.python.org/3/library/multiprocessing.html#multiprocessing.set_start_method) should only be called in a process ONCE. Calling this method more than once raises a [runtime error](#6511). Using module unloading and refactoring imports allows the ddtrace library to have a unique reference to the multiprocessing library. ## 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/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## 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. - [x] 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) Co-authored-by: Munir Abdinur <[email protected]>
1 parent a2776ce commit f49bd41

File tree

3 files changed

+58
-2
lines changed

3 files changed

+58
-2
lines changed

ddtrace/internal/remoteconfig/_connectors.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from ctypes import c_char
22
import json
3+
import multiprocessing
34
import os
45
import sys
56
from typing import Any
@@ -38,8 +39,6 @@ class PublisherSubscriberConnector(object):
3839
"""
3940

4041
def __init__(self):
41-
import multiprocessing
42-
4342
self.data = multiprocessing.Array(c_char, SHARED_MEMORY_SIZE, lock=False)
4443
# Checksum attr validates if the Publisher send new data
4544
self.checksum = -1
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
issues:
3+
- |
4+
There are known issues configuring python's builtin multiprocessing library when ddtrace is installed.
5+
To use the multiprocessing library with ddtrace ensure ``DD_UNLOAD_MODULES_FROM_SITECUSTOMIZE`` is set to ``True``.

tests/appsec/test_remoteconfiguration_e2e.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,3 +374,55 @@ def test_load_testing_appsec_1click_and_ip_blocking_gunicorn_block_and_kill_chil
374374
_unblock_ip(token)
375375

376376
_request_200(gunicorn_client, debug_mode=False)
377+
378+
379+
@pytest.mark.parametrize(
380+
"module_unloading_env",
381+
[
382+
True,
383+
pytest.param(
384+
False,
385+
marks=pytest.mark.xfail(
386+
reason="FIXME: multiprocessing is only supported when DD_UNLOAD_MODULES_FROM_SITECUSTOMIZE is set"
387+
),
388+
),
389+
],
390+
ids=[
391+
"module_unloading_enabled",
392+
"module_unloading_unset",
393+
],
394+
)
395+
@pytest.mark.skipif(sys.version_info[0] < 3, reason="Python2.7 is not supported")
396+
def test_compatiblity_with_multiprocessing(module_unloading_env, ddtrace_run_python_code_in_subprocess):
397+
"""This test validates that module unloading resolves multiprocessing context errors"""
398+
code = """
399+
import multiprocessing
400+
from multiprocessing import Process, Value, Array
401+
402+
def f(n, a):
403+
n.value = 420
404+
for i in range(len(a)):
405+
a[i] = i*10
406+
407+
if __name__ == '__main__':
408+
multiprocessing.set_start_method('spawn')
409+
num = Value('d', 0.0)
410+
arr = Array('i', range(10))
411+
412+
p = Process(target=f, args=(num, arr))
413+
p.start()
414+
p.join()
415+
416+
assert arr[:] == [0, 10, 20, 30, 40, 50, 60, 70, 80, 90]
417+
assert num.value == 420
418+
print("success")
419+
"""
420+
env = os.environ.copy()
421+
env["DD_REMOTE_CONFIGURATION_ENABLED"] = "true"
422+
423+
if module_unloading_env is True:
424+
env["DD_UNLOAD_MODULES_FROM_SITECUSTOMIZE"] = "true"
425+
426+
out, stderr, status, _ = ddtrace_run_python_code_in_subprocess(code, env=env)
427+
assert status == 0, stderr
428+
assert out == b"success\n"

0 commit comments

Comments
 (0)