Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit fa51a75

Browse filesBrowse files
suligapsuligap
committed
Add deadlock prevention
Detects and prevents deadlocks during the library misuse, eg. by injecting code into the critical sections that itself might want to obtain the relevant lock. A follow up to prometheus#1076. Signed-off-by: Przemysław Suliga <mail@suligap.net>
1 parent ef95c4b commit fa51a75
Copy full SHA for fa51a75

File tree

Expand file treeCollapse file tree

6 files changed

+81
-13
lines changed
Filter options
Expand file treeCollapse file tree

6 files changed

+81
-13
lines changed

‎prometheus_client/errors.py

Copy file name to clipboard
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
2+
class PrometheusClientRuntimeError(RuntimeError):
3+
pass

‎prometheus_client/metrics.py

Copy file name to clipboardExpand all lines: prometheus_client/metrics.py
+4-5Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import os
2-
from threading import Lock
32
import time
43
import types
54
from typing import (
@@ -13,7 +12,7 @@
1312
from .metrics_core import Metric
1413
from .registry import Collector, CollectorRegistry, REGISTRY
1514
from .samples import Exemplar, Sample
16-
from .utils import floatToGoString, INF
15+
from .utils import floatToGoString, INF, WarnLock
1716
from .validation import (
1817
_validate_exemplar, _validate_labelnames, _validate_metric_name,
1918
)
@@ -120,7 +119,7 @@ def __init__(self: T,
120119

121120
if self._is_parent():
122121
# Prepare the fields needed for child metrics.
123-
self._lock = Lock()
122+
self._lock = WarnLock()
124123
self._metrics: Dict[Sequence[str], T] = {}
125124

126125
if self._is_observable():
@@ -673,7 +672,7 @@ class Info(MetricWrapperBase):
673672

674673
def _metric_init(self):
675674
self._labelname_set = set(self._labelnames)
676-
self._lock = Lock()
675+
self._lock = WarnLock()
677676
self._value = {}
678677

679678
def info(self, val: Dict[str, str]) -> None:
@@ -735,7 +734,7 @@ def __init__(self,
735734

736735
def _metric_init(self) -> None:
737736
self._value = 0
738-
self._lock = Lock()
737+
self._lock = WarnLock()
739738

740739
def state(self, state: str) -> None:
741740
"""Set enum metric state."""

‎prometheus_client/registry.py

Copy file name to clipboardExpand all lines: prometheus_client/registry.py
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
from abc import ABC, abstractmethod
22
import copy
3-
from threading import Lock
43
from typing import Dict, Iterable, List, Optional
54

65
from .metrics_core import Metric
6+
from .utils import WarnLock
77

88

99
# Ideally this would be a Protocol, but Protocols are only available in Python >= 3.8.
@@ -30,7 +30,7 @@ def __init__(self, auto_describe: bool = False, target_info: Optional[Dict[str,
3030
self._collector_to_names: Dict[Collector, List[str]] = {}
3131
self._names_to_collectors: Dict[str, Collector] = {}
3232
self._auto_describe = auto_describe
33-
self._lock = Lock()
33+
self._lock = WarnLock()
3434
self._target_info: Optional[Dict[str, str]] = {}
3535
self.set_target_info(target_info)
3636

‎prometheus_client/utils.py

Copy file name to clipboardExpand all lines: prometheus_client/utils.py
+40Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
import math
2+
from threading import Lock, RLock
3+
4+
from .errors import PrometheusClientRuntimeError
25

36
INF = float("inf")
47
MINUS_INF = float("-inf")
@@ -22,3 +25,40 @@ def floatToGoString(d):
2225
mantissa = f'{s[0]}.{s[1:dot]}{s[dot + 1:]}'.rstrip('0.')
2326
return f'{mantissa}e+0{dot - 1}'
2427
return s
28+
29+
30+
class WarnLock:
31+
"""A wrapper around RLock and Lock that prevents deadlocks.
32+
33+
Raises a RuntimeError when it detects attempts to re-enter the critical
34+
section from a single thread. Intended to be used as a context manager.
35+
"""
36+
error_msg = (
37+
'Attempt to enter a non reentrant context from a single thread.'
38+
' It is possible that the client code is trying to register or update'
39+
' metrics from within metric registration code or from a signal handler'
40+
' while metrics are being registered or updated.'
41+
' This is unsafe and cannot be allowed. It would result in a deadlock'
42+
' if this exception was not raised.'
43+
)
44+
45+
def __init__(self):
46+
self._rlock = RLock()
47+
self._lock = Lock()
48+
49+
def __enter__(self):
50+
self._rlock.acquire()
51+
if not self._lock.acquire(blocking=False):
52+
self._rlock.release()
53+
raise PrometheusClientRuntimeError(self.error_msg)
54+
55+
def __exit__(self, exc_type, exc_value, traceback):
56+
self._lock.release()
57+
self._rlock.release()
58+
59+
def _locked(self):
60+
# For use in tests.
61+
if self._rlock.acquire(blocking=False):
62+
self._rlock.release()
63+
return False
64+
return True

‎prometheus_client/values.py

Copy file name to clipboardExpand all lines: prometheus_client/values.py
+4-5Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import os
2-
from threading import Lock
32
import warnings
43

54
from .mmap_dict import mmap_key, MmapedDict
5+
from .utils import WarnLock
66

77

88
class MutexValue:
@@ -13,7 +13,7 @@ class MutexValue:
1313
def __init__(self, typ, metric_name, name, labelnames, labelvalues, help_text, **kwargs):
1414
self._value = 0.0
1515
self._exemplar = None
16-
self._lock = Lock()
16+
self._lock = WarnLock()
1717

1818
def inc(self, amount):
1919
with self._lock:
@@ -47,10 +47,9 @@ def MultiProcessValue(process_identifier=os.getpid):
4747
files = {}
4848
values = []
4949
pid = {'value': process_identifier()}
50-
# Use a single global lock when in multi-processing mode
51-
# as we presume this means there is no threading going on.
50+
# Use a single global lock when in multi-processing mode.
5251
# This avoids the need to also have mutexes in __MmapDict.
53-
lock = Lock()
52+
lock = WarnLock()
5453

5554
class MmapedValue:
5655
"""A float protected by a mutex backed by a per-process mmaped file."""

‎tests/test_core.py

Copy file name to clipboardExpand all lines: tests/test_core.py
+28-1Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
StateSetMetricFamily, Summary, SummaryMetricFamily, UntypedMetricFamily,
1414
)
1515
from prometheus_client.decorator import getargspec
16+
from prometheus_client.errors import PrometheusClientRuntimeError
1617
from prometheus_client.metrics import _get_use_created
1718
from prometheus_client.validation import (
1819
disable_legacy_validation, enable_legacy_validation,
@@ -134,6 +135,19 @@ def test_exemplar_too_long(self):
134135
'y123456': '7+15 characters',
135136
})
136137

138+
def test_single_thread_deadlock_detection(self):
139+
counter = self.counter
140+
141+
class Tracked(float):
142+
def __radd__(self, other):
143+
counter.inc(10)
144+
return self + other
145+
146+
expected_msg = 'Attempt to enter a non reentrant context from a single thread.'
147+
self.assertRaisesRegex(
148+
PrometheusClientRuntimeError, expected_msg, counter.inc, Tracked(100)
149+
)
150+
137151

138152
class TestDisableCreated(unittest.TestCase):
139153
def setUp(self):
@@ -1004,7 +1018,20 @@ def test_restricted_registry_does_not_yield_while_locked(self):
10041018
m = Metric('target', 'Target metadata', 'info')
10051019
m.samples = [Sample('target_info', {'foo': 'bar'}, 1)]
10061020
for _ in registry.restricted_registry(['target_info', 's_sum']).collect():
1007-
self.assertFalse(registry._lock.locked())
1021+
self.assertFalse(registry._lock._locked())
1022+
1023+
def test_registry_deadlock_detection(self):
1024+
registry = CollectorRegistry(auto_describe=True)
1025+
1026+
class RecursiveCollector:
1027+
def collect(self):
1028+
Counter('x', 'help', registry=registry)
1029+
return [CounterMetricFamily('c_total', 'help', value=1)]
1030+
1031+
expected_msg = 'Attempt to enter a non reentrant context from a single thread.'
1032+
self.assertRaisesRegex(
1033+
PrometheusClientRuntimeError, expected_msg, registry.register, RecursiveCollector()
1034+
)
10081035

10091036

10101037
if __name__ == '__main__':

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.