From 591e91f4752d1cab70027ac7ab07166ab05adec2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Suliga?= <1270737+suligap@users.noreply.github.com> Date: Tue, 3 Dec 2024 15:53:16 +0100 Subject: [PATCH 1/2] Drop incorrect use of reentrant locks (#1076) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes a correctness bug introduced in 0014e9776350a252930671ed170edee464f9b428 resulting in lost updates during some scenarios. The code being locked is not reentrant safe. It's preferable to deadlock in these situations instead of silently loosing updates for example. Signed-off-by: Przemysław Suliga --- prometheus_client/metrics.py | 8 ++++---- prometheus_client/registry.py | 4 ++-- prometheus_client/values.py | 6 +++--- tests/test_core.py | 10 +--------- 4 files changed, 10 insertions(+), 18 deletions(-) diff --git a/prometheus_client/metrics.py b/prometheus_client/metrics.py index 3bda92c4..af512115 100644 --- a/prometheus_client/metrics.py +++ b/prometheus_client/metrics.py @@ -1,5 +1,5 @@ import os -from threading import RLock +from threading import Lock import time import types from typing import ( @@ -144,7 +144,7 @@ def __init__(self: T, if self._is_parent(): # Prepare the fields needed for child metrics. - self._lock = RLock() + self._lock = Lock() self._metrics: Dict[Sequence[str], T] = {} if self._is_observable(): @@ -697,7 +697,7 @@ class Info(MetricWrapperBase): def _metric_init(self): self._labelname_set = set(self._labelnames) - self._lock = RLock() + self._lock = Lock() self._value = {} def info(self, val: Dict[str, str]) -> None: @@ -759,7 +759,7 @@ def __init__(self, def _metric_init(self) -> None: self._value = 0 - self._lock = RLock() + self._lock = Lock() def state(self, state: str) -> None: """Set enum metric state.""" diff --git a/prometheus_client/registry.py b/prometheus_client/registry.py index 4326b39a..694e4bd8 100644 --- a/prometheus_client/registry.py +++ b/prometheus_client/registry.py @@ -1,6 +1,6 @@ from abc import ABC, abstractmethod import copy -from threading import RLock +from threading import Lock from typing import Dict, Iterable, List, Optional from .metrics_core import Metric @@ -30,7 +30,7 @@ def __init__(self, auto_describe: bool = False, target_info: Optional[Dict[str, self._collector_to_names: Dict[Collector, List[str]] = {} self._names_to_collectors: Dict[str, Collector] = {} self._auto_describe = auto_describe - self._lock = RLock() + self._lock = Lock() self._target_info: Optional[Dict[str, str]] = {} self.set_target_info(target_info) diff --git a/prometheus_client/values.py b/prometheus_client/values.py index 05331f82..6ff85e3b 100644 --- a/prometheus_client/values.py +++ b/prometheus_client/values.py @@ -1,5 +1,5 @@ import os -from threading import RLock +from threading import Lock import warnings from .mmap_dict import mmap_key, MmapedDict @@ -13,7 +13,7 @@ class MutexValue: def __init__(self, typ, metric_name, name, labelnames, labelvalues, help_text, **kwargs): self._value = 0.0 self._exemplar = None - self._lock = RLock() + self._lock = Lock() def inc(self, amount): with self._lock: @@ -50,7 +50,7 @@ def MultiProcessValue(process_identifier=os.getpid): # Use a single global lock when in multi-processing mode # as we presume this means there is no threading going on. # This avoids the need to also have mutexes in __MmapDict. - lock = RLock() + lock = Lock() class MmapedValue: """A float protected by a mutex backed by a per-process mmaped file.""" diff --git a/tests/test_core.py b/tests/test_core.py index f80fb882..8a54a02d 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -16,14 +16,6 @@ from prometheus_client.metrics import _get_use_created -def is_locked(lock): - "Tries to obtain a lock, returns True on success, False on failure." - locked = lock.acquire(blocking=False) - if locked: - lock.release() - return not locked - - def assert_not_observable(fn, *args, **kwargs): """ Assert that a function call falls with a ValueError exception containing @@ -971,7 +963,7 @@ def test_restricted_registry_does_not_yield_while_locked(self): m = Metric('target', 'Target metadata', 'info') m.samples = [Sample('target_info', {'foo': 'bar'}, 1)] for _ in registry.restricted_registry(['target_info', 's_sum']).collect(): - self.assertFalse(is_locked(registry._lock)) + self.assertFalse(registry._lock.locked()) if __name__ == '__main__': From b3f61b3ccf237424f03cc5aa3ac6ecc81c82455f Mon Sep 17 00:00:00 2001 From: Chris Marchbanks Date: Tue, 3 Dec 2024 07:55:50 -0700 Subject: [PATCH 2/2] Release 0.21.1 Signed-off-by: Chris Marchbanks --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 438f643a..4a282c49 100644 --- a/setup.py +++ b/setup.py @@ -8,7 +8,7 @@ setup( name="prometheus_client", - version="0.21.0", + version="0.21.1", author="Brian Brazil", author_email="brian.brazil@robustperception.io", description="Python client for the Prometheus monitoring system.",