Skip to content

Commit 5d9da34

Browse files
authored
fix(profiling): ensure RLock uses original allocate_lock (backport #4131) (#4135)
This is an automatic backport of pull request #4131 done by [Mergify](https://mergify.com). --- <details> <summary>Mergify commands and options</summary> <br /> More conditions and actions can be found in the [documentation](https://docs.mergify.com/). You can also trigger Mergify actions by commenting on this pull request: - `@Mergifyio refresh` will re-evaluate the rules - `@Mergifyio rebase` will rebase this PR on its base branch - `@Mergifyio update` will merge the base branch into this PR - `@Mergifyio backport <destination>` will backport this PR on `<destination>` branch Additionally, on Mergify [dashboard](https://dashboard.mergify.com/) you can: - look at your merge queues - generate the Mergify configuration with the config editor. Finally, you can contact us on https://mergify.com </details>
1 parent 3e058eb commit 5d9da34

File tree

4 files changed

+58
-1
lines changed

4 files changed

+58
-1
lines changed

ddtrace/internal/nogevent.py

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,43 @@ def is_module_patched(module):
3737
thread_get_ident = get_original(six.moves._thread.__name__, "get_ident")
3838
Thread = get_original("threading", "Thread")
3939
Lock = get_original("threading", "Lock")
40-
RLock = get_original("threading", "RLock")
40+
41+
if six.PY2 and is_module_patched("threading"):
42+
_allocate_lock = get_original("threading", "_allocate_lock")
43+
_threading_RLock = get_original("threading", "_RLock")
44+
_threading_Verbose = get_original("threading", "_Verbose")
45+
46+
class _RLock(_threading_RLock):
47+
"""Patched RLock to ensure threading._allocate_lock is called rather than
48+
gevent.threading._allocate_lock if patching has occurred. This is not
49+
necessary in Python 3 where the RLock function uses the _CRLock so is
50+
unaffected by gevent patching.
51+
"""
52+
53+
def __init__(self, verbose=None):
54+
# We want to avoid calling the RLock init as it will allocate a gevent lock
55+
# That means we have to reproduce the code from threading._RLock.__init__ here
56+
# https://github.com/python/cpython/blob/8d21aa21f2cbc6d50aab3f420bb23be1d081dac4/Lib/threading.py#L132-L136
57+
_threading_Verbose.__init__(self, verbose)
58+
self.__block = _allocate_lock()
59+
self.__owner = None
60+
self.__count = 0
61+
62+
def RLock(*args, **kwargs):
63+
return _RLock(*args, **kwargs)
64+
65+
66+
else:
67+
# We do not patch RLock in Python 3 however for < 3.7 the C implementation of
68+
# RLock might not be available as the _thread module is optional. In that
69+
# case, the Python implementation will be used. This means there is still
70+
# the possibility that RLock in Python 3 will cause problems for gevent with
71+
# ddtrace profiling enabled though it remains an open question when that
72+
# would be the case for the supported platforms.
73+
# https://github.com/python/cpython/blob/c19983125a42a4b4958b11a26ab5e03752c956fc/Lib/threading.py#L38-L41
74+
# https://github.com/python/cpython/blob/c19983125a42a4b4958b11a26ab5e03752c956fc/Doc/library/_thread.rst#L26-L27
75+
RLock = get_original("threading", "RLock")
76+
4177

4278
is_threading_patched = is_module_patched("threading")
4379

docs/spelling_wordlist.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ runnable
149149
runtime
150150
runtimes
151151
runtime-id
152+
RLock
152153
sanic
153154
screenshots
154155
serializable
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
profiling: internal use of RLock needs to ensure original threading locks are used rather than gevent threading lock. Because of an indirection in the initialization of the original RLock, we end up getting an underlying gevent lock. We work around this behavior with gevent by creating a patched RLock for use internally.

tests/profiling/test_nogevent.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import os
2+
3+
import pytest
4+
import six
5+
6+
from ddtrace.internal import nogevent
7+
8+
9+
TESTING_GEVENT = os.getenv("DD_PROFILE_TEST_GEVENT", False)
10+
11+
12+
@pytest.mark.skipif(not TESTING_GEVENT or six.PY3, reason="Not testing gevent or testing on Python 3")
13+
def test_nogevent_rlock():
14+
import gevent
15+
16+
assert not isinstance(nogevent.RLock()._RLock__block, gevent.thread.LockType)

0 commit comments

Comments
 (0)