Skip to content

Commit 8ea4d38

Browse files
vlad-scherbichbrettlangdon
authored andcommitted
chore(profiling): fix 'test_semaphore_and_bounded_semaphore_collectors_coexist' test (#15536)
## Description Fix a segfault in the test_semaphore_and_bounded_semaphore_collectors_coexist test that occurred on Python 3.12. The test was missing the required ddup.config() and ddup.start() initialization before using lock collectors. This test was introduced in #15375 (unsure how it passed there...), but it should be working now. ## What Happened The test used `ThreadingSemaphoreCollector` and `ThreadingBoundedSemaphoreCollector` without initializing the ddup module first. When the profiler tried to push sample data via handle.push_acquire(), it crashed with a segfault because the C extension's internal state (memory buffers, output file handles) was uninitialized. ## Changes * Added ddup initialization to test_semaphore_and_bounded_semaphore_collectors_coexist (the fix itself) * Extracted init_ddup() helper into new tests/profiling/collector/test_utils.py module (drive-by) * Updated 3 tests to use the shared helper, reducing code duplication (drive-by) ## Testing * CI
1 parent 785596f commit 8ea4d38

File tree

2 files changed

+61
-51
lines changed

2 files changed

+61
-51
lines changed

tests/profiling/collector/test_threading.py

Lines changed: 40 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
ThreadingLockCollector, ThreadingRLockCollector, ThreadingSemaphoreCollector, ThreadingBoundedSemaphoreCollector
5656
]
5757

58+
5859
# Module-level globals for testing global lock profiling
5960
_test_global_lock: LockTypeInst
6061

@@ -422,18 +423,10 @@ def test_assertion_error_raised_with_enable_asserts():
422423
import mock
423424
import pytest
424425

425-
from ddtrace.internal.datadog.profiling import ddup
426426
from ddtrace.profiling.collector.threading import ThreadingLockCollector
427+
from tests.profiling.collector.test_utils import init_ddup
427428

428-
# Initialize ddup (required before using collectors)
429-
assert ddup.is_available, "ddup is not available"
430-
ddup.config(
431-
env="test",
432-
service="test_asserts",
433-
version="1.0",
434-
output_filename="/tmp/test_asserts",
435-
)
436-
ddup.start()
429+
init_ddup("test_asserts")
437430

438431
with ThreadingLockCollector(capture_pct=100):
439432
lock = threading.Lock()
@@ -456,18 +449,10 @@ def test_all_exceptions_suppressed_by_default() -> None:
456449

457450
import mock
458451

459-
from ddtrace.internal.datadog.profiling import ddup
460452
from ddtrace.profiling.collector.threading import ThreadingLockCollector
453+
from tests.profiling.collector.test_utils import init_ddup
461454

462-
# Initialize ddup (required before using collectors)
463-
assert ddup.is_available, "ddup is not available"
464-
ddup.config(
465-
env="test",
466-
service="test_exceptions",
467-
version="1.0",
468-
output_filename="/tmp/test_exceptions",
469-
)
470-
ddup.start()
455+
init_ddup("test_exceptions")
471456

472457
with ThreadingLockCollector(capture_pct=100):
473458
lock = threading.Lock()
@@ -488,6 +473,41 @@ def test_all_exceptions_suppressed_by_default() -> None:
488473
lock.release()
489474

490475

476+
def test_semaphore_and_bounded_semaphore_collectors_coexist() -> None:
477+
"""Test that Semaphore and BoundedSemaphore collectors can run simultaneously.
478+
479+
Tests proper patching where inheritance is involved if both parent and child classes are patched,
480+
e.g. when BoundedSemaphore's c-tor calls Semaphore c-tor.
481+
We expect that the call to Semaphore c-tor goes to the unpatched version, and NOT our patched version.
482+
"""
483+
from tests.profiling.collector.test_utils import init_ddup
484+
485+
init_ddup("test_semaphore_and_bounded_semaphore_collectors_coexist")
486+
487+
# Both collectors active at the same time - this triggers the inheritance case
488+
with ThreadingSemaphoreCollector(capture_pct=100), ThreadingBoundedSemaphoreCollector(capture_pct=100):
489+
sem = threading.Semaphore(2)
490+
sem.acquire()
491+
sem.release()
492+
493+
bsem = threading.BoundedSemaphore(3)
494+
bsem.acquire()
495+
bsem.release()
496+
497+
# If inheritance delegation failed, these attributes will be missing.
498+
wrapped_bsem = bsem.__wrapped__
499+
assert hasattr(wrapped_bsem, "_cond"), "BoundedSemaphore._cond not initialized (inheritance bug)"
500+
assert hasattr(wrapped_bsem, "_value"), "BoundedSemaphore._value not initialized (inheritance bug)"
501+
assert hasattr(wrapped_bsem, "_initial_value"), "BoundedSemaphore._initial_value not initialized"
502+
503+
# Verify BoundedSemaphore behavior is preserved (i.e. it raises on over-release)
504+
bsem2 = threading.BoundedSemaphore(1)
505+
bsem2.acquire()
506+
bsem2.release()
507+
with pytest.raises(ValueError, match="Semaphore released too many times"):
508+
bsem2.release()
509+
510+
491511
class BaseThreadingLockCollectorTest:
492512
# These should be implemented by child classes
493513
@property
@@ -1595,34 +1615,3 @@ def test_bounded_behavior_preserved(self) -> None:
15951615
# This proves our profiling wrapper doesn't break BoundedSemaphore's behavior
15961616
with pytest.raises(ValueError, match="Semaphore released too many times"):
15971617
sem.release()
1598-
1599-
1600-
def test_semaphore_and_bounded_semaphore_collectors_coexist() -> None:
1601-
"""Test that Semaphore and BoundedSemaphore collectors can run simultaneously.
1602-
1603-
Tests proper patching where inheritance is involved if both parent and child classes are patched,
1604-
e.g. when BoundedSemaphore's c-tor calls Semaphore c-tor.
1605-
We expect that the call to Semaphore c-tor goes to the unpatched version, and NOT our patched version.
1606-
"""
1607-
# Both collectors active at the same time - this triggers the inheritance case
1608-
with ThreadingSemaphoreCollector(capture_pct=100), ThreadingBoundedSemaphoreCollector(capture_pct=100):
1609-
sem = threading.Semaphore(2)
1610-
sem.acquire()
1611-
sem.release()
1612-
1613-
bsem = threading.BoundedSemaphore(3)
1614-
bsem.acquire()
1615-
bsem.release()
1616-
1617-
# If inheritance delegation failed, these attributes will be missing.
1618-
wrapped_bsem = bsem.__wrapped__
1619-
assert hasattr(wrapped_bsem, "_cond"), "BoundedSemaphore._cond not initialized (inheritance bug)"
1620-
assert hasattr(wrapped_bsem, "_value"), "BoundedSemaphore._value not initialized (inheritance bug)"
1621-
assert hasattr(wrapped_bsem, "_initial_value"), "BoundedSemaphore._initial_value not initialized"
1622-
1623-
# Verify BoundedSemaphore behavior is preserved (i.e. it raises on over-release)
1624-
bsem2 = threading.BoundedSemaphore(1)
1625-
bsem2.acquire()
1626-
bsem2.release()
1627-
with pytest.raises(ValueError, match="Semaphore released too many times"):
1628-
bsem2.release()
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
"""Shared utilities for profiling collector tests."""
2+
3+
from ddtrace.internal.datadog.profiling import ddup
4+
5+
6+
def init_ddup(test_name: str) -> None:
7+
"""Initialize ddup for profiling tests.
8+
9+
Must be called before using any lock collectors.
10+
11+
Args:
12+
test_name: Name of the test, used for service name and output filename.
13+
"""
14+
assert ddup.is_available, "ddup is not available"
15+
ddup.config(
16+
env="test",
17+
service=test_name,
18+
version="1.0",
19+
output_filename="/tmp/" + test_name,
20+
)
21+
ddup.start()

0 commit comments

Comments
 (0)