From 8b80a28e825b102417eceb429f64d5ce52f3c2e7 Mon Sep 17 00:00:00 2001 From: hyoinandout <68385607+hyoinandout@users.noreply.github.com> Date: Fri, 24 May 2024 06:34:56 +0900 Subject: [PATCH] Fix class BoundedAttributes to have RLock rather than Lock (#3859) * Fix class BoundedAttributes to have RLock rather than Lock Co-authored-by: Christoph Heer * 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 Co-authored-by: Diego Hurtado --- CHANGELOG.md | 2 ++ .../src/opentelemetry/attributes/__init__.py | 2 +- .../tests/attributes/test_attributes.py | 14 ++++++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b25747004a8..8c2360b640c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/opentelemetry-api/src/opentelemetry/attributes/__init__.py b/opentelemetry-api/src/opentelemetry/attributes/__init__.py index a3c3c31197b..72ee38abd79 100644 --- a/opentelemetry-api/src/opentelemetry/attributes/__init__.py +++ b/opentelemetry-api/src/opentelemetry/attributes/__init__.py @@ -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 diff --git a/opentelemetry-api/tests/attributes/test_attributes.py b/opentelemetry-api/tests/attributes/test_attributes.py index ad2f741fb1f..61388de722b 100644 --- a/opentelemetry-api/tests/attributes/test_attributes.py +++ b/opentelemetry-api/tests/attributes/test_attributes.py @@ -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)