Skip to content

Commit

Permalink
Fix class BoundedAttributes to have RLock rather than Lock (open-tele…
Browse files Browse the repository at this point in the history
…metry#3859)

* Fix class BoundedAttributes to have RLock rather than Lock

Co-authored-by: Christoph Heer <[email protected]>

* Add a testcase for a commit titled "Fix class BoundedAttributes to have RLock rather than Lock"

* Move comments of tests/attributes/test_attributes.py::TestBoundedAttribute.test_locking to its docstring

* Add issue reference at the end of the docstring's first line

* Add changelog

* Modify the testcase to set a fixed value under BoundedAttributes' lock and assert accordingly

This testcase passes only if BoundedAttributes use RLock, not Lock

---------

Co-authored-by: Christoph Heer <[email protected]>
Co-authored-by: Diego Hurtado <[email protected]>
  • Loading branch information
3 people authored May 23, 2024
1 parent a156bf1 commit 8b80a28
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

- Fix class BoundedAttributes to have RLock rather than Lock
([#3859](https://github.com/open-telemetry/opentelemetry-python/pull/3859))
- Remove thread lock by loading RuntimeContext explicitly.
([#3763](https://github.com/open-telemetry/opentelemetry-python/pull/3763))
- Update proto version to v1.2.0
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-api/src/opentelemetry/attributes/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ def __init__(
self.max_value_len = max_value_len
# OrderedDict is not used until the maxlen is reached for efficiency.
self._dict = {} # type: dict | OrderedDict
self._lock = threading.Lock() # type: threading.Lock
self._lock = threading.RLock() # type: threading.RLock
if attributes:
for key, value in attributes.items():
self[key] = value
Expand Down
14 changes: 14 additions & 0 deletions opentelemetry-api/tests/attributes/test_attributes.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,3 +180,17 @@ def test_immutable(self):
bdict = BoundedAttributes()
with self.assertRaises(TypeError):
bdict["should-not-work"] = "dict immutable"

def test_locking(self):
"""Supporting test case for a commit titled: Fix class BoundedAttributes to have RLock rather than Lock. See #3858.
The change was introduced because __iter__ of the class BoundedAttributes holds lock, and we observed some deadlock symptoms
in the codebase. This test case is to verify that the fix works as expected.
"""
bdict = BoundedAttributes(immutable=False)

with bdict._lock:
for num in range(100):
bdict[str(num)] = num

for num in range(100):
self.assertEqual(bdict[str(num)], num)

0 comments on commit 8b80a28

Please sign in to comment.