fix(profiling): ensure RLock uses original allocate_lock (#4131) (#4138)

## Description

#4108 replaced an internal use of the original `threading.Lock` function rather than the `gevent.threading.Lock` with the original `threading.RLock` function. However, at least in Python 2, there is an indirection with the implementation of `threading._RLock` class where it the `threading._allocate_lock` called is in fact `gevent.threading._allocate_lock`.

When the gevent patched lock is used instead, we can encounter deadlocks or even gevent exceptions like the following:

```
ddtrace.profiling.collector.stack._ThreadSpanLinks.get_active_span_from_thread_id
  File "ddtrace/profiling/_threading.pyx", line 112, in ddtrace.profiling._threading._ThreadLink.get_object
  File "/usr/local/lib/python2.7/threading.py", line 174, in acquire
    rc = self.__block.acquire(blocking)
  File "src/gevent/_semaphore.py", line 198, in gevent._semaphore.Semaphore.acquire (src/gevent/gevent._semaphore.c:4541)
  File "src/gevent/_semaphore.py", line 226, in gevent._semaphore.Semaphore.acquire (src/gevent/gevent._semaphore.c:4367)
  File "src/gevent/_semaphore.py", line 166, in gevent._semaphore.Semaphore._do_wait (src/gevent/gevent._semaphore.c:3562)
  File "/usr/local/lib/python2.7/site-packages/gevent/hub.py", line 630, in switch
    return RawGreenlet.switch(self)
gevent.hub.LoopExit: ('This operation would block forever', <Hub at 0x7fe2bda052d0 epoll pending=0 ref=0 fileno=267>)
```

The added test for the issue: https://app.circleci.com/pipelines/github/DataDog/dd-trace-py/20715/workflows/602ec8d6-d8ba-4191-ada8-0da76adbdc6b/jobs/1405224

## Checklist
- [x] Title must conform to [conventional commit](https://github.com/conventional-changelog/commitlint/tree/master/%40commitlint/config-conventional).
- [x] Add additional sections for `feat` and `fix` pull requests.
- [x] Ensure tests are passing for affected code.
- [x] [Library documentation](https://github.com/DataDog/dd-trace-py/tree/1.x/docs) and/or [Datadog's documentation site](https://github.com/DataDog/documentation/) is updated. Link to doc PR in description.

## Reviewer Checklist
- [x] Title is accurate.
- [x] Description motivates each change.
- [x] No unnecessary changes were introduced in this PR.
- [x] PR cannot be broken up into smaller PRs.
- [x] Avoid breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary.
- [x] Tests provided or description of manual testing performed is included in the code or PR.
- [x] Release note has been added for fixes and features, or else `changelog/no-changelog` label added.
- [x] All relevant GitHub issues are correctly linked.
- [ ] Backports are identified and tagged with Mergifyio.
- [ ] Add to milestone.

(cherry picked from commit b0d8765)

# Conflicts:
#	ddtrace/internal/nogevent.py

Co-authored-by: Tahir H. Butt <[email protected]>
Co-authored-by: Kyle Verhoog <[email protected]>

Author: 3 people

ddtrace/internal/nogevent.py

@@ -37,7 +37,43 @@ def is_module_patched(module):
 thread_get_ident = get_original(six.moves._thread.__name__, "get_ident")
 Thread = get_original("threading", "Thread")
 Lock = get_original("threading", "Lock")
-RLock = get_original("threading", "RLock")
+
+
+if six.PY2 and is_module_patched("threading"):
+    _allocate_lock = get_original("threading", "_allocate_lock")
+    _threading_RLock = get_original("threading", "_RLock")
+    _threading_Verbose = get_original("threading", "_Verbose")
+
+    class _RLock(_threading_RLock):
+        """Patched RLock to ensure threading._allocate_lock is called rather than
+        gevent.threading._allocate_lock if patching has occurred. This is not
+        necessary in Python 3 where the RLock function uses the _CRLock so is
+        unaffected by gevent patching.
+        """
+
+        def __init__(self, verbose=None):
+            # We want to avoid calling the RLock init as it will allocate a gevent lock
+            # That means we have to reproduce the code from threading._RLock.__init__ here
+            # https://github.com/python/cpython/blob/8d21aa21f2cbc6d50aab3f420bb23be1d081dac4/Lib/threading.py#L132-L136
+            _threading_Verbose.__init__(self, verbose)
+            self.__block = _allocate_lock()
+            self.__owner = None
+            self.__count = 0
+
+    def RLock(*args, **kwargs):
+        return _RLock(*args, **kwargs)
+
+else:
+    # We do not patch RLock in Python 3 however for < 3.7 the C implementation of
+    # RLock might not be available as the _thread module is optional.  In that
+    # case, the Python implementation will be used. This means there is still
+    # the possibility that RLock in Python 3 will cause problems for gevent with
+    # ddtrace profiling enabled though it remains an open question when that
+    # would be the case for the supported platforms.
+    # https://github.com/python/cpython/blob/c19983125a42a4b4958b11a26ab5e03752c956fc/Lib/threading.py#L38-L41
+    # https://github.com/python/cpython/blob/c19983125a42a4b4958b11a26ab5e03752c956fc/Doc/library/_thread.rst#L26-L27
+    RLock = get_original("threading", "RLock")
+
 
 is_threading_patched = is_module_patched("threading")
 

docs/spelling_wordlist.txt

@@ -133,6 +133,7 @@ rq
 runnable
 runtime
 runtime-id
+RLock
 sanic
 screenshots
 serializable

releasenotes/notes/nogevent-rlock-original-9e387eafb2be3d27.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - |
+    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

@@ -0,0 +1,16 @@
+import os
+
+import pytest
+import six
+
+from ddtrace.internal import nogevent
+
+
+TESTING_GEVENT = os.getenv("DD_PROFILE_TEST_GEVENT", False)
+
+
+@pytest.mark.skipif(not TESTING_GEVENT or six.PY3, reason="Not testing gevent or testing on Python 3")
+def test_nogevent_rlock():
+    import gevent
+
+    assert not isinstance(nogevent.RLock()._RLock__block, gevent.thread.LockType)