Skip to content

Commit 8938c15

Browse files
committed
Reduce timeout from _transaction_lock.acquire() wait
1 parent 71f4501 commit 8938c15

File tree

1 file changed

+41
-8
lines changed

1 file changed

+41
-8
lines changed

src/filelock/_read_write.py

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import sqlite3
33
import threading
44
import logging
5+
import time
56
from _error import Timeout
67
from filelock._api import AcquireReturnProxy, BaseFileLock
78
from typing import Literal, Any
@@ -15,15 +16,23 @@
1516
# systems. Use even a lower value to be safe. This 2 bln milliseconds is about 23 days.
1617
_MAX_SQLITE_TIMEOUT_MS = 2_000_000_000 - 1
1718

18-
def timeout_for_sqlite(timeout: float = -1, blocking: bool = True) -> int:
19+
def timeout_for_sqlite(timeout: float, blocking: bool, already_waited: float) -> int:
1920
if blocking is False:
2021
return 0
22+
2123
if timeout == -1:
2224
return _MAX_SQLITE_TIMEOUT_MS
25+
2326
if timeout < 0:
2427
raise ValueError("timeout must be a non-negative number or -1")
2528

29+
if timeout > 0:
30+
timeout = timeout - already_waited
31+
if timeout < 0:
32+
timeout = 0
33+
2634
assert timeout >= 0
35+
2736
timeout_ms = int(timeout * 1000)
2837
if timeout_ms > _MAX_SQLITE_TIMEOUT_MS or timeout_ms < 0:
2938
_LOGGER.warning("timeout %s is too large for SQLite, using %s ms instead", timeout, _MAX_SQLITE_TIMEOUT_MS)
@@ -97,16 +106,22 @@ def __init__(
97106
def acquire_read(self, timeout: float = -1, blocking: bool = True) -> AcquireReturnProxy:
98107
"""Acquire a read lock. If a lock is already held, it must be a read lock.
99108
Upgrading from read to write is prohibited."""
109+
110+
# Attempt to re-enter already held lock.
100111
with self._internal_lock:
101112
if self._lock_level > 0:
102113
# Must already be in read mode.
103114
if self._current_mode != "read":
104-
raise RuntimeError("Cannot acquire read lock when a write lock is held (no upgrade allowed)")
115+
raise RuntimeError(
116+
f"Cannot acquire read lock on {self.lock_file} (lock id: {id(self)}): "
117+
"already holding a write lock (downgrade not allowed)"
118+
)
105119
self._lock_level += 1
106120
return AcquireReturnProxy(lock=self)
107121

108122
timeout_ms = timeout_for_sqlite(timeout, blocking)
109123

124+
start_time = time.perf_counter()
110125
# Acquire the transaction lock so that the (possibly blocking) SQLite work
111126
# happens without conflicting with other threads' transaction work.
112127
if not self._transaction_lock.acquire(blocking, timeout):
@@ -115,11 +130,16 @@ def acquire_read(self, timeout: float = -1, blocking: bool = True) -> AcquireRet
115130
# Double-check: another thread might have completed acquisition meanwhile.
116131
with self._internal_lock:
117132
if self._lock_level > 0:
118-
# Must already be in read mode.
119133
if self._current_mode != "read":
120-
raise RuntimeError("Cannot acquire read lock when a write lock is held (no upgrade allowed)")
134+
raise RuntimeError(
135+
f"Cannot acquire read lock on {self.lock_file} (lock id: {id(self)}): "
136+
"already holding a write lock (downgrade not allowed)"
137+
)
121138
self._lock_level += 1
122139
return AcquireReturnProxy(lock=self)
140+
141+
waited = time.perf_counter() - start_time
142+
timeout_ms = timeout_for_sqlite(timeout, blocking, waited)
123143

124144
self.con.execute('PRAGMA busy_timeout=?;', (timeout_ms,))
125145
self.con.execute('BEGIN TRANSACTION;')
@@ -143,25 +163,38 @@ def acquire_read(self, timeout: float = -1, blocking: bool = True) -> AcquireRet
143163
def acquire_write(self, timeout: float = -1, blocking: bool = True) -> AcquireReturnProxy:
144164
"""Acquire a write lock. If a lock is already held, it must be a write lock.
145165
Upgrading from read to write is prohibited."""
166+
167+
# Attempt to re-enter already held lock.
146168
with self._internal_lock:
147169
if self._lock_level > 0:
148170
if self._current_mode != "write":
149-
raise RuntimeError("Cannot acquire write lock: already holding a read lock (no upgrade allowed)")
171+
raise RuntimeError(
172+
f"Cannot acquire write lock on {self.lock_file} (lock id: {id(self)}): "
173+
"already holding a read lock (upgrade not allowed)"
174+
)
150175
self._lock_level += 1
151176
return AcquireReturnProxy(lock=self)
152177

153-
timeout_ms = timeout_for_sqlite(timeout, blocking)
178+
start_time = time.perf_counter()
179+
# Acquire the transaction lock so that the (possibly blocking) SQLite work
180+
# happens without conflicting with other threads' transaction work.
154181
if not self._transaction_lock.acquire(blocking, timeout):
155182
raise Timeout(self.lock_file)
156183
try:
157184
# Double-check: another thread might have completed acquisition meanwhile.
158185
with self._internal_lock:
159186
if self._lock_level > 0:
160187
if self._current_mode != "write":
161-
raise RuntimeError("Cannot acquire write lock: already holding a read lock (no upgrade allowed)")
188+
raise RuntimeError(
189+
f"Cannot acquire write lock on {self.lock_file} (lock id: {id(self)}): "
190+
"already holding a read lock (upgrade not allowed)"
191+
)
162192
self._lock_level += 1
163193
return AcquireReturnProxy(lock=self)
164194

195+
waited = time.perf_counter() - start_time
196+
timeout_ms = timeout_for_sqlite(timeout, blocking, waited)
197+
165198
self.con.execute('PRAGMA busy_timeout=?;', (timeout_ms,))
166199
self.con.execute('BEGIN EXCLUSIVE TRANSACTION;')
167200

@@ -183,7 +216,7 @@ def release(self, force: bool = False) -> None:
183216
if self._lock_level == 0:
184217
if force:
185218
return
186-
raise RuntimeError("Cannot release a lock that is not held")
219+
raise RuntimeError(f"Cannot release a lock on {self.lock_file} (lock id: {id(self)}) that is not held")
187220
if force:
188221
self._lock_level = 0
189222
else:

0 commit comments

Comments
 (0)