Validate RLock acquisition arguments before reentrant success (#9035)

Extract the Python 3.14-compatible timeout validation order from #8954. Share validation with Lock and run it before increasing the RLock recursion count. Keep canonical general-lock coverage and add only owned-lock validation regressions to the existing thread snippet.

Assisted-by: OpenAI Codex:model identifier unavailable
This commit is contained in:
youknowdot authored and GitHub committed 2026-10-10 20:12:12 +09:00
1 parent bc5b9abd75
commit 64061cd38a
2 files changed
+40 -19

No files matched your search

+22 -19
View File
@@ -104,28 +104,29 @@ pub(crate) mod _thread {
}
}
fn validate_acquire_args(args: AcquireArgs, vm: &VirtualMachine) -> PyResult<(bool, f64)> {
let timeout = args.timeout.to_secs_f64();
if !args.blocking && timeout != -1.0 {
return Err(vm.new_value_error("can't specify a timeout for a non-blocking call"));
}
if args.blocking && timeout < 0.0 && timeout != -1.0 {
return Err(vm.new_value_error("timeout value must be a non-negative number"));
}
if timeout > TIMEOUT_MAX {
return Err(vm.new_overflow_error("timeout value is too large"));
}
Ok((args.blocking, timeout))
}
macro_rules! acquire_lock_impl {
($mu:expr, $args:expr, $vm:expr) => {{
let (mu, args, vm) = ($mu, $args, $vm);
let timeout = args.timeout.to_secs_f64();
match args.blocking {
($mu:expr, $blocking:expr, $timeout:expr, $vm:expr) => {{
let (mu, blocking, timeout, vm) = ($mu, $blocking, $timeout, $vm);
match blocking {
true if timeout == -1.0 => {
vm.allow_threads(|| mu.lock());
Ok(true)
}
true if timeout < 0.0 => {
Err(vm.new_value_error("timeout value must be a non-negative number"))
}
true => {
if timeout > TIMEOUT_MAX {
return Err(vm.new_overflow_error("timeout value is too large"));
}
Ok(vm.allow_threads(|| mu.try_lock_for(Duration::from_secs_f64(timeout))))
}
false if timeout != -1.0 => {
Err(vm.new_value_error("can't specify a timeout for a non-blocking call"))
}
true => Ok(vm.allow_threads(|| mu.try_lock_for(Duration::from_secs_f64(timeout)))),
false => Ok(mu.try_lock()),
}
}};
@@ -166,7 +167,8 @@ pub(crate) mod _thread {
#[pymethod]
#[pymethod(name = "acquire_lock")]
fn acquire(&self, args: AcquireArgs, vm: &VirtualMachine) -> PyResult<bool> {
acquire_lock_impl!(&self.mu, args, vm)
let (blocking, timeout) = validate_acquire_args(args, vm)?;
acquire_lock_impl!(&self.mu, blocking, timeout, vm)
}
#[pymethod]
@@ -252,6 +254,7 @@ pub(crate) mod _thread {
#[pymethod]
#[pymethod(name = "acquire_lock")]
fn acquire(&self, args: AcquireArgs, vm: &VirtualMachine) -> PyResult<bool> {
let (blocking, timeout) = validate_acquire_args(args, vm)?;
if self.mu.is_owned_by_current_thread() {
// Re-entrant acquisition: just increment our count.
// parking_lot stays at 1 level; we track recursion ourselves.
@@ -259,7 +262,7 @@ pub(crate) mod _thread {
.fetch_add(1, core::sync::atomic::Ordering::Relaxed);
return Ok(true);
}
let result = acquire_lock_impl!(&self.mu, args, vm)?;
let result = acquire_lock_impl!(&self.mu, blocking, timeout, vm)?;
if result {
self.count.store(1, core::sync::atomic::Ordering::Relaxed);
}
+18
View File
@@ -1,3 +1,21 @@
import _thread
assert _thread.TIMEOUT_MAX in [9223372036.0, 4294967.0]
from testutils import assert_raises
# Reentrant calls must validate arguments before incrementing the count.
for error, kwargs in (
(ValueError, {"blocking": False, "timeout": 1}),
(ValueError, {"timeout": -2}),
(OverflowError, {"timeout": _thread.TIMEOUT_MAX + 1}),
):
lock = _thread.RLock()
assert lock.acquire()
try:
with assert_raises(error):
lock.acquire(**kwargs)
assert lock._recursion_count() == 1
finally:
while lock._is_owned():
lock.release()