Skip to content

Commit 09b54db

Browse files
authored
chore: initialise HTTP client when cloning modules (#14777)
## Description A recent attempt to reduce start-up time by lazy-loading the HTTP client modules resulted in a potential gevent support regression whereby the lazily created HTTP client could end up making requests via the gevent-monkey-patched socket module. This introduces the possibility of spurious deadlock during the processing of requests. We prevent this by force-importing the HTTP clients on bootstrap to ensure that we grab references to an original copy of the socket module. This prevents our HTTP clients from calling into gevent when uploading payloads to the agent, preventing the potential deadlocks.
1 parent b8cb1b2 commit 09b54db

File tree

4 files changed

+48
-12
lines changed

4 files changed

+48
-12
lines changed

ddtrace/appsec/_asm_request_context.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import json
33
import re
44
from types import TracebackType
5+
from typing import TYPE_CHECKING
56
from typing import Any
67
from typing import Callable
78
from typing import Dict
@@ -17,8 +18,6 @@
1718
from ddtrace.appsec._constants import APPSEC
1819
from ddtrace.appsec._constants import SPAN_DATA_NAMES
1920
from ddtrace.appsec._constants import Constant_Class
20-
from ddtrace.appsec._utils import DDWaf_info
21-
from ddtrace.appsec._utils import DDWaf_result
2221
from ddtrace.appsec._utils import Telemetry_result
2322
from ddtrace.appsec._utils import get_triggers
2423
from ddtrace.contrib.internal.trace_utils_base import _normalize_tag_name
@@ -29,6 +28,10 @@
2928
from ddtrace.settings.asm import config as asm_config
3029

3130

31+
if TYPE_CHECKING:
32+
from ddtrace.appsec._utils import DDWaf_info
33+
from ddtrace.appsec._utils import DDWaf_result
34+
3235
logger = ddlogger.get_logger(__name__)
3336

3437

@@ -95,7 +98,7 @@ def __init__(self, span: Optional[Span] = None, rc_products: str = ""):
9598
else:
9699
self.framework = self.span.name
97100
self.framework = self.framework.lower().replace(" ", "_")
98-
self.waf_info: Optional[Callable[[], DDWaf_info]] = None
101+
self.waf_info: Optional[Callable[[], "DDWaf_info"]] = None
99102
self.waf_addresses: Dict[str, Any] = {}
100103
self.callbacks: Dict[str, Any] = {_CONTEXT_CALL: []}
101104
self.telemetry: Telemetry_result = Telemetry_result()
@@ -375,15 +378,15 @@ def set_waf_callback(value) -> None:
375378
set_value(_CALLBACKS, _WAF_CALL, value)
376379

377380

378-
def set_waf_info(info: Callable[[], DDWaf_info]) -> None:
381+
def set_waf_info(info: Callable[[], "DDWaf_info"]) -> None:
379382
env = _get_asm_context()
380383
if env is None:
381384
logger.warning(WARNING_TAGS.SET_WAF_INFO_NO_ASM_CONTEXT, extra=log_extra, stack_info=True)
382385
return
383386
env.waf_info = info
384387

385388

386-
def call_waf_callback(custom_data: Optional[Dict[str, Any]] = None, **kwargs) -> Optional[DDWaf_result]:
389+
def call_waf_callback(custom_data: Optional[Dict[str, Any]] = None, **kwargs) -> Optional["DDWaf_result"]:
387390
if not asm_config._asm_enabled:
388391
return None
389392
callback = get_value(_CALLBACKS, _WAF_CALL)
@@ -480,7 +483,7 @@ def asm_request_context_set(
480483
def set_waf_telemetry_results(
481484
rules_version: str,
482485
is_blocked: bool,
483-
waf_results: DDWaf_result,
486+
waf_results: "DDWaf_result",
484487
rule_type: Optional[str],
485488
is_sampled: bool,
486489
) -> None:

ddtrace/appsec/_utils.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
from typing import List
1111
from typing import Optional
1212
from typing import Union
13-
import uuid
1413

1514
from ddtrace.appsec._constants import API_SECURITY
1615
from ddtrace.appsec._constants import APPSEC
@@ -236,6 +235,10 @@ def _safe_userid(user_id):
236235
return user_id
237236
except ValueError:
238237
try:
238+
# Import uuid lazily because this also imports threading via the
239+
# platform module
240+
import uuid
241+
239242
_ = uuid.UUID(user_id)
240243
return user_id
241244
except ValueError:

ddtrace/bootstrap/sitecustomize.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,11 @@ def drop(module_name):
5555
if not asbool(do_cleanup):
5656
return
5757

58+
# We need to import these modules to make sure they grab references to the
59+
# right modules before we start unloading stuff.
60+
import ddtrace.internal.http # noqa
61+
import ddtrace.internal.uds # noqa
62+
5863
# Unload all the modules that we have imported, except for the ddtrace one.
5964
# NB: this means that every `import threading` anywhere in `ddtrace/` code
6065
# uses a copy of that module that is distinct from the copy that user code
@@ -94,6 +99,10 @@ def drop(module_name):
9499
# submodule makes use of threading so it is critical to unload when
95100
# gevent is used.
96101
"concurrent.futures",
102+
# We unload the threading module in case it was imported by
103+
# CPython on boot.
104+
"threading",
105+
"_thread",
97106
]
98107
)
99108
for u in UNLOAD_MODULES:

tests/internal/test_auto.py

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,39 @@
1-
import sys
2-
31
import pytest
42

53

6-
@pytest.mark.skipif(sys.version_info < (3, 12, 5), reason="Python < 3.12.5 eagerly loads the threading module")
4+
# DEV: This test must pass ALWAYS. If this test fails, it means that something
5+
# needs to be fixed somewhere. Please DO NOT skip this test under any
6+
# circumstance!
77
@pytest.mark.subprocess(
88
env=dict(
99
DD_UNLOAD_MODULES_FROM_SITECUSTOMIZE="true",
10-
DD_REMOTE_CONFIGURATION_ENABLED="1",
11-
)
10+
),
11+
parametrize={
12+
"DD_REMOTE_CONFIGURATION_ENABLED": ("1", "0"),
13+
},
1214
)
1315
def test_auto():
16+
import os
1417
import sys
1518

1619
import ddtrace.auto # noqa:F401
1720

1821
assert "threading" not in sys.modules
22+
assert "socket" not in sys.modules
23+
assert "subprocess" not in sys.modules
24+
25+
if os.getenv("DD_REMOTE_CONFIGURATION_ENABLED") == "0":
26+
# When unloading modules we must have the HTTP clients imported already
27+
assert "ddtrace.internal.http" in sys.modules
28+
29+
# emulate socket patching (e.g. by gevent)
30+
import socket # noqa:F401
31+
32+
socket.create_connection = None
33+
socket.socket = None
34+
35+
from ddtrace.internal.http import HTTPConnection # noqa:F401
36+
import ddtrace.internal.uds as uds
37+
38+
assert HTTPConnection("localhost")._create_connection is not None
39+
assert uds.socket.socket is not None

0 commit comments

Comments
 (0)