From 19561ae6535e3450c5e056cea63fe62f879af188 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Przemys=C5=82aw=20Suliga?= <mail@suligap.net>
Date: Sun, 1 Dec 2024 14:56:17 +0100
Subject: [PATCH] Drop incorrect use of reentrant locks
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 <mail@suligap.net>
---
 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 cceaafda..03e1e66a 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 056d8e58..c374bf20 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
@@ -986,7 +978,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__':