Skip to content

Commit 587d6d6

Browse files
authored
Merge branch 'main' into 4.0-breaking-changes
2 parents a4b42e9 + f7bf348 commit 587d6d6

File tree

10 files changed

+144
-31
lines changed

10 files changed

+144
-31
lines changed

ddtrace/internal/_unpatched.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,13 @@
1010
# to get a reference to the right threading module.
1111
import threading as _threading # noqa
1212
import gc as _gc # noqa
13-
1413
import sys
1514

15+
threading_Lock = _threading.Lock
16+
threading_RLock = _threading.RLock
17+
threading_Event = _threading.Event
18+
19+
1620
previous_loaded_modules = frozenset(sys.modules.keys())
1721
from subprocess import Popen as unpatched_Popen # noqa # nosec B404
1822
from os import close as unpatched_close # noqa: F401, E402

ddtrace/internal/forksafe.py

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,13 @@
55
import functools
66
import logging
77
import os
8-
import threading
98
import typing
109
import weakref
1110

1211
import wrapt
1312

13+
from ddtrace.internal import _unpatched
14+
1415

1516
log = logging.getLogger(__name__)
1617

@@ -138,13 +139,13 @@ def _reset_object(self):
138139
self.__wrapped__ = self._self_wrapped_class()
139140

140141

141-
def Lock() -> threading.Lock:
142-
return ResetObject(threading.Lock) # type: ignore
142+
def Lock() -> _unpatched.threading_Lock:
143+
return ResetObject(_unpatched.threading_Lock) # type: ignore
143144

144145

145-
def RLock() -> threading.RLock:
146-
return ResetObject(threading.RLock) # type: ignore
146+
def RLock() -> _unpatched.threading_RLock:
147+
return ResetObject(_unpatched.threading_RLock) # type: ignore
147148

148149

149-
def Event() -> threading.Event:
150-
return ResetObject(threading.Event) # type: ignore
150+
def Event() -> _unpatched.threading_Event:
151+
return ResetObject(_unpatched.threading_Event) # type: ignore

ddtrace/internal/ipc.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
from contextlib import contextmanager
21
import os
32
import secrets
43
import tempfile
@@ -57,9 +56,7 @@ class ReadLock(BaseUnixLock):
5756
class WriteLock(BaseUnixLock):
5857
__acquire_mode__ = fcntl.LOCK_EX
5958

60-
@contextmanager
61-
def open_file(path, mode):
62-
yield unpatched_open(path, mode)
59+
open_file = unpatched_open
6360

6461
except ModuleNotFoundError:
6562
# Availability: Windows
@@ -78,7 +75,7 @@ def release(self):
7875

7976
ReadLock = WriteLock = BaseWinLock # type: ignore
8077

81-
def open_file(path, mode):
78+
def open_file(path, mode): # type: ignore
8279
import _winapi
8380

8481
# force all modes to be read/write binary
@@ -93,14 +90,17 @@ def open_file(path, mode):
9390
return unpatched_open(fd, mode)
9491

9592

96-
TMPDIR = Path(tempfile.gettempdir())
93+
try:
94+
TMPDIR: typing.Optional[Path] = Path(tempfile.gettempdir())
95+
except FileNotFoundError:
96+
TMPDIR = None
9797

9898

9999
class SharedStringFile:
100100
"""A simple shared-file implementation for multiprocess communication."""
101101

102102
def __init__(self) -> None:
103-
self.filename: typing.Optional[str] = str(TMPDIR / secrets.token_hex(8))
103+
self.filename: typing.Optional[str] = str(TMPDIR / secrets.token_hex(8)) if TMPDIR is not None else None
104104

105105
def put(self, data: str) -> None:
106106
"""Put a string into the file."""
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
Prevent a potential ``ResourceWarning`` in multiprocess scenarios.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
Prevent startup failure when a temporary directory is not available.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
core: This fix resolves an issue where forksafe locks used patched threading primitives from the profiling module, causing performance issues. The forksafe module now uses unpatched threading primitives (``Lock``, ``RLock``, ``Event``).

scripts/cformat.sh

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ enumerate_files() {
4242
for ext in "${extensions[@]}"; do
4343
find_conditions+=("-o" "-name" "$ext")
4444
done
45-
unset 'find_conditions[-1]'
45+
find_conditions=("${find_conditions[@]:1}") # Remove first -o
4646
find "$BASE_DIR" -type f \( "${find_conditions[@]}" \)
4747
else
4848
git ls-files "${extensions[@]}"
@@ -52,7 +52,7 @@ enumerate_files() {
5252
# Script defaults
5353
UPDATE_MODE=false
5454
ENUM_ALL=false
55-
BASE_DIR=$(dirname "$(realpath "$0")")
55+
BASE_DIR=$(dirname "$(dirname "$(realpath "$0")")")
5656
CLANG_FORMAT=clang-format
5757

5858
# NB: consumes the arguments
@@ -67,20 +67,21 @@ while (( "$#" )); do
6767
*)
6868
;;
6969
esac
70+
shift
7071
done
7172

7273
# Environment variable overrides
7374
[[ -n "${CFORMAT_FIX:-}" ]] && UPDATE_MODE=true
7475
[[ -n "${CFORMAT_ALL:-}" ]] && ENUM_ALL=true
75-
[[ -n "${CFORMAT_BIN:-}" ]] && CLANG_FORMAT="$CLANG_FORMAT_BIN"
76+
[[ -n "${CFORMAT_BIN:-}" ]] && CLANG_FORMAT="$CFORMAT_BIN"
7677

7778
if [[ "$UPDATE_MODE" == "true" ]]; then
7879
# Update mode: Format files in-place
7980
enumerate_files \
8081
| exclude_patterns \
8182
| while IFS= read -r file; do
83+
echo "Formatting $file";
8284
${CLANG_FORMAT} -i "$file"
83-
echo "Formatting $file"
8485
done
8586
else
8687
# Check mode: Compare formatted output to existing files

scripts/run-tests

Lines changed: 73 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,45 @@ class TestRunner:
248248
print(f"Warning: Failed to get riot venvs for pattern '{pattern}': {e}")
249249
return []
250250

251+
def get_all_riot_venvs(self) -> List[RiotVenv]:
252+
"""Get all available riot venvs without pattern filtering."""
253+
try:
254+
venvs = []
255+
256+
# Get all venv instances without pattern filtering
257+
for n, inst in enumerate(riotfile.venv.instances()):
258+
if not inst.name:
259+
continue
260+
261+
# Extract package information from the instance
262+
packages_info = ""
263+
if hasattr(inst, 'pkgs') and inst.pkgs:
264+
all_packages = [f"{pkg}: {version}" for pkg, version in inst.pkgs.items()]
265+
packages_info = ", ".join(all_packages) if all_packages else "standard packages"
266+
267+
# Extract command from the instance
268+
command = ""
269+
if hasattr(inst, 'cmd'):
270+
command = str(inst.cmd)
271+
elif hasattr(inst, 'command'):
272+
command = str(inst.command)
273+
274+
venvs.append(RiotVenv(
275+
number=n,
276+
hash=inst.short_hash if hasattr(inst, 'short_hash') else f"hash{n}",
277+
name=inst.name,
278+
python_version=str(inst.py._hint) if hasattr(inst, 'py') and hasattr(inst.py, '_hint') else "3.10",
279+
packages=packages_info,
280+
suite_name="",
281+
command=command
282+
))
283+
284+
return venvs
285+
286+
except Exception as e:
287+
print(f"Warning: Failed to get all riot venvs: {e}")
288+
return []
289+
251290
def start_services(self, services: Set[str]) -> bool:
252291
"""Start required Docker services."""
253292
if not services:
@@ -539,6 +578,25 @@ class TestRunner:
539578

540579
return selected_venvs
541580

581+
def get_venvs_by_hash_direct(self, venv_hashes: List[str]) -> List[RiotVenv]:
582+
"""Get specific venvs by their hashes from all available venvs.
583+
584+
This method doesn't require suite information and is used when --venv is provided directly.
585+
"""
586+
all_venvs = self.get_all_riot_venvs()
587+
venv_hashes_set = set(venv_hashes)
588+
matched_venvs = [venv for venv in all_venvs if venv.hash in venv_hashes_set]
589+
590+
if not matched_venvs:
591+
return []
592+
593+
# Print info about found venvs
594+
print(f"📌 Found {len(matched_venvs)} venv(s):")
595+
for venv in matched_venvs:
596+
print(f" • {venv.hash}: {venv.name} ({venv.display_name})")
597+
598+
return matched_venvs
599+
542600

543601
def main():
544602
parser = argparse.ArgumentParser(
@@ -612,7 +670,19 @@ Examples:
612670

613671
runner = TestRunner()
614672

615-
# Determine which files to check
673+
# Special handling for --venv: skip all file discovery and suite matching
674+
if args.venv:
675+
print("🎯 Using directly specified venvs (skipping file/suite analysis)")
676+
selected_venvs = runner.get_venvs_by_hash_direct(args.venv)
677+
if not selected_venvs:
678+
print(f"❌ No venvs found matching hashes: {', '.join(args.venv)}")
679+
return 1
680+
# When using --venv directly, skip service management
681+
print(f"⚠️ Skipping service management (run manually if needed)")
682+
success = runner.run_tests(selected_venvs, {}, riot_args=riot_args, dry_run=args.dry_run)
683+
return 0 if success else 1
684+
685+
# Normal flow: determine which files to check
616686
if args.files:
617687
# Use explicitly provided files
618688
files = set(args.files)
@@ -646,17 +716,8 @@ Examples:
646716
runner.output_suites_json(matching_suites)
647717
return 0
648718

649-
# Determine venv selection method
650-
if args.venv:
651-
# Use provided venvs (no interactive prompts)
652-
selected_venvs = runner.select_venvs_by_hash(matching_suites, args.venv)
653-
if not selected_venvs:
654-
print(f"❌ No venvs found matching hashes: {', '.join(args.venv)}")
655-
return 1
656-
print(f"📌 Selected {len(selected_venvs)} venv(s) from provided hashes")
657-
else:
658-
# Interactive venv selection
659-
selected_venvs = runner.interactive_venv_selection(matching_suites)
719+
# Interactive venv selection
720+
selected_venvs = runner.interactive_venv_selection(matching_suites)
660721

661722
# Execute tests
662723
success = runner.run_tests(selected_venvs, matching_suites, riot_args=riot_args, dry_run=args.dry_run)

tests/internal/test_forksafe.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from collections import Counter
22
import os
3+
import sys
34

45
import pytest
56

@@ -204,6 +205,37 @@ def test_lock_fork():
204205
assert exit_code == 12
205206

206207

208+
@pytest.mark.skipif(sys.version_info >= (3, 14), reason="Profiling is not supported on Python 3.14 yet")
209+
@pytest.mark.subprocess(
210+
env=dict(DD_PROFILING_ENABLED="1"),
211+
ddtrace_run=True,
212+
)
213+
def test_lock_unpatched():
214+
"""Check that a forksafe.Lock is not patched when profiling is enabled."""
215+
216+
from ddtrace.internal import forksafe
217+
from ddtrace.profiling import bootstrap
218+
from ddtrace.profiling.collector.threading import ThreadingLockCollector
219+
220+
# When Profiler is started, bootstrap.profiler is set to the Profiler
221+
# instance. We explicitly access the Profiler instance and the collector list
222+
# to verify that the forksafe.Lock is not using the same class that is
223+
# patched by ThreadingLockCollector that's running.
224+
profiler = bootstrap.profiler._profiler
225+
lock_collector = None
226+
for c in profiler._collectors:
227+
if isinstance(c, ThreadingLockCollector):
228+
lock_collector = c
229+
break
230+
231+
assert lock_collector is not None, "ThreadingLockCollector not found in profiler collectors"
232+
233+
lock = forksafe.Lock()
234+
assert (
235+
lock_collector._get_patch_target() is not lock._self_wrapped_class
236+
), "forksafe.Lock is using the same class that is patched by ThreadingLockCollector"
237+
238+
207239
def test_rlock_basic():
208240
# type: (...) -> None
209241
"""Check that a forksafe.RLock implements the correct threading.RLock interface"""

tests/internal/test_tracer_flare.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
from ddtrace.internal.flare.handler import _handle_tracer_flare
2424
from ddtrace.internal.logger import get_logger
2525
from ddtrace.internal.remoteconfig._connectors import PublisherSubscriberConnector
26+
from ddtrace.internal.utils.retry import fibonacci_backoff_with_jitter
2627
from tests.utils import remote_config_build_payload as build_payload
2728

2829

@@ -214,6 +215,7 @@ def test_json_logs(self):
214215
self.flare.clean_up_files()
215216
self.flare.revert_configs()
216217

218+
@fibonacci_backoff_with_jitter(attempts=5, initial_wait=0.1)
217219
def confirm_cleanup(self):
218220
assert not self.flare.flare_dir.exists(), f"The directory {self.flare.flare_dir} still exists"
219221
# Only check for file handler cleanup if prepare() was called

0 commit comments

Comments
 (0)