From 307852d3d2a2f2a0063529b965c3afbe324cbf65 Mon Sep 17 00:00:00 2001 From: Andrej730 Date: Wed, 26 Aug 2026 11:42:53 +0500 Subject: [PATCH 1/5] `str.format`: reject non-existing `!b` format conversion (#8597) --- crates/common/src/cformat.rs | 38 ++++++++----- crates/common/src/format.rs | 2 - crates/vm/src/cformat.rs | 79 ++++++++++++-------------- crates/vm/src/format.rs | 3 - extra_tests/snippets/builtin_format.py | 9 +++ 5 files changed, 71 insertions(+), 60 deletions(-) diff --git a/crates/common/src/cformat.rs b/crates/common/src/cformat.rs index db11e0a339f..f018dd32f93 100644 --- a/crates/common/src/cformat.rs +++ b/crates/common/src/cformat.rs @@ -55,6 +55,12 @@ impl fmt::Display for CFormatError { pub type CFormatConversion = super::format::FormatConversion; +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum CFormatContext { + Str, + Bytes, +} + #[derive(Debug, PartialEq, Eq, Clone, Copy)] #[repr(u8)] pub enum CNumberType { @@ -98,6 +104,7 @@ pub enum CFormatType { Float(CFloatType), Character(CCharacterType), String(CFormatConversion), + Bytes, } impl CFormatType { @@ -108,6 +115,7 @@ impl CFormatType { Self::Float(x) => x as u8 as char, Self::Character(x) => x as u8 as char, Self::String(x) => x as u8 as char, + Self::Bytes => 'b', } } } @@ -296,14 +304,14 @@ impl FromStr for CFormatSpecKeyed { return Err((CFormatErrorType::MissingModuloSign, 1)); } - Self::parse(&mut chars) + Self::parse(&mut chars, CFormatContext::Str) } } pub type ParseIter = Peekable>; impl CFormatSpecKeyed { - pub fn parse(iter: &mut ParseIter) -> Result + pub fn parse(iter: &mut ParseIter, context: CFormatContext) -> Result where I: Iterator, { @@ -313,7 +321,7 @@ impl CFormatSpecKeyed { parse_quantity(iter, isize::MAX as usize, CFormatErrorType::WidthTooBig)?; let precision = parse_precision(iter)?; consume_length(iter); - let format_type = parse_format_type(iter)?; + let format_type = parse_format_type(iter, context)?; let spec = CFormatSpec { flags, @@ -618,7 +626,10 @@ where iter.next_if(|(_, c)| matches!(c.to_char_lossy(), 'h' | 'l' | 'L')); } -fn parse_format_type(iter: &mut ParseIter) -> Result +fn parse_format_type( + iter: &mut ParseIter, + context: CFormatContext, +) -> Result where C: FormatChar, I: Iterator, @@ -646,8 +657,8 @@ where 'c' => CFormatType::Character(CCharacterType::Character), 'r' => CFormatType::String(CFormatConversion::Repr), 's' => CFormatType::String(CFormatConversion::Str), - 'b' => CFormatType::String(CFormatConversion::Bytes), 'a' => CFormatType::String(CFormatConversion::Ascii), + 'b' if context == CFormatContext::Bytes => CFormatType::Bytes, _ => return Err((CFormatErrorType::UnsupportedFormatChar(c.into()), index)), }) } @@ -784,7 +795,7 @@ impl CFormatStrOrBytes { self.parts.iter_mut() } - pub fn parse(iter: &mut ParseIter) -> Result + pub fn parse(iter: &mut ParseIter, context: CFormatContext) -> Result where S: FormatBuf, I: Iterator, @@ -808,10 +819,11 @@ impl CFormatStrOrBytes { )); } - let spec = CFormatSpecKeyed::parse(iter).map_err(|err| CFormatError { - typ: err.0, - index: err.1, - })?; + let spec = + CFormatSpecKeyed::parse(iter, context).map_err(|err| CFormatError { + typ: err.0, + index: err.1, + })?; parts.push((index, CFormatPart::Spec(spec))); if let Some(&(index, _)) = iter.peek() { @@ -848,7 +860,7 @@ pub type CFormatBytes = CFormatStrOrBytes>; impl CFormatBytes { pub fn parse_from_bytes(bytes: &[u8]) -> Result { let mut iter = bytes.iter().copied().enumerate().peekable(); - Self::parse(&mut iter) + Self::parse(&mut iter, CFormatContext::Bytes) } } @@ -859,7 +871,7 @@ impl FromStr for CFormatString { fn from_str(text: &str) -> Result { let mut iter = text.chars().enumerate().peekable(); - Self::parse(&mut iter) + Self::parse(&mut iter, CFormatContext::Str) } } @@ -868,7 +880,7 @@ pub type CFormatWtf8 = CFormatStrOrBytes; impl CFormatWtf8 { pub fn parse_from_wtf8(s: &Wtf8) -> Result { let mut iter = s.code_points().enumerate().peekable(); - Self::parse(&mut iter) + Self::parse(&mut iter, CFormatContext::Str) } } diff --git a/crates/common/src/format.rs b/crates/common/src/format.rs index 7a5e3ac0fa1..0308721975a 100644 --- a/crates/common/src/format.rs +++ b/crates/common/src/format.rs @@ -37,7 +37,6 @@ pub enum FormatConversion { Str = b's', Repr = b'r', Ascii = b'b', - Bytes = b'a', } impl FormatParse for FormatConversion { @@ -59,7 +58,6 @@ impl FormatConversion { 's' => Some(Self::Str), 'r' => Some(Self::Repr), 'a' => Some(Self::Ascii), - 'b' => Some(Self::Bytes), _ => None, } } diff --git a/crates/vm/src/cformat.rs b/crates/vm/src/cformat.rs index 7d47da39928..00147103851 100644 --- a/crates/vm/src/cformat.rs +++ b/crates/vm/src/cformat.rs @@ -32,38 +32,37 @@ fn spec_format_bytes( obj: PyObjectRef, ) -> PyResult> { match &spec.format_type { - CFormatType::String(conversion) => match conversion { - // Unlike strings, %r and %a are identical for bytes: the behaviour corresponds to - // %a for strings (not %r) - CFormatConversion::Repr | CFormatConversion::Ascii => { - let b = builtins::ascii(obj, vm)?.as_bytes().to_vec(); - Ok(b) + // Unlike strings, %r and %a are identical for bytes: the behaviour corresponds to + // %a for strings (not %r) + CFormatType::String(CFormatConversion::Repr | CFormatConversion::Ascii) => { + let b = builtins::ascii(obj, vm)?.as_bytes().to_vec(); + Ok(b) + } + // %b and %s are equivalent for bytes formatting. + // Mirrors CPython's format_obj() in bytesobject.c + CFormatType::Bytes | CFormatType::String(CFormatConversion::Str) => { + if let Some(bytes) = obj.downcast_ref::() { + return Ok(spec.format_bytes(bytes.as_bytes())); } - // format_obj - CFormatConversion::Str | CFormatConversion::Bytes => { - if let Some(bytes) = obj.downcast_ref::() { - return Ok(spec.format_bytes(bytes.as_bytes())); - } - if let Some(bytearray) = obj.downcast_ref::() { - return Ok(spec.format_bytes(&bytearray.borrow_buf())); - } - if let Some(method) = vm.get_special_method(&obj, identifier!(vm, __bytes__))? { - let bytes = method.invoke((), vm)?; - let bytes = PyBytes::try_from_borrowed_object(vm, &bytes)?; - return Ok(spec.format_bytes(bytes.as_bytes())); - } - if obj.check_buffer() { - let buffer = PyBuffer::from_object(vm, &obj, BufferFlags::FULL_RO)?; - return Ok(buffer.contiguous_or_collect(|bytes| spec.format_bytes(bytes))); - } - let msg = format!( - "%b requires a bytes-like object, or an object that \ - implements __bytes__, not '{}'", - obj.class().name() - ); - Err(vm.new_type_error(msg)) + if let Some(bytearray) = obj.downcast_ref::() { + return Ok(spec.format_bytes(&bytearray.borrow_buf())); } - }, + if let Some(method) = vm.get_special_method(&obj, identifier!(vm, __bytes__))? { + let bytes = method.invoke((), vm)?; + let bytes = PyBytes::try_from_borrowed_object(vm, &bytes)?; + return Ok(spec.format_bytes(bytes.as_bytes())); + } + if obj.check_buffer() { + let buffer = PyBuffer::from_object(vm, &obj, BufferFlags::FULL_RO)?; + return Ok(buffer.contiguous_or_collect(|bytes| spec.format_bytes(bytes))); + } + let msg = format!( + "%b requires a bytes-like object, or an object that \ + implements __bytes__, not '{}'", + obj.class().name() + ); + Err(vm.new_type_error(msg)) + } CFormatType::Number(number_type) => match number_type { CNumberType::DecimalD | CNumberType::DecimalI | CNumberType::DecimalU => { match_class!(match &obj { @@ -166,7 +165,6 @@ fn spec_format_string( vm: &VirtualMachine, spec: &CFormatSpec, obj: PyObjectRef, - idx: usize, ) -> PyResult { match &spec.format_type { CFormatType::String(conversion) => { @@ -174,16 +172,13 @@ fn spec_format_string( CFormatConversion::Ascii => builtins::ascii(obj, vm)?.as_wtf8().to_owned(), CFormatConversion::Str => obj.str(vm)?.as_wtf8().to_owned(), CFormatConversion::Repr => obj.repr(vm)?.as_wtf8().to_owned(), - CFormatConversion::Bytes => { - // idx is the position of the %, we want the position of the b - return Err(vm.new_value_error(format!( - "unsupported format character 'b' (0x62) at index {}", - idx + 1 - ))); - } }; Ok(spec.format_string(result)) } + CFormatType::Bytes => { + // 'b' is rejected at parse time in Str context, see `CFormatContext`. + unreachable!("%b cannot be parsed in a str format string") + } CFormatType::Number(number_type) => match number_type { CNumberType::DecimalD | CNumberType::DecimalI | CNumberType::DecimalU => { match_class!(match &obj { @@ -487,12 +482,12 @@ pub(crate) fn cformat_string( } // dict - for (idx, part) in format { + for (_, part) in format { match part { CFormatPart::Literal(literal) => result.push_wtf8(&literal), CFormatPart::Spec(CFormatSpecKeyed { mapping_key, spec }) => { let value = values_obj.get_item(&mapping_key.unwrap(), vm)?; - let part_result = spec_format_string(vm, &spec, value, idx)?; + let part_result = spec_format_string(vm, &spec, value)?; result.push_wtf8(&part_result); } } @@ -510,7 +505,7 @@ pub(crate) fn cformat_string( let mut value_iter = values.iter(); - for (idx, part) in format { + for (_, part) in format { match part { CFormatPart::Literal(literal) => result.push_wtf8(&literal), CFormatPart::Spec(CFormatSpecKeyed { mut spec, .. }) => { @@ -526,7 +521,7 @@ pub(crate) fn cformat_string( return Err(vm.new_type_error("not enough arguments for format string")); }; - let part_result = spec_format_string(vm, &spec, value.clone(), idx)?; + let part_result = spec_format_string(vm, &spec, value.clone())?; result.push_wtf8(&part_result); } } diff --git a/crates/vm/src/format.rs b/crates/vm/src/format.rs index 75f5c32f0c3..de2391dcde5 100644 --- a/crates/vm/src/format.rs +++ b/crates/vm/src/format.rs @@ -153,9 +153,6 @@ fn format_internal( Some(FormatConversion::Str) => argument.str(vm)?.into(), Some(FormatConversion::Repr) => argument.repr(vm)?.into(), Some(FormatConversion::Ascii) => builtins::ascii(argument, vm)?.into(), - Some(FormatConversion::Bytes) => { - vm.call_method(&argument, identifier!(vm, decode).as_str(), ())? - } None => { return Err( vm.new_value_error(format!("Unknown conversion specifier {c}")) diff --git a/extra_tests/snippets/builtin_format.py b/extra_tests/snippets/builtin_format.py index 6b7403da133..844d9e45138 100644 --- a/extra_tests/snippets/builtin_format.py +++ b/extra_tests/snippets/builtin_format.py @@ -66,6 +66,15 @@ def test_zero_padding(): else: raise AssertionError("expected ValueError for unknown conversion specifier '!x'") +# 'b' is a valid conversion specifier for %-style bytes formatting, but not for str.format(). +try: + "{0!b}".format(3) +except ValueError as error: + if str(error) != "Unknown conversion specifier b": + raise AssertionError(f"unexpected error message: {error}") from error +else: + raise AssertionError("expected ValueError for unknown conversion specifier '!b'") + assert "{:,}".format(100) == "100" assert "{:,}".format(1024) == "1,024" assert "{:_}".format(65536) == "65_536" From f9d986b7ed0d3bb22788bb18f71382ed673696c2 Mon Sep 17 00:00:00 2001 From: Joshua Megnauth <48846352+joshuamegnauth54@users.noreply.github.com> Date: Wed, 26 Aug 2026 09:34:32 -0400 Subject: [PATCH 2/5] Update test_unicodedata to 3.15 (#8598) See: #8548 --- Lib/test/test_unicodedata.py | 625 ++++++++++++++++++++++++++++++++--- 1 file changed, 584 insertions(+), 41 deletions(-) diff --git a/Lib/test/test_unicodedata.py b/Lib/test/test_unicodedata.py index 4280e8d450b..a1252fb6603 100644 --- a/Lib/test/test_unicodedata.py +++ b/Lib/test/test_unicodedata.py @@ -12,7 +12,9 @@ import sys import unicodedata import unittest +import weakref from test.support import ( + gc_collect, open_urlresource, requires_resource, script_helper, @@ -30,14 +32,33 @@ def iterallchars(): maxunicode = 0xffff if quicktest else sys.maxunicode return map(chr, range(maxunicode + 1)) + +def check_version(testfile): + hdr = testfile.readline() + return unicodedata.unidata_version in hdr + + +def download_test_data_file(filename): + TESTDATAURL = f"http://www.pythontest.net/unicode/{unicodedata.unidata_version}/{filename}" + + try: + return open_urlresource(TESTDATAURL, encoding="utf-8", check=check_version) + except PermissionError: + raise unittest.SkipTest( + f"Permission error when downloading {TESTDATAURL} " + f"into the test data directory" + ) + except (OSError, HTTPException) as exc: + raise unittest.SkipTest(f"Failed to download {TESTDATAURL}: {exc}") + + class UnicodeMethodsTest(unittest.TestCase): # update this, if the database changes - expectedchecksum = ('486bf97d506d0ccf0e463fd1f40c51029805af5a' + expectedchecksum = ('47a99fa654ef1f50e89d2e9697b7b041fccb5a05' if quicktest else - '9e43ee3929471739680c0e705482b4ae1c4122e4') + '8b2615a9fc627676cbc0b6fac0191177df97ef5f') - @unittest.expectedFailure # TODO: RUSTPYTHON; + 9e43ee3929471739680c0e705482b4ae1c4122e4 def test_method_checksum(self): h = hashlib.sha1() for char in iterallchars(): @@ -84,17 +105,9 @@ def test_method_checksum(self): self.assertEqual(result, self.expectedchecksum) -class UnicodeFunctionsTest(unittest.TestCase): - db = unicodedata - old = False - - # Update this if the database changes. Make sure to do a full rebuild - # (e.g. 'make distclean && make') to get the correct checksum. - expectedchecksum = ('1ba453ec456896f1190d849b6e9b7c2e1a4128e0' - if quicktest else - '46ca89d9fe34881d0be3a4a4b29f5aa8c019640c') +class BaseUnicodeFunctionsTest: - @unittest.expectedFailure # TODO: RUSTPYTHON; AttributeError: module 'unicodedata' has no attribute 'digit' + @unittest.skip # TODO: RUSTPYTHON; AssertionError: '7bda75e48a961a01ab328358980cefc5c0a1666d' != '68cd01e2c680b851c1fcab012efb5635' def test_function_checksum(self): db = self.db data = [] @@ -118,6 +131,7 @@ def test_function_checksum(self): result = h.hexdigest() self.assertEqual(result, self.expectedchecksum) + @unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: None != 'TANGUT IDEOGRAPH-17000' def test_name(self): name = self.db.name self.assertRaises(ValueError, name, '\0') @@ -149,12 +163,16 @@ def test_name(self): 'EGYPTIAN HIEROGLYPH-13460') self.assertEqual(name('\U000143FA', None), None if self.old else 'EGYPTIAN HIEROGLYPH-143FA') + self.assertEqual(name('\U00017000', None), None if self.old else + 'TANGUT IDEOGRAPH-17000') self.assertEqual(name('\U00018B00', None), None if self.old else 'KHITAN SMALL SCRIPT CHARACTER-18B00') self.assertEqual(name('\U00018CD5', None), None if self.old else 'KHITAN SMALL SCRIPT CHARACTER-18CD5') self.assertEqual(name('\U00018CFF', None), None if self.old else 'KHITAN SMALL SCRIPT CHARACTER-18CFF') + self.assertEqual(name('\U00018D1E', None), None if self.old else + 'TANGUT IDEOGRAPH-18D1E') self.assertEqual(name('\U0001B170', None), None if self.old else 'NUSHU CHARACTER-1B170') self.assertEqual(name('\U0001B2FB', None), None if self.old else @@ -164,8 +182,8 @@ def test_name(self): 'MIDDLE LEFT AND MIDDLE RIGHT TO LOWER CENTRE') self.assertEqual(name('\U0002A6D6'), 'CJK UNIFIED IDEOGRAPH-2A6D6') self.assertEqual(name('\U0002FA1D'), 'CJK COMPATIBILITY IDEOGRAPH-2FA1D') - self.assertEqual(name('\U000323AF', None), None if self.old else - 'CJK UNIFIED IDEOGRAPH-323AF') + self.assertEqual(name('\U00033479', None), None if self.old else + 'CJK UNIFIED IDEOGRAPH-33479') @requires_resource('cpu') def test_name_inverse_lookup(self): @@ -182,7 +200,7 @@ def test_no_names_in_pua(self): char = chr(i) self.assertRaises(ValueError, self.db.name, char) - @unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: KeyError not raised by lookup + @unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: KeyError not raised by lookup def test_lookup_nonexistant(self): # just make sure that lookup can fail for nonexistent in [ @@ -225,7 +243,7 @@ def test_digit(self): self.assertRaises(TypeError, self.db.digit, 'xx') self.assertRaises(ValueError, self.db.digit, 'x') - @unittest.skip # TODO: RUSTPYTHON; - None != 1e+20 (for 3.2.0; passes on latest) + @unittest.skip # TODO: RUSTPYTHON; None != 1e+20 (for 3.2.0; passes on latest) def test_numeric(self): self.assertEqual(self.db.numeric('A',None), None) self.assertEqual(self.db.numeric('9'), 9) @@ -306,7 +324,6 @@ def test_category(self): self.assertRaises(TypeError, self.db.category) self.assertRaises(TypeError, self.db.category, 'xx') - # NOTE: RUSTPYTHON; This test is from 3.15. See RustPython#8548 for motivation. def test_bidirectional(self): self.assertEqual(self.db.bidirectional('\uFFFE'), '' if self.old else 'BN') self.assertEqual(self.db.bidirectional(' '), 'WS') @@ -336,7 +353,16 @@ def test_bidirectional(self): self.assertRaises(TypeError, self.db.bidirectional) self.assertRaises(TypeError, self.db.bidirectional, 'xx') - @unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: 'D4CC 11B6' != '1111 1171 11B6' + def test_bidirectional_unassigned(self): + self.assertEqual(self.db.bidirectional('\u0378'), '' if self.old else 'L') + self.assertEqual(self.db.bidirectional('\u077F'), '' if self.old else 'AL') + self.assertEqual(self.db.bidirectional('\u20CF'), '' if self.old else 'ET') + self.assertEqual(self.db.bidirectional('\u0590'), '' if self.old else 'R') + self.assertEqual(self.db.bidirectional('\uFFFF'), '' if self.old else 'BN') + self.assertEqual(self.db.bidirectional('\U0001FFFE'), '' if self.old else 'BN') + self.assertEqual(self.db.bidirectional('\U00010D01'), '' if self.old else 'AL') + + @unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: ' 03A3 != '' def test_decomposition(self): self.assertEqual(self.db.decomposition('\uFFFE'),'') self.assertEqual(self.db.decomposition('\u00bc'), ' 0031 2044 0034') @@ -354,6 +380,8 @@ def test_decomposition(self): self.assertEqual(self.db.decomposition('\U0001e06d'), '' if self.old else ' 04B1') # New in 16.0.0 self.assertEqual(self.db.decomposition('\U0001CCD6'), '' if self.old else ' 0041') + # New in 17.0.0 + self.assertEqual(self.db.decomposition('\uA7F1'), '' if self.old else ' 0053') # Hangul characters self.assertEqual(self.db.decomposition('\uAC00'), '1100 1161') @@ -403,6 +431,8 @@ def test_combining(self): self.assertEqual(self.db.combining('\U00010efd'), 0 if self.old else 220) # New in 16.0.0 self.assertEqual(self.db.combining('\u0897'), 0 if self.old else 230) + # New in 17.0.0 + self.assertEqual(self.db.combining('\u1ACF'), 0 if self.old else 230) self.assertRaises(TypeError, self.db.combining) self.assertRaises(TypeError, self.db.combining, 'xx') @@ -589,6 +619,34 @@ def test_issue10254(self): b = 'C\u0338' * 20 + '\xC7' self.assertEqual(self.db.normalize('NFC', a), b) + def test_long_combining_mark_run(self): + # gh-149079: avoid quadratic canonical ordering. + payload = "a" + ("\u0300\u0327" * 32) + nfd = "a" + ("\u0327" * 32) + ("\u0300" * 32) + nfc = "\u00e0" + ("\u0327" * 32) + ("\u0300" * 31) + + self.assertEqual(self.db.normalize("NFD", payload), nfd) + self.assertEqual(self.db.normalize("NFKD", payload), nfd) + self.assertEqual(self.db.normalize("NFC", payload), nfc) + self.assertEqual(self.db.normalize("NFKC", payload), nfc) + + def test_combining_mark_run_fast_paths(self): + # gh-149079: cover short runs and already-sorted long runs. + short_payload = "a" + ("\u0300\u0327" * 9) + "\u0300" + short_nfd = "a" + ("\u0327" * 9) + ("\u0300" * 10) + short_nfc = "\u00e0" + ("\u0327" * 9) + ("\u0300" * 9) + long_sorted = "a" + ("\u0327" * 30) + ("\u0300" * 30) + long_sorted_nfc = "\u00e0" + ("\u0327" * 30) + ("\u0300" * 29) + + self.assertEqual(self.db.normalize("NFD", short_payload), short_nfd) + self.assertEqual(self.db.normalize("NFKD", short_payload), short_nfd) + self.assertEqual(self.db.normalize("NFC", short_payload), short_nfc) + self.assertEqual(self.db.normalize("NFKC", short_payload), short_nfc) + self.assertEqual(self.db.normalize("NFD", long_sorted), long_sorted) + self.assertEqual(self.db.normalize("NFKD", long_sorted), long_sorted) + self.assertEqual(self.db.normalize("NFC", long_sorted), long_sorted_nfc) + self.assertEqual(self.db.normalize("NFKC", long_sorted), long_sorted_nfc) + def test_issue29456(self): # Fix #29456 u1176_str_a = '\u1100\u1176\u11a8' @@ -641,6 +699,8 @@ def test_east_asian_width(self): # New in 16.0.0 self.assertEqual(eaw('\u2630'), 'N' if self.old else 'W') self.assertEqual(eaw('\U0001FAE9'), 'N' if self.old else 'W') + # New in 17.0.0 + self.assertEqual(eaw('\U00016FF2'), 'N' if self.old else 'W') @unittest.skip # TODO: RUSTPYTHON; AssertionError: 'N' != 'W' (passed on latest, fails on 3.2) def test_east_asian_width_unassigned(self): @@ -661,30 +721,434 @@ def test_east_asian_width_unassigned(self): self.assertEqual(eaw(char), 'A') self.assertIs(self.db.name(char, None), None) +class UnicodeFunctionsTest(unittest.TestCase, BaseUnicodeFunctionsTest): + db = unicodedata + old = False + + # Update this if the database changes. Make sure to do a full rebuild + # (e.g. 'make distclean && make') to get the correct checksum. + expectedchecksum = ('00b13fa975a60b1d3f490f1fc8c126ab24990c75' + if quicktest else + 'ebfc9dd281c2226998fd435744dd2e9321899beb') + + @requires_resource('network') + @unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: None != 'TANGUT IDEOGRAPH-17000' + def test_all_names(self): + TESTDATAFILE = "DerivedName.txt" + testdata = download_test_data_file(TESTDATAFILE) + + with testdata: + self.run_name_tests(testdata) + + def run_name_tests(self, testdata): + names_ref = {} + + def parse_cp(s): + return int(s, 16) + + # Parse data + for line in testdata: + line = line.strip() + if not line or line.startswith("#"): + continue + raw_cp, name = line.split("; ") + # Check for a range + if ".." in raw_cp: + cp1, cp2 = map(parse_cp, raw_cp.split("..")) + # remove ‘*’ at the end + assert name[-1] == '*', (raw_cp, name) + name = name[:-1] + for cp in range(cp1, cp2 + 1): + names_ref[cp] = f"{name}{cp:04X}" + elif name[-1] == '*': + cp = parse_cp(raw_cp) + name = name[:-1] + names_ref[cp] = f"{name}{cp:04X}" + else: + assert '*' not in name, (raw_cp, name) + cp = parse_cp(raw_cp) + names_ref[cp] = name + + for cp in range(0, sys.maxunicode + 1): + self.assertEqual(self.db.name(chr(cp), None), names_ref.get(cp)) + + @unittest.expectedFailure # TODO: RUSTPYTHON; AttributeError: module 'unicodedata' has no attribute 'isxidstart' + def test_isxidstart(self): + self.assertTrue(self.db.isxidstart('S')) + self.assertTrue(self.db.isxidstart('\u0AD0')) # GUJARATI OM + self.assertTrue(self.db.isxidstart('\u0EC6')) # LAO KO LA + self.assertTrue(self.db.isxidstart('\u17DC')) # KHMER SIGN AVAKRAHASANYA + self.assertTrue(self.db.isxidstart('\uA015')) # YI SYLLABLE WU + self.assertTrue(self.db.isxidstart('\uFE7B')) # ARABIC KASRA MEDIAL FORM + + self.assertFalse(self.db.isxidstart(' ')) + self.assertFalse(self.db.isxidstart('0')) + self.assertRaises(TypeError, self.db.isxidstart) + self.assertRaises(TypeError, self.db.isxidstart, 'xx') + + @unittest.expectedFailure # TODO: RUSTPYTHON; AttributeError: module 'unicodedata' has no attribute 'isxidcontinue' + def test_isxidcontinue(self): + self.assertTrue(self.db.isxidcontinue('S')) + self.assertTrue(self.db.isxidcontinue('_')) + self.assertTrue(self.db.isxidcontinue('0')) + self.assertTrue(self.db.isxidcontinue('\u00BA')) # MASCULINE ORDINAL INDICATOR + self.assertTrue(self.db.isxidcontinue('\u0640')) # ARABIC TATWEEL + self.assertTrue(self.db.isxidcontinue('\u0710')) # SYRIAC LETTER ALAPH + self.assertTrue(self.db.isxidcontinue('\u0B3E')) # ORIYA VOWEL SIGN AA + self.assertTrue(self.db.isxidcontinue('\u17D7')) # KHMER SIGN LEK TOO + + self.assertFalse(self.db.isxidcontinue(' ')) + self.assertRaises(TypeError, self.db.isxidcontinue) + self.assertRaises(TypeError, self.db.isxidcontinue, 'xx') + + @unittest.expectedFailure # TODO: RUSTPYTHON; AttributeError: module 'unicodedata' has no attribute 'grapheme_cluster_break' + def test_grapheme_cluster_break(self): + gcb = self.db.grapheme_cluster_break + self.assertEqual(gcb(' '), 'Other') + self.assertEqual(gcb('x'), 'Other') + self.assertEqual(gcb('\U0010FFFF'), 'Other') + self.assertEqual(gcb('\r'), 'CR') + self.assertEqual(gcb('\n'), 'LF') + self.assertEqual(gcb('\0'), 'Control') + self.assertEqual(gcb('\t'), 'Control') + self.assertEqual(gcb('\x1F'), 'Control') + self.assertEqual(gcb('\x7F'), 'Control') + self.assertEqual(gcb('\x9F'), 'Control') + self.assertEqual(gcb('\U000E0001'), 'Control') + self.assertEqual(gcb('\u0300'), 'Extend') + self.assertEqual(gcb('\u200C'), 'Extend') + self.assertEqual(gcb('\U000E01EF'), 'Extend') + self.assertEqual(gcb('\u1159'), 'L') + self.assertEqual(gcb('\u11F9'), 'T') + self.assertEqual(gcb('\uD788'), 'LV') + self.assertEqual(gcb('\uD7A3'), 'LVT') + # New in 5.0.0 + self.assertEqual(gcb('\u05BA'), 'Extend') + self.assertEqual(gcb('\u20EF'), 'Extend') + # New in 5.1.0 + self.assertEqual(gcb('\u2064'), 'Control') + self.assertEqual(gcb('\uAA4D'), 'SpacingMark') + # New in 5.2.0 + self.assertEqual(gcb('\u0816'), 'Extend') + self.assertEqual(gcb('\uA97C'), 'L') + self.assertEqual(gcb('\uD7C6'), 'V') + self.assertEqual(gcb('\uD7FB'), 'T') + # New in 6.0.0 + self.assertEqual(gcb('\u093A'), 'Extend') + self.assertEqual(gcb('\U00011002'), 'SpacingMark') + # New in 6.1.0 + self.assertEqual(gcb('\U000E0FFF'), 'Control') + self.assertEqual(gcb('\U00016F7E'), 'SpacingMark') + # New in 6.2.0 + self.assertEqual(gcb('\U0001F1E6'), 'Regional_Indicator') + self.assertEqual(gcb('\U0001F1FF'), 'Regional_Indicator') + # New in 6.3.0 + self.assertEqual(gcb('\u180E'), 'Control') + self.assertEqual(gcb('\u1A1B'), 'Extend') + # New in 7.0.0 + self.assertEqual(gcb('\u0E33'), 'SpacingMark') + self.assertEqual(gcb('\u0EB3'), 'SpacingMark') + self.assertEqual(gcb('\U0001BCA3'), 'Control') + self.assertEqual(gcb('\U0001E8D6'), 'Extend') + self.assertEqual(gcb('\U0001163E'), 'SpacingMark') + # New in 8.0.0 + self.assertEqual(gcb('\u08E3'), 'Extend') + self.assertEqual(gcb('\U00011726'), 'SpacingMark') + # New in 9.0.0 + self.assertEqual(gcb('\u0600'), 'Prepend') + self.assertEqual(gcb('\U000E007F'), 'Extend') + self.assertEqual(gcb('\U00011CB4'), 'SpacingMark') + self.assertEqual(gcb('\u200D'), 'ZWJ') + # New in 10.0.0 + self.assertEqual(gcb('\U00011D46'), 'Prepend') + self.assertEqual(gcb('\U00011D47'), 'Extend') + self.assertEqual(gcb('\U00011A97'), 'SpacingMark') + # New in 11.0.0 + self.assertEqual(gcb('\U000110CD'), 'Prepend') + self.assertEqual(gcb('\u07FD'), 'Extend') + self.assertEqual(gcb('\U00011EF6'), 'SpacingMark') + # New in 12.0.0 + self.assertEqual(gcb('\U00011A84'), 'Prepend') + self.assertEqual(gcb('\U00013438'), 'Control') + self.assertEqual(gcb('\U0001E2EF'), 'Extend') + self.assertEqual(gcb('\U00016F87'), 'SpacingMark') + # New in 13.0.0 + self.assertEqual(gcb('\U00011941'), 'Prepend') + self.assertEqual(gcb('\U00016FE4'), 'Extend') + self.assertEqual(gcb('\U00011942'), 'SpacingMark') + # New in 14.0.0 + self.assertEqual(gcb('\u0891'), 'Prepend') + self.assertEqual(gcb('\U0001E2AE'), 'Extend') + # New in 15.0.0 + self.assertEqual(gcb('\U00011F02'), 'Prepend') + self.assertEqual(gcb('\U0001343F'), 'Control') + self.assertEqual(gcb('\U0001E4EF'), 'Extend') + self.assertEqual(gcb('\U00011F3F'), 'SpacingMark') + # New in 16.0.0 + self.assertEqual(gcb('\U000113D1'), 'Prepend') + self.assertEqual(gcb('\U0001E5EF'), 'Extend') + self.assertEqual(gcb('\U0001612C'), 'SpacingMark') + self.assertEqual(gcb('\U00016D63'), 'V') + # New in 17.0.0 + self.assertEqual(gcb('\u1AEB'), 'Extend') + self.assertEqual(gcb('\U00011B67'), 'SpacingMark') + + self.assertRaises(TypeError, gcb) + self.assertRaises(TypeError, gcb, b'x') + self.assertRaises(TypeError, gcb, 120) + self.assertRaises(TypeError, gcb, '') + self.assertRaises(TypeError, gcb, 'xx') + + @unittest.expectedFailure # TODO: RUSTPYTHON; AttributeError: module 'unicodedata' has no attribute 'indic_conjunct_break' + def test_indic_conjunct_break(self): + incb = self.db.indic_conjunct_break + self.assertEqual(incb(' '), 'None') + self.assertEqual(incb('x'), 'None') + self.assertEqual(incb('\U0010FFFF'), 'None') + # New in 15.1.0 + self.assertEqual(incb('\u094D'), 'Linker') + self.assertEqual(incb('\u0D4D'), 'Linker') + self.assertEqual(incb('\u0915'), 'Consonant') + self.assertEqual(incb('\u0D3A'), 'Consonant') + self.assertEqual(incb('\u0300'), 'Extend') + self.assertEqual(incb('\U0001E94A'), 'Extend') + # New in 16.0.0 + self.assertEqual(incb('\u034F'), 'Extend') + self.assertEqual(incb('\U000E01EF'), 'Extend') + # New in 17.0.0 + self.assertEqual(incb('\u1039'), 'Linker') + self.assertEqual(incb('\U00011F42'), 'Linker') + self.assertEqual(incb('\u1000'), 'Consonant') + self.assertEqual(incb('\U00011F33'), 'Consonant') + self.assertEqual(incb('\U0001E6F5'), 'Extend') + + self.assertRaises(TypeError, incb) + self.assertRaises(TypeError, incb, b'x') + self.assertRaises(TypeError, incb, 120) + self.assertRaises(TypeError, incb, '') + self.assertRaises(TypeError, incb, 'xx') + + @unittest.expectedFailure # TODO: RUSTPYTHON; AttributeError: module 'unicodedata' has no attribute 'extended_pictographic' + def test_extended_pictographic(self): + ext_pict = self.db.extended_pictographic + self.assertIs(ext_pict(' '), False) + self.assertIs(ext_pict('x'), False) + self.assertIs(ext_pict('\U0010FFFF'), False) + # New in 13.0.0 + self.assertIs(ext_pict('\xA9'), True) + self.assertIs(ext_pict('\u203C'), True) + self.assertIs(ext_pict('\U0001FAD6'), True) + self.assertIs(ext_pict('\U0001FFFD'), True) + # New in 17.0.0 + self.assertIs(ext_pict('\u2388'), False) + self.assertIs(ext_pict('\U0001FA6D'), False) + + self.assertRaises(TypeError, ext_pict) + self.assertRaises(TypeError, ext_pict, b'x') + self.assertRaises(TypeError, ext_pict, 120) + self.assertRaises(TypeError, ext_pict, '') + self.assertRaises(TypeError, ext_pict, 'xx') + + @unittest.expectedFailure # TODO: RUSTPYTHON; AttributeError: module 'unicodedata' has no attribute 'iter_graphemes' + def test_grapheme_break(self): + def graphemes(*args): + return list(map(str, self.db.iter_graphemes(*args))) + + self.assertRaises(TypeError, self.db.iter_graphemes) + self.assertRaises(TypeError, self.db.iter_graphemes, b'x') + self.assertRaises(TypeError, self.db.iter_graphemes, 'x', 0, 0, 0) + + self.assertEqual(graphemes(''), []) + self.assertEqual(graphemes('abcd'), ['a', 'b', 'c', 'd']) + self.assertEqual(graphemes('abcd', 1), ['b', 'c', 'd']) + self.assertEqual(graphemes('abcd', 1, 3), ['b', 'c']) + self.assertEqual(graphemes('abcd', -3), ['b', 'c', 'd']) + self.assertEqual(graphemes('abcd', 1, -1), ['b', 'c']) + self.assertEqual(graphemes('abcd', 3, 1), []) + self.assertEqual(graphemes('abcd', 5), []) + self.assertEqual(graphemes('abcd', 0, 5), ['a', 'b', 'c', 'd']) + self.assertEqual(graphemes('abcd', -5), ['a', 'b', 'c', 'd']) + self.assertEqual(graphemes('abcd', 0, -5), []) + # GB3 + self.assertEqual(graphemes('\r\n'), ['\r\n']) + # GB4 + self.assertEqual(graphemes('\r\u0308'), ['\r', '\u0308']) + self.assertEqual(graphemes('\n\u0308'), ['\n', '\u0308']) + self.assertEqual(graphemes('\0\u0308'), ['\0', '\u0308']) + # GB5 + self.assertEqual(graphemes('\u06dd\r'), ['\u06dd', '\r']) + self.assertEqual(graphemes('\u06dd\n'), ['\u06dd', '\n']) + self.assertEqual(graphemes('\u06dd\0'), ['\u06dd', '\0']) + # GB6 + self.assertEqual(graphemes('\u1100\u1160'), ['\u1100\u1160']) + self.assertEqual(graphemes('\u1100\uAC00'), ['\u1100\uAC00']) + self.assertEqual(graphemes('\u1100\uAC01'), ['\u1100\uAC01']) + # GB7 + self.assertEqual(graphemes('\uAC00\u1160'), ['\uAC00\u1160']) + self.assertEqual(graphemes('\uAC00\u11A8'), ['\uAC00\u11A8']) + self.assertEqual(graphemes('\u1160\u1160'), ['\u1160\u1160']) + self.assertEqual(graphemes('\u1160\u11A8'), ['\u1160\u11A8']) + # GB8 + self.assertEqual(graphemes('\uAC01\u11A8'), ['\uAC01\u11A8']) + self.assertEqual(graphemes('\u11A8\u11A8'), ['\u11A8\u11A8']) + # GB9 + self.assertEqual(graphemes('a\u0300'), ['a\u0300']) + self.assertEqual(graphemes('a\u200D'), ['a\u200D']) + # GB9a + self.assertEqual(graphemes('\u0905\u0903'), ['\u0905\u0903']) + # GB9b + self.assertEqual(graphemes('\u06dd\u0661'), ['\u06dd\u0661']) + # GB9c + self.assertEqual(graphemes('\u0915\u094d\u0924'), + ['\u0915\u094d\u0924']) + self.assertEqual(graphemes('\u0915\u094D\u094D\u0924'), + ['\u0915\u094D\u094D\u0924']) + self.assertEqual(graphemes('\u0915\u094D\u0924\u094D\u092F'), + ['\u0915\u094D\u0924\u094D\u092F']) + # GB11 + self.assertEqual(graphemes( + '\U0001F9D1\U0001F3FE\u200D\u2764\uFE0F' + '\u200D\U0001F48B\u200D\U0001F9D1\U0001F3FC'), + ['\U0001F9D1\U0001F3FE\u200D\u2764\uFE0F' + '\u200D\U0001F48B\u200D\U0001F9D1\U0001F3FC']) + # GB12 + self.assertEqual(graphemes( + '\U0001F1FA\U0001F1E6\U0001F1FA\U0001F1F3'), + ['\U0001F1FA\U0001F1E6', '\U0001F1FA\U0001F1F3']) + # GB13 + self.assertEqual(graphemes( + 'a\U0001F1FA\U0001F1E6\U0001F1FA\U0001F1F3'), + ['a', '\U0001F1FA\U0001F1E6', '\U0001F1FA\U0001F1F3']) + + @unittest.expectedFailure # TODO: RUSTPYTHON; AttributeError: module 'unicodedata' has no attribute 'block' + def test_block(self): + self.assertEqual(self.db.block('\u0000'), 'Basic Latin') + self.assertEqual(self.db.block('\u0041'), 'Basic Latin') + self.assertEqual(self.db.block('\u007F'), 'Basic Latin') + self.assertEqual(self.db.block('\u0080'), 'Latin-1 Supplement') + self.assertEqual(self.db.block('\u00FF'), 'Latin-1 Supplement') + self.assertEqual(self.db.block('\u1159'), 'Hangul Jamo') + self.assertEqual(self.db.block('\u11F9'), 'Hangul Jamo') + self.assertEqual(self.db.block('\uD788'), 'Hangul Syllables') + self.assertEqual(self.db.block('\uD7A3'), 'Hangul Syllables') + # New in 5.0.0 + self.assertEqual(self.db.block('\u05BA'), 'Hebrew') + self.assertEqual(self.db.block('\u20EF'), 'Combining Diacritical Marks for Symbols') + # New in 5.1.0 + self.assertEqual(self.db.block('\u2064'), 'General Punctuation') + self.assertEqual(self.db.block('\uAA4D'), 'Cham') + # New in 5.2.0 + self.assertEqual(self.db.block('\u0816'), 'Samaritan') + self.assertEqual(self.db.block('\uA97C'), 'Hangul Jamo Extended-A') + self.assertEqual(self.db.block('\uD7C6'), 'Hangul Jamo Extended-B') + self.assertEqual(self.db.block('\uD7FB'), 'Hangul Jamo Extended-B') + # New in 6.0.0 + self.assertEqual(self.db.block('\u093A'), 'Devanagari') + self.assertEqual(self.db.block('\U00011002'), 'Brahmi') + # New in 6.1.0 + self.assertEqual(self.db.block('\U000E0FFF'), 'No_Block') + self.assertEqual(self.db.block('\U00016F7E'), 'Miao') + # New in 6.2.0 + self.assertEqual(self.db.block('\U0001F1E6'), 'Enclosed Alphanumeric Supplement') + self.assertEqual(self.db.block('\U0001F1FF'), 'Enclosed Alphanumeric Supplement') + # New in 6.3.0 + self.assertEqual(self.db.block('\u180E'), 'Mongolian') + self.assertEqual(self.db.block('\u1A1B'), 'Buginese') + # New in 7.0.0 + self.assertEqual(self.db.block('\u0E33'), 'Thai') + self.assertEqual(self.db.block('\u0EB3'), 'Lao') + self.assertEqual(self.db.block('\U0001BCA3'), 'Shorthand Format Controls') + self.assertEqual(self.db.block('\U0001E8D6'), 'Mende Kikakui') + self.assertEqual(self.db.block('\U0001163E'), 'Modi') + # New in 8.0.0 + self.assertEqual(self.db.block('\u08E3'), 'Arabic Extended-A') + self.assertEqual(self.db.block('\U00011726'), 'Ahom') + # New in 9.0.0 + self.assertEqual(self.db.block('\u0600'), 'Arabic') + self.assertEqual(self.db.block('\U000E007F'), 'Tags') + self.assertEqual(self.db.block('\U00011CB4'), 'Marchen') + self.assertEqual(self.db.block('\u200D'), 'General Punctuation') + # New in 10.0.0 + self.assertEqual(self.db.block('\U00011D46'), 'Masaram Gondi') + self.assertEqual(self.db.block('\U00011D47'), 'Masaram Gondi') + self.assertEqual(self.db.block('\U00011A97'), 'Soyombo') + # New in 11.0.0 + self.assertEqual(self.db.block('\U000110CD'), 'Kaithi') + self.assertEqual(self.db.block('\u07FD'), 'NKo') + self.assertEqual(self.db.block('\U00011EF6'), 'Makasar') + # New in 12.0.0 + self.assertEqual(self.db.block('\U00011A84'), 'Soyombo') + self.assertEqual(self.db.block('\U00013438'), 'Egyptian Hieroglyph Format Controls') + self.assertEqual(self.db.block('\U0001E2EF'), 'Wancho') + self.assertEqual(self.db.block('\U00016F87'), 'Miao') + # New in 13.0.0 + self.assertEqual(self.db.block('\U00011941'), 'Dives Akuru') + self.assertEqual(self.db.block('\U00016FE4'), 'Ideographic Symbols and Punctuation') + self.assertEqual(self.db.block('\U00011942'), 'Dives Akuru') + # New in 14.0.0 + self.assertEqual(self.db.block('\u0891'), 'Arabic Extended-B') + self.assertEqual(self.db.block('\U0001E2AE'), 'Toto') + # New in 15.0.0 + self.assertEqual(self.db.block('\U00011F02'), 'Kawi') + self.assertEqual(self.db.block('\U0001343F'), 'Egyptian Hieroglyph Format Controls') + self.assertEqual(self.db.block('\U0001E4EF'), 'Nag Mundari') + self.assertEqual(self.db.block('\U00011F3F'), 'Kawi') + # New in 16.0.0 + self.assertEqual(self.db.block('\U000113D1'), 'Tulu-Tigalari') + self.assertEqual(self.db.block('\U0001E5EF'), 'Ol Onal') + self.assertEqual(self.db.block('\U0001612C'), 'Gurung Khema') + self.assertEqual(self.db.block('\U00016D63'), 'Kirat Rai') + # New in 17.0.0 + self.assertEqual(self.db.block('\u1AEB'), 'Combining Diacritical Marks Extended') + self.assertEqual(self.db.block('\U00011B67'), 'Sharada Supplement') + # Unassigned + self.assertEqual(self.db.block('\U00100000'), 'Supplementary Private Use Area-B') + self.assertEqual(self.db.block('\U0010FFFF'), 'Supplementary Private Use Area-B') + + @unittest.expectedFailure # TODO: RUSTPYTHON; AttributeError: module 'unicodedata' has no attribute 'block' + def test_block_invalid_input(self): + self.assertRaises(TypeError, self.db.block) + self.assertRaises(TypeError, self.db.block, b'x') + self.assertRaises(TypeError, self.db.block, 120) + self.assertRaises(TypeError, self.db.block, '') + self.assertRaises(TypeError, self.db.block, 'xx') + @unittest.expectedFailure # TODO: RUSTPYTHON; + N def test_east_asian_width_9_0_changes(self): return super().test_east_asian_width_9_0_changes() + @unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: 'D4CC 11B6' != '1111 1171 11B6' + def test_decomposition(self): + return super().test_decomposition() + + @unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: KeyError not raised by lookup + def test_lookup_nonexistant(self): + return super().test_lookup_nonexistant() + + @unittest.expectedFailure # TODO: RUSTPYTHON; AttributeError: module 'unicodedata' has no attribute 'digit' + def test_function_checksum(self): + return super().test_function_checksum() + -class Unicode_3_2_0_FunctionsTest(UnicodeFunctionsTest): +class Unicode_3_2_0_FunctionsTest(unittest.TestCase, BaseUnicodeFunctionsTest): db = unicodedata.ucd_3_2_0 old = True expectedchecksum = ('883824cb6c0ccf994e4451ebf281e2d6d479af47' if quicktest else - 'caf1a7f2f380f927461837f1901ef20683f98683') + '68cd01e2c680b851c1fcab012efb5635b2229c2b') @unittest.expectedFailure # TODO: RUSTPYTHON def test_normalization(self): return super().test_normalization() - @unittest.expectedSuccess # TODO: RUSTPYTHON - def test_combining(self): - return super().test_combining() - @unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: 'LATIN SMALL LETTER D WITH CURL' != None def test_name(self): return super().test_name() + @unittest.expectedSuccess # TODO: RUSTPYTHON + def test_combining(self): + return super().test_combining() + class UnicodeMiscTest(unittest.TestCase): db = unicodedata @@ -712,6 +1176,23 @@ def test_failed_import_during_compiling(self): "(can't load unicodedata module)" self.assertIn(error, result.err.decode("ascii")) + @unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError Process return code is 1 + def test_unicodedata_unload_reload(self): + # gh-149449: dropping unicodedata and running gc must not leave the + # cached _ucnhash_CAPI pointer dangling. + code = ( + "import gc, sys\n" + "assert '\\N{GRINNING FACE}'.encode(" + " 'ascii', errors='namereplace') == b'\\\\N{GRINNING FACE}'\n" + "compile(r\"x = '\\\\N{LATIN CAPITAL LETTER A}'\", '', 'exec')\n" + "del sys.modules['unicodedata']\n" + "gc.collect()\n" + "assert '\\N{WINKING FACE}'.encode(" + " 'ascii', errors='namereplace') == b'\\\\N{WINKING FACE}'\n" + "compile(r\"x = '\\\\N{LATIN CAPITAL LETTER B}'\", '', 'exec')\n" + ) + script_helper.assert_python_ok("-c", code) + def test_decimal_numeric_consistent(self): # Test that decimal and numeric are consistent, # i.e. if a character has a decimal value, @@ -788,13 +1269,20 @@ def test_linebreak_7643(self): self.assertEqual(len(lines), 1, r"%a should not be a linebreak" % c) + @unittest.expectedFailure # TODO: RUSTPYTHON; AttributeError: module 'unicodedata' has no attribute 'iter_graphemes' + def test_segment_object(self): + segments = list(unicodedata.iter_graphemes('spa\u0300m')) + self.assertEqual(len(segments), 4, segments) + segment = segments[2] + self.assertEqual(segment.start, 2) + self.assertEqual(segment.end, 4) + self.assertEqual(str(segment), 'a\u0300') + self.assertEqual(repr(segment), '') + self.assertRaises(TypeError, iter, segment) + self.assertRaises(TypeError, len, segment) -class NormalizationTest(unittest.TestCase): - @staticmethod - def check_version(testfile): - hdr = testfile.readline() - return unicodedata.unidata_version in hdr +class NormalizationTest(unittest.TestCase): @staticmethod def unistr(data): data = [int(x, 16) for x in data.split(" ")] @@ -804,17 +1292,7 @@ def unistr(data): @requires_resource('cpu') def test_normalization(self): TESTDATAFILE = "NormalizationTest.txt" - TESTDATAURL = f"http://www.pythontest.net/unicode/{unicodedata.unidata_version}/{TESTDATAFILE}" - - # Hit the exception early - try: - testdata = open_urlresource(TESTDATAURL, encoding="utf-8", - check=self.check_version) - except PermissionError: - self.skipTest(f"Permission error when downloading {TESTDATAURL} " - f"into the test data directory") - except (OSError, HTTPException) as exc: - self.skipTest(f"Failed to download {TESTDATAURL}: {exc}") + testdata = download_test_data_file(TESTDATAFILE) with testdata: self.run_normalization_tests(testdata, unicodedata) @@ -911,5 +1389,70 @@ class MyStr(str): self.assertIs(type(normalize(form, MyStr(input_str))), str) +class GraphemeBreakTest(unittest.TestCase): + @requires_resource('network') + @unittest.expectedFailure # TODO: RUSTPYTHON; AttributeError module 'unicodedata' has no attribute 'iter_graphemes' + def test_grapheme_break(self): + TESTDATAFILE = "GraphemeBreakTest.txt" + testdata = download_test_data_file(TESTDATAFILE) + + with testdata: + self.run_grapheme_break_tests(testdata) + + def run_grapheme_break_tests(self, testdata): + for line in testdata: + line, _, comment = line.partition('#') + line = line.strip() + if not line: + continue + comment = comment.strip() + + chunks = [] + breaks = [] + pos = 0 + for field in line.replace('×', ' ').split(): + if field == '÷': + chunks.append('') + breaks.append(pos) + else: + chunks[-1] += chr(int(field, 16)) + pos += 1 + self.assertEqual(chunks.pop(), '', line) + input = ''.join(chunks) + with self.subTest(line): + result = list(unicodedata.iter_graphemes(input)) + self.assertEqual(list(map(str, result)), chunks, comment) + self.assertEqual([x.start for x in result], breaks[:-1], comment) + self.assertEqual([x.end for x in result], breaks[1:], comment) + for i in range(1, len(breaks) - 1): + result = list(unicodedata.iter_graphemes(input, breaks[i])) + self.assertEqual(list(map(str, result)), chunks[i:], comment) + self.assertEqual([x.start for x in result], breaks[i:-1], comment) + self.assertEqual([x.end for x in result], breaks[i+1:], comment) + + @unittest.expectedFailure # TODO: RUSTPYTHON; AttributeError: module 'unicodedata' has no attribute 'iter_graphemes' + def test_reference_loops(self): + # Test that reference loops involving GraphemeBreakIterator or + # Segment can be broken by the garbage collector. + class S(str): + pass + + s = S('abc') + s.ref = unicodedata.iter_graphemes(s) + wr = weakref.ref(s) + del s + self.assertIsNotNone(wr()) + gc_collect() + self.assertIsNone(wr()) + + s = S('abc') + s.ref = next(unicodedata.iter_graphemes(s)) + wr = weakref.ref(s) + del s + self.assertIsNotNone(wr()) + gc_collect() + self.assertIsNone(wr()) + + if __name__ == "__main__": unittest.main() From 7931cb062b13f04bff7aea1a5d0a490acdf635ad Mon Sep 17 00:00:00 2001 From: Shahar Naveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Wed, 26 Aug 2026 17:38:10 +0300 Subject: [PATCH 3/5] Update some tests & libs to `3.14.7` (#8595) * test_memoryio * test_ordered_dict * more * urllib * test_compileall * urllib test * more * test_descr * test_enumerate --- Lib/test/list_tests.py | 3 +- Lib/test/mapping_tests.py | 1 + Lib/test/test_calendar.py | 5 + Lib/test/test_codeop.py | 173 +++++++++++++++++--------- Lib/test/test_compileall.py | 65 +++++++--- Lib/test/test_dataclasses/__init__.py | 133 +++++++++++++++++++- Lib/test/test_descr.py | 11 ++ Lib/test/test_dictviews.py | 5 +- Lib/test/test_enumerate.py | 26 ++++ Lib/test/test_memoryio.py | 8 +- Lib/test/test_ordered_dict.py | 35 ++++++ Lib/test/test_random.py | 15 +++ Lib/test/test_robotparser.py | 28 +++++ Lib/test/test_urllib.py | 24 ++++ Lib/test/test_urllib2.py | 29 +++++ Lib/test/test_urlparse.py | 15 ++- Lib/urllib/parse.py | 4 +- Lib/urllib/request.py | 3 +- Lib/urllib/robotparser.py | 16 ++- 19 files changed, 504 insertions(+), 95 deletions(-) diff --git a/Lib/test/list_tests.py b/Lib/test/list_tests.py index e76f79c274e..ad9a9ea8303 100644 --- a/Lib/test/list_tests.py +++ b/Lib/test/list_tests.py @@ -6,7 +6,7 @@ from functools import cmp_to_key from test import seq_tests -from test.support import ALWAYS_EQ, NEVER_EQ +from test.support import ALWAYS_EQ, NEVER_EQ, run_with_limited_c_stack from test.support import skip_emscripten_stack_overflow, skip_wasi_stack_overflow @@ -60,6 +60,7 @@ def test_repr(self): self.assertEqual(str(a2), "[0, 1, 2, [...], 3]") self.assertEqual(repr(a2), "[0, 1, 2, [...], 3]") + @run_with_limited_c_stack(200_000) @skip_wasi_stack_overflow() @skip_emscripten_stack_overflow() def test_repr_deep(self): diff --git a/Lib/test/mapping_tests.py b/Lib/test/mapping_tests.py index 20306e1526d..1358200add5 100644 --- a/Lib/test/mapping_tests.py +++ b/Lib/test/mapping_tests.py @@ -622,6 +622,7 @@ def __repr__(self): d = self._full_mapping({1: BadRepr()}) self.assertRaises(Exc, repr, d) + @support.run_with_limited_c_stack() @support.skip_wasi_stack_overflow() @support.skip_emscripten_stack_overflow() @support.skip_if_sanitizer("requires deep stack", ub=True) diff --git a/Lib/test/test_calendar.py b/Lib/test/test_calendar.py index ca93e99d142..50ccf66b8da 100644 --- a/Lib/test/test_calendar.py +++ b/Lib/test/test_calendar.py @@ -509,6 +509,11 @@ def test_deprecation_warning(self): "The 'January' attribute is deprecated, use 'JANUARY' instead" ): calendar.January + with self.assertWarnsRegex( + DeprecationWarning, + "The 'February' attribute is deprecated, use 'FEBRUARY' instead" + ): + calendar.February def test_isleap(self): # Make sure that the return is right for a few years, and diff --git a/Lib/test/test_codeop.py b/Lib/test/test_codeop.py index 2e1568d5ea2..248041fdf19 100644 --- a/Lib/test/test_codeop.py +++ b/Lib/test/test_codeop.py @@ -4,43 +4,64 @@ """ import unittest import warnings -from test.support import warnings_helper +from test.support import subTests, warnings_helper from textwrap import dedent +import functools -from codeop import compile_command, PyCF_DONT_IMPLY_DEDENT +from codeop import compile_command, CommandCompiler, Compile +from codeop import PyCF_DONT_IMPLY_DEDENT, PyCF_ONLY_AST +import ast + + +WRAPPING_COMPILERS = [compile_command, CommandCompiler()] +RAW_COMPILERS = [Compile()] +COMPILERS = WRAPPING_COMPILERS + RAW_COMPILERS -class CodeopTests(unittest.TestCase): - def assertValid(self, str, symbol='single'): +class CodeopTests(unittest.TestCase): + def assertValid(self, str, symbol='single', *, compiler): '''succeed iff str is a valid piece of code''' expected = compile(str, "", symbol, PyCF_DONT_IMPLY_DEDENT) - self.assertEqual(compile_command(str, "", symbol), expected) + self.assertEqual(compiler(str, "", symbol), expected) - def assertIncomplete(self, str, symbol='single'): + def assertIncomplete(self, str, symbol='single', *, compiler): '''succeed iff str is the start of a valid piece of code''' - self.assertEqual(compile_command(str, symbol=symbol), None) - - def assertInvalid(self, str, symbol='single', is_syntax=1): + if compiler in WRAPPING_COMPILERS: + self.assertEqual(compiler(str, "", symbol=symbol), None) + else: + # Compile has should raise like built-in compile + with self.assertRaises(SyntaxError) as cm_original_error: + compile(str, "", symbol, compiler.flags) + expected_error = cm_original_error.exception + with self.assertRaises(type(expected_error)) as cm_wrapped_error: + compiler(str, "", symbol=symbol) + self.assertEqual( + expected_error.args, + cm_wrapped_error.exception.args + ) + + def assertInvalid(self, str, symbol='single', is_syntax=1, *, compiler): '''succeed iff str is the start of an invalid piece of code''' try: - compile_command(str,symbol=symbol) + compiler(str,"", symbol=symbol) self.fail("No exception raised for invalid code") except SyntaxError: self.assertTrue(is_syntax) except OverflowError: self.assertTrue(not is_syntax) - def test_valid(self): - av = self.assertValid - - # special case - self.assertEqual(compile_command(""), - compile("pass", "", 'single', - PyCF_DONT_IMPLY_DEDENT)) - self.assertEqual(compile_command("\n"), - compile("pass", "", 'single', - PyCF_DONT_IMPLY_DEDENT)) - + @subTests('compiler', WRAPPING_COMPILERS) + def test_empty(self, compiler): + self.assertEqual( + compiler("", "", 'single'), + compile("pass", "", 'single', PyCF_DONT_IMPLY_DEDENT)) + self.assertEqual( + compiler("\n", "", 'single'), + compile("pass", "", 'single', PyCF_DONT_IMPLY_DEDENT)) + + @subTests('compiler', COMPILERS) + def test_valid(self, compiler): + av = functools.partial(self.assertValid, compiler=compiler) av("a = 1") av("\na = 1") av("a = 1\n") @@ -93,8 +114,9 @@ def test_valid(self): av("@a.b.c\ndef f():\n pass\n") @unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: at 0xc99532080 file "", line 1> != None - def test_incomplete(self): - ai = self.assertIncomplete + @subTests('compiler', COMPILERS) + def test_incomplete(self, compiler): + ai = functools.partial(self.assertIncomplete, compiler=compiler) ai("(a **") ai("(a,b,") @@ -227,8 +249,9 @@ def test_incomplete(self): ai('a = f"""') ai('a = \\') - def test_invalid(self): - ai = self.assertInvalid + @subTests('compiler', COMPILERS) + def test_invalid(self, compiler): + ai = functools.partial(self.assertInvalid, compiler=compiler) ai("a b") ai("a @") @@ -264,8 +287,9 @@ def test_invalid(self): ai("[i for i in range(10)] = (1, 2, 3)") - def test_invalid_exec(self): - ai = self.assertInvalid + @subTests('compiler', COMPILERS) + def test_invalid_exec(self, compiler): + ai = functools.partial(self.assertInvalid, compiler=compiler) ai("raise = 4", symbol="exec") ai('def a-b', symbol='exec') ai('await?', symbol='exec') @@ -273,59 +297,96 @@ def test_invalid_exec(self): ai('a await raise b', symbol='exec') ai('a await raise b?+1', symbol='exec') - def test_filename(self): - self.assertEqual(compile_command("a = 1\n", "abc").co_filename, - compile("a = 1\n", "abc", 'single').co_filename) - self.assertNotEqual(compile_command("a = 1\n", "abc").co_filename, - compile("a = 1\n", "def", 'single').co_filename) - - def test_warning(self): + @subTests('compiler', COMPILERS) + def test_filename(self, compiler): + self.assertEqual( + compiler("a = 1\n", "abc", "single").co_filename, + compile("a = 1\n", "abc", 'single').co_filename + ) + self.assertNotEqual( + compiler("a = 1\n", "abc", "single").co_filename, + compile("a = 1\n", "def", 'single').co_filename + ) + + def assertReturnsModule(self, code, compiler): + retval = compiler(code, "", 'exec', PyCF_ONLY_AST) + self.assertIsInstance(retval, ast.Module) + + @subTests('compiler', RAW_COMPILERS) + def test_ast_return_value(self, compiler): + validate_ast = self.assertReturnsModule + validate_ast("x = 5", compiler) + validate_ast("\nx = 5", compiler) + validate_ast("x = 5\n", compiler) + validate_ast("x = 5\n\n", compiler) + validate_ast("\n\nx = 5\n\n", compiler) + + @subTests('compiler', COMPILERS) + def test_warning(self, compiler): # Test that the warning is only returned once. with warnings_helper.check_warnings( ('"is" with \'str\' literal', SyntaxWarning), ('"\\\\e" is an invalid escape sequence', SyntaxWarning), ) as w: - compile_command(r"'\e' is 0") - self.assertEqual(len(w.warnings), 2) + compiler(r"'\e' is 0", "", "single") + self.assertEqual(len(w.warnings), 2) # bpo-41520: check SyntaxWarning treated as an SyntaxError with warnings.catch_warnings(), self.assertRaises(SyntaxError): warnings.simplefilter('error', SyntaxWarning) - compile_command('1 is 1', symbol='exec') + compiler('1 is 1', "", 'exec') # Check SyntaxWarning treated as an SyntaxError with warnings.catch_warnings(), self.assertRaises(SyntaxError): warnings.simplefilter('error', SyntaxWarning) - compile_command(r"'\e'", symbol='exec') + compiler(r"'\e'", "", 'exec') - def test_incomplete_warning(self): + @subTests('compiler', WRAPPING_COMPILERS) + def test_incomplete_warning(self, compiler): with warnings.catch_warnings(record=True) as w: warnings.simplefilter('always') - self.assertIncomplete("'\\e' + (") + compiler("'\\e' + (") self.assertEqual(w, []) @unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: 0 != 1 - def test_invalid_warning(self): + @subTests('compiler', RAW_COMPILERS) + def test_raw_raises_error(self, compiler): + warnings_cm = warnings_helper.check_warnings( + ('"\\\\e" is an invalid esceape sequence', SyntaxWarning) + ) + with self.assertRaises(SyntaxError), warnings_cm as w: + compiler("'\\e' + (", "", 'single') + self.assertEqual(len(w.warnings), 1) + + @unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: 0 != 1 + @subTests('compiler', COMPILERS) + def test_invalid_warning(self, compiler): with warnings.catch_warnings(record=True) as w: warnings.simplefilter('always') - self.assertInvalid("'\\e' 1") + self.assertInvalid("'\\e' 1", compiler=compiler) self.assertEqual(len(w), 1) - self.assertEqual(w[0].category, SyntaxWarning) - self.assertRegex(str(w[0].message), 'invalid escape sequence') - self.assertEqual(w[0].filename, '') - - def assertSyntaxErrorMatches(self, code, message): - with self.subTest(code): - with self.assertRaisesRegex(SyntaxError, message): - compile_command(code, symbol='exec') - - def test_syntax_errors(self): - self.assertSyntaxErrorMatches( - dedent("""\ + for warning in w: + self.assertEqual(warning.category, SyntaxWarning) + self.assertRegex(str(warning), 'invalid escape sequence') + self.assertEqual(warning.filename, '') + + @subTests('compiler', COMPILERS) + def test_syntax_errors(self, compiler): + code = dedent("""\ def foo(x,x): pass - """), "duplicate argument 'x' in function definition") - + """) + message = "duplicate argument 'x' in function definition" + with self.assertRaisesRegex(SyntaxError, message): + compiler(code, "", 'exec') + + @subTests('compiler', RAW_COMPILERS) + def test_future_imports(self, compiler): + original_flags = compiler.flags + compiler('from __future__ import annotations', "", 'single') + self.assertGreater(compiler.flags, original_flags) + # reset flags to ensure test has no side-effects + compiler.flags = original_flags if __name__ == "__main__": diff --git a/Lib/test/test_compileall.py b/Lib/test/test_compileall.py index bb178d487e5..be5e7d7feee 100644 --- a/Lib/test/test_compileall.py +++ b/Lib/test/test_compileall.py @@ -550,6 +550,24 @@ def temporary_pycache_prefix(self): finally: sys.pycache_prefix = old_prefix + @contextlib.contextmanager + def no_pycache_prefix(self): + """Ignore any ambient pycache prefix for the duration of the test. + + Some tests assume bytecode is written next to the source in a + __pycache__ directory. When the test suite is run with + PYTHONPYCACHEPREFIX set, neutralize it both in this process (used by + cache_from_source) and in any spawned subprocesses. + """ + old_prefix = sys.pycache_prefix + sys.pycache_prefix = None + try: + with os_helper.EnvironmentVarGuard() as env: + env.unset('PYTHONPYCACHEPREFIX') + yield + finally: + sys.pycache_prefix = old_prefix + def _get_run_args(self, args): return [*support.optim_args_from_interpreter_flags(), '-S', '-m', 'compileall', @@ -648,15 +666,16 @@ def test_legacy_paths(self): def test_multiple_runs(self): # Bug 8527 reported that multiple calls produced empty # __pycache__/__pycache__ directories. - self.assertRunOK('-q', self.pkgdir) - # Verify the __pycache__ directory contents. - self.assertTrue(os.path.exists(self.pkgdir_cachedir)) - cachecachedir = os.path.join(self.pkgdir_cachedir, '__pycache__') - self.assertFalse(os.path.exists(cachecachedir)) - # Call compileall again. - self.assertRunOK('-q', self.pkgdir) - self.assertTrue(os.path.exists(self.pkgdir_cachedir)) - self.assertFalse(os.path.exists(cachecachedir)) + with self.no_pycache_prefix(): + self.assertRunOK('-q', self.pkgdir) + # Verify the __pycache__ directory contents. + self.assertTrue(os.path.exists(self.pkgdir_cachedir)) + cachecachedir = os.path.join(self.pkgdir_cachedir, '__pycache__') + self.assertFalse(os.path.exists(cachecachedir)) + # Call compileall again. + self.assertRunOK('-q', self.pkgdir) + self.assertTrue(os.path.exists(self.pkgdir_cachedir)) + self.assertFalse(os.path.exists(cachecachedir)) @without_source_date_epoch # timestamp invalidation test def test_force(self): @@ -729,10 +748,13 @@ def test_symlink_loop(self): script_helper.make_pkg(pkg) os.symlink('.', os.path.join(pkg, 'evil')) os.symlink('.', os.path.join(pkg, 'evil2')) - self.assertRunOK('-q', self.pkgdir) - self.assertCompiled(os.path.join( - self.pkgdir, 'spam', 'evil', 'evil2', '__init__.py' - )) + # This relies on the __pycache__ layout (shared across the symlinked + # paths), so neutralize any ambient PYTHONPYCACHEPREFIX. + with self.no_pycache_prefix(): + self.assertRunOK('-q', self.pkgdir) + self.assertCompiled(os.path.join( + self.pkgdir, 'spam', 'evil', 'evil2', '__init__.py' + )) def test_quiet(self): noisy = self.assertRunOK(self.pkgdir) @@ -819,13 +841,16 @@ def test_include_on_stdin(self): f2 = script_helper.make_script(self.pkgdir, 'f2', '') f3 = script_helper.make_script(self.pkgdir, 'f3', '') f4 = script_helper.make_script(self.pkgdir, 'f4', '') - p = script_helper.spawn_python(*(self._get_run_args(()) + ['-i', '-'])) - p.stdin.write((f3+os.linesep).encode('ascii')) - script_helper.kill_python(p) - self.assertNotCompiled(f1) - self.assertNotCompiled(f2) - self.assertCompiled(f3) - self.assertNotCompiled(f4) + # spawn_python() runs with -E, ignoring PYTHONPYCACHEPREFIX, so make + # cache_from_source() in this process agree by neutralizing it too. + with self.no_pycache_prefix(): + p = script_helper.spawn_python(*(self._get_run_args(()) + ['-i', '-'])) + p.stdin.write((f3+os.linesep).encode('ascii')) + script_helper.kill_python(p) + self.assertNotCompiled(f1) + self.assertNotCompiled(f2) + self.assertCompiled(f3) + self.assertNotCompiled(f4) def test_compiles_as_much_as_possible(self): bingfn = script_helper.make_script(self.pkgdir, 'bing', 'syntax(error') diff --git a/Lib/test/test_dataclasses/__init__.py b/Lib/test/test_dataclasses/__init__.py index 96f42183296..962d75abc2d 100644 --- a/Lib/test/test_dataclasses/__init__.py +++ b/Lib/test/test_dataclasses/__init__.py @@ -2754,6 +2754,55 @@ def __eq__(self, other): self.assertEqual(C(1), 5) self.assertNotEqual(C(1), 1) + def test_eq_field_by_field(self): + @dataclasses.dataclass + class Point: + x: int + y: int + + p1 = Point(1, 2) + p2 = Point(1, 2) + p3 = Point(2, 1) + self.assertEqual(p1, p2) + self.assertNotEqual(p1, p3) + + def test_eq_type_check(self): + @dataclasses.dataclass + class A: + x: int + + @dataclasses.dataclass + class B: + x: int + + a = A(1) + b = B(1) + self.assertNotEqual(a, b) + + def test_eq_custom_field(self): + class AlwaysEqual(int): + def __eq__(self, other): + return True + + @dataclasses.dataclass + class Foo: + x: AlwaysEqual + y: int + + f1 = Foo(AlwaysEqual(1), 2) + f2 = Foo(AlwaysEqual(2), 2) + self.assertEqual(f1, f2) + + def test_eq_nan_field(self): + @dataclasses.dataclass + class D: + x: float + + nan = float('nan') + d1 = D(nan) + d2 = D(nan) + self.assertNotEqual(d1, d2) + class TestOrdering(unittest.TestCase): def test_functools_total_ordering(self): @@ -3290,6 +3339,47 @@ def test_non_frozen_normal_derived(self): class D: x: int y: int = 10 + z: int = 1 + + @property + def readonly(self) -> int: + return self.x + + @property + def prop(self) -> int: + return self.z + + @prop.setter + def prop(self, val: int) -> None: + object.__setattr__(self, 'z', val) + + @prop.deleter + def prop(self) -> None: + object.__setattr__(self, 'z', 0) + + d = D(5) + self.assertEqual(d.x, 5) + self.assertEqual(d.y, 10) + self.assertEqual(d.z, 1) + self.assertEqual(d.readonly, 5) + self.assertEqual(d.prop, 1) + + with self.assertRaises(FrozenInstanceError): + d.x = 5 + with self.assertRaises(FrozenInstanceError): + d.readonly = 5 + with self.assertRaises(FrozenInstanceError): + d.z = 5 + with self.assertRaises(FrozenInstanceError): + d.prop = 5 + with self.assertRaises(FrozenInstanceError): + del d.prop + + self.assertEqual(d.x, 5) + self.assertEqual(d.y, 10) + self.assertEqual(d.z, 1) + self.assertEqual(d.readonly, 5) + self.assertEqual(d.prop, 1) class S(D): pass @@ -3297,16 +3387,40 @@ class S(D): s = S(3) self.assertEqual(s.x, 3) self.assertEqual(s.y, 10) + self.assertEqual(s.z, 1) + self.assertEqual(s.readonly, 3) + self.assertEqual(s.prop, 1) + # Can set new attrs: s.cached = True + self.assertTrue(s.cached) + # Can mutate them: + s.cached = False + self.assertFalse(s.cached) + + # Can also change writable properties: + with self.assertRaisesRegex( + AttributeError, + 'object has no setter', + ) as cm: + s.readonly = 5 + self.assertNotIsInstance(cm.exception, FrozenInstanceError) + s.prop = 2 + self.assertEqual(s.x, 3) + self.assertEqual(s.readonly, 3) + self.assertEqual(s.prop, 2) + self.assertEqual(s.z, 2) # But can't change the frozen attributes. with self.assertRaises(FrozenInstanceError): s.x = 5 with self.assertRaises(FrozenInstanceError): s.y = 5 + with self.assertRaises(FrozenInstanceError): + s.z = 5 self.assertEqual(s.x, 3) self.assertEqual(s.y, 10) - self.assertEqual(s.cached, True) + self.assertEqual(s.z, 2) + self.assertIs(s.cached, False) with self.assertRaises(FrozenInstanceError): del s.x @@ -3314,11 +3428,26 @@ class S(D): with self.assertRaises(FrozenInstanceError): del s.y self.assertEqual(s.y, 10) + with self.assertRaisesRegex( + AttributeError, + 'object has no deleter', + ) as cm: + del s.readonly + self.assertNotIsInstance(cm.exception, FrozenInstanceError) + self.assertEqual(s.x, 3) + self.assertEqual(s.readonly, 3) del s.cached self.assertNotHasAttr(s, 'cached') - with self.assertRaises(AttributeError) as cm: + with self.assertRaisesRegex( + AttributeError, + "object has no attribute 'cached'", + ) as cm: del s.cached self.assertNotIsInstance(cm.exception, FrozenInstanceError) + del s.prop + self.assertEqual(s.z, 0) + self.assertEqual(s.prop, 0) + del s.prop def test_non_frozen_normal_derived_from_empty_frozen(self): @dataclass(frozen=True) diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py index da919fae6e4..fc39240fb19 100644 --- a/Lib/test/test_descr.py +++ b/Lib/test/test_descr.py @@ -815,6 +815,15 @@ class X(C, int()): class X(int(), C): pass + @unittest.skipIf(_testcapi is None, 'need the _testcapi module') + def test_type_with_null_new_metaclass(self): + metaclass = _testcapi.HeapCTypeMetaclassNullNew + base = _testcapi.pytype_fromspec_meta(metaclass) + + # Exercise type_new's metaclass selection path, not a direct call. + with self.assertRaisesRegex(TypeError, r"cannot create '.*' instances"): + type("Derived", (base,), {}) + def test_module_subclasses(self): # Testing Python subclass of module... log = [] @@ -3698,6 +3707,7 @@ def f(a): return a self.assertEqual(ba, b'abc\xbd?') @unittest.skip("TODO: RUSTPYTHON; rustpython segmentation fault") + @support.skip_if_huge_c_stack() @support.skip_wasi_stack_overflow() @support.skip_emscripten_stack_overflow() def test_recursive_call(self): @@ -4914,6 +4924,7 @@ class Thing: # CALL_METHOD_DESCRIPTOR_O deque.append(thing, thing) + @support.skip_if_huge_c_stack() @support.skip_emscripten_stack_overflow() @support.skip_wasi_stack_overflow() def test_repr_as_str(self): diff --git a/Lib/test/test_dictviews.py b/Lib/test/test_dictviews.py index 4e5b9184025..024d20a63a1 100644 --- a/Lib/test/test_dictviews.py +++ b/Lib/test/test_dictviews.py @@ -2,7 +2,9 @@ import copy import pickle import unittest -from test.support import skip_emscripten_stack_overflow, skip_wasi_stack_overflow, exceeds_recursion_limit +from test.support import (skip_emscripten_stack_overflow, + skip_wasi_stack_overflow, run_with_limited_c_stack, + exceeds_recursion_limit) class DictSetTest(unittest.TestCase): @@ -278,6 +280,7 @@ def test_recursive_repr(self): self.assertIsInstance(r, str) @unittest.skip("TODO: RUSTPYTHON; segfault") + @run_with_limited_c_stack() @skip_wasi_stack_overflow() @skip_emscripten_stack_overflow() def test_deeply_nested_repr(self): diff --git a/Lib/test/test_enumerate.py b/Lib/test/test_enumerate.py index 5cb54cff9b7..c8b85fe8692 100644 --- a/Lib/test/test_enumerate.py +++ b/Lib/test/test_enumerate.py @@ -3,8 +3,11 @@ import sys import pickle import gc +import threading + from test import support +from test.support import threading_helper class G: 'Sequence using __getitem__' @@ -292,5 +295,28 @@ def enum(self, iterable, start=sys.maxsize + 1): (sys.maxsize+3,'c')] +@threading_helper.requires_working_threading() +class TestThreadSafety(EnumerateStartTestCase): + def test_thread_safety_while_iterating(self): + # gh-153932: calling reduce while iterating should pass with TSAN + + en = enumerate(range(10_000)) + stop = threading.Event() + + def advance(): + for _ in en: + pass + stop.set() + + def read(): + while not stop.is_set(): + en.__reduce__() + + threads = [threading.Thread(target=advance), threading.Thread(target=read)] + + with threading_helper.start_threads(threads): + pass + + if __name__ == "__main__": unittest.main() diff --git a/Lib/test/test_memoryio.py b/Lib/test/test_memoryio.py index 7ad3aa8a527..43045a981de 100644 --- a/Lib/test/test_memoryio.py +++ b/Lib/test/test_memoryio.py @@ -869,7 +869,10 @@ def test_setstate(self): @support.cpython_only def test_sizeof(self): - basesize = support.calcobjsize('P2n2Pn') + if support.Py_GIL_DISABLED: + basesize = support.calcobjsize('P2n2Pni') + else: + basesize = support.calcobjsize('P2n2Pn') check = self.check_sizeof self.assertEqual(object.__sizeof__(io.BytesIO()), basesize) check(io.BytesIO(), basesize ) @@ -925,7 +928,6 @@ def test_cow_mutable(self): def test_flags(self): return super().test_flags() - class CStringIOTest(PyStringIOTest): ioclass = io.StringIO UnsupportedOperation = io.UnsupportedOperation @@ -996,6 +998,7 @@ def __str__(self): def test_flags(self): return super().test_flags() + class CStringIOPickleTest(PyStringIOPickleTest): UnsupportedOperation = io.UnsupportedOperation @@ -1005,5 +1008,6 @@ def __new__(cls, *args, **kwargs): def __init__(self, *args, **kwargs): pass + if __name__ == '__main__': unittest.main() diff --git a/Lib/test/test_ordered_dict.py b/Lib/test/test_ordered_dict.py index ae7935ac07e..01678de56a9 100644 --- a/Lib/test/test_ordered_dict.py +++ b/Lib/test/test_ordered_dict.py @@ -878,6 +878,41 @@ def side_effect(self): self.assertDictEqual(dict1, dict.fromkeys((0, 4.2))) self.assertDictEqual(dict2, dict.fromkeys((0, Key(), 4.2))) + @unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: RuntimeError not raised by copy + def test_issue148660_copy_clear_in_key_eq(self): + # gh-148660: od.copy() must not crash when a key's __eq__ clears od + # while copy() is inserting into the new dict. + armed = False + calls = 0 + class Key: + def __hash__(self): + return 1 + def __eq__(self, other): + nonlocal calls + if armed: + calls += 1 + if calls == 2: + od.clear() + return self is other + od = self.OrderedDict() + od[Key()] = "v1" + od[Key()] = "v2" + armed = True + msg = "OrderedDict mutated during iteration" + self.assertRaisesRegex(RuntimeError, msg, od.copy) + + @unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: RuntimeError not raised by copy + def test_issue148660_copy_clear_in_subclass_getitem(self): + # gh-148660: od.copy() must not crash when a subclass __getitem__ + # clears od. + class OD(self.OrderedDict): + def __getitem__(self, key): + od.clear() + return "v" + od = OD([(1, "v1"), (2, "v2")]) + msg = "OrderedDict mutated during iteration" + self.assertRaisesRegex(RuntimeError, msg, od.copy) + @unittest.skipUnless(c_coll, 'requires the C version of the collections module') class CPythonOrderedDictTests(OrderedDictTests, diff --git a/Lib/test/test_random.py b/Lib/test/test_random.py index 0217ebd132b..c231126d58c 100644 --- a/Lib/test/test_random.py +++ b/Lib/test/test_random.py @@ -1074,6 +1074,21 @@ def test_avg_std(self): self.assertAlmostEqual(s2/(N-1), sigmasqrd, places=2, msg='%s%r' % (variate.__name__, args)) + def test_binomialvariate_log_zero(self): + # gh-149222: Variety random() return 0.0 no input Error + with unittest.mock.patch.object(random.Random, 'random', side_effect=[0.0] + [0.5] * 20): + result = random.binomialvariate(10, 0.5) + self.assertIsInstance(result, int) + self.assertIn(result, range(11)) + + def test_binomialvariate_btrs_random_zero(self): + for p, expected in ((0.25, 25), (0.75, 75)): + with self.subTest(p=p): + g = random.Random() + with unittest.mock.patch.object( + g, 'random', side_effect=(0.0, 0.5, 0.5)): + self.assertEqual(g.binomialvariate(100, p), expected) + def test_constant(self): g = random.Random() N = 100 diff --git a/Lib/test/test_robotparser.py b/Lib/test/test_robotparser.py index cd1477037e9..1ec64da064d 100644 --- a/Lib/test/test_robotparser.py +++ b/Lib/test/test_robotparser.py @@ -188,6 +188,8 @@ def test_request_rate(self): parsed_request_rate.seconds, self.request_rate.seconds ) + else: + self.assertIsNone(parsed_request_rate) class EmptyFileTest(BaseRequestRateTest, unittest.TestCase): @@ -246,6 +248,32 @@ class InvalidCrawlDelayTest(BaseRobotTest, unittest.TestCase): bad = [] +class NonDecimalDigitsTest(BaseRequestRateTest, unittest.TestCase): + # Non-decimal Unicode digits pass str.isdigit() but int() rejects + # them, so the directive must be silently ignored, not raise. + robots_txt = """\ +User-Agent: * +Disallow: /tmp/ +Crawl-delay: ² +Request-rate: ²/5 + """ + good = ['/foo.html'] + bad = ['/tmp/'] + crawl_delay = None + request_rate = None + + +class NonDecimalDenominatorTest(BaseRequestRateTest, unittest.TestCase): + robots_txt = """\ +User-agent: * +Disallow: /tmp/ +Request-rate: 5/² + """ + good = ['/foo.html'] + request_rate = None + bad = ['/tmp/'] + + class AnotherInvalidRequestRateTest(BaseRobotTest, unittest.TestCase): # also test that Allow and Diasallow works well with each other robots_txt = """\ diff --git a/Lib/test/test_urllib.py b/Lib/test/test_urllib.py index 2dd739b77b8..1e5f79998e7 100644 --- a/Lib/test/test_urllib.py +++ b/Lib/test/test_urllib.py @@ -467,6 +467,25 @@ def test_redirect_limit_independent(self): finally: self.unfakehttp() + def test_http_error_attribute_values(self): + hdrs = { + "Authorization": "Bearer foobar", + "Accept": "application/json" + } + err = urllib.error.HTTPError("http://something", 404, "foo", hdrs, None) + self.assertEqual(err.filename, "http://something") + self.assertEqual(err.code, 404) + self.assertEqual(err.msg, "foo") + self.assertEqual(err.reason, "foo") + self.assertEqual(err.hdrs, hdrs) + self.assertEqual(err.headers, hdrs) + err.close() + + def test_http_error_default_fp(self): + err = urllib.error.HTTPError("http://something", 404, "foo", {}, None) + self.assertIsInstance(err.fp, io.BytesIO) + err.close() + def test_empty_socket(self): # urlopen() raises OSError if the underlying socket does not send any # data. (#1680230) @@ -513,6 +532,11 @@ def test_ftp_nonexisting(self): self.assertFalse(e.exception.filename) self.assertTrue(e.exception.reason) + def test_url_error_stringified(self): + reason = 'sixseven' + err = urllib.error.URLError(reason) + self.assertEqual(str(err), f'') + class urlopen_DataTests(unittest.TestCase): """Test urlopen() opening a data URL.""" diff --git a/Lib/test/test_urllib2.py b/Lib/test/test_urllib2.py index 7d7f2fa00d3..eeea9cda2f4 100644 --- a/Lib/test/test_urllib2.py +++ b/Lib/test/test_urllib2.py @@ -963,6 +963,35 @@ def test_http(self): self.assertEqual(req.unredirected_hdrs["Host"], "baz") self.assertEqual(req.unredirected_hdrs["Spam"], "foo") + def test_http_header_priority(self): + # gh-47005: regular headers set via add_header() must override + # unredirected headers with the same name in do_open(), consistent + # with get_header() and header_items(). + cases = [ + ("Content-Type", "application/json", "application/x-www-form-urlencoded"), + ("Content-Length", "99", "0"), + ("Host", "override.example.com", "internal.example.com"), + ("Authorization", "Bearer user-token", "Basic stale="), + ("Cookie", "a=1", "b=2"), + ("User-Agent", "MyApp/1.0", "Python-urllib/test"), + ] + h = urllib.request.AbstractHTTPHandler() + h.parent = MockOpener() + + for key, regular, unredirected in cases: + req = Request("http://example.com/", headers={key: regular}) + req.timeout = None + req.add_unredirected_header(key, unredirected) + + http = MockHTTPClass() + h.do_open(http, req) + + sent_headers = dict(http.req_headers) + self.assertEqual(sent_headers[key], regular) + # key is capitalized by add_header() and add_unredirected_header() calls + self.assertEqual(req.get_header(key.capitalize()), regular) + self.assertEqual(dict(req.header_items())[key.capitalize()], regular) + def test_http_body_file(self): # A regular file - chunked encoding is used unless Content Length is # already set. diff --git a/Lib/test/test_urlparse.py b/Lib/test/test_urlparse.py index b2bde5a9b1d..98f0b190ee7 100644 --- a/Lib/test/test_urlparse.py +++ b/Lib/test/test_urlparse.py @@ -1426,15 +1426,20 @@ def test_splitting_bracketed_hosts(self): self.assertEqual(p1.username, 'user') self.assertEqual(p1.path, '/path') self.assertEqual(p1.port, 1234) - p2 = urllib.parse.urlsplit('scheme://user@[0439:23af:2309::fae7%test]/path?query') - self.assertEqual(p2.hostname, '0439:23af:2309::fae7%test') + p2 = urllib.parse.urlsplit('scheme://user@[V6a.ip]:1234/path?query') + self.assertEqual(p2.hostname, 'v6a.ip') self.assertEqual(p2.username, 'user') self.assertEqual(p2.path, '/path') - self.assertIs(p2.port, None) - p3 = urllib.parse.urlsplit('scheme://user@[0439:23af:2309::fae7:1234:192.0.2.146%test]/path?query') - self.assertEqual(p3.hostname, '0439:23af:2309::fae7:1234:192.0.2.146%test') + self.assertEqual(p2.port, 1234) + p3 = urllib.parse.urlsplit('scheme://user@[0439:23af:2309::fae7%test]/path?query') + self.assertEqual(p3.hostname, '0439:23af:2309::fae7%test') self.assertEqual(p3.username, 'user') self.assertEqual(p3.path, '/path') + self.assertIs(p3.port, None) + p4 = urllib.parse.urlsplit('scheme://user@[0439:23af:2309::fae7:1234:192.0.2.146%test]/path?query') + self.assertEqual(p4.hostname, '0439:23af:2309::fae7:1234:192.0.2.146%test') + self.assertEqual(p4.username, 'user') + self.assertEqual(p4.path, '/path') def test_port_casting_failure_message(self): message = "Port could not be cast to integer value as 'oracle'" diff --git a/Lib/urllib/parse.py b/Lib/urllib/parse.py index a651e815ddc..c9f8a33e72c 100644 --- a/Lib/urllib/parse.py +++ b/Lib/urllib/parse.py @@ -461,8 +461,8 @@ def _check_bracketed_netloc(netloc): # Valid bracketed hosts are defined in # https://www.rfc-editor.org/rfc/rfc3986#page-49 and https://url.spec.whatwg.org/ def _check_bracketed_host(hostname): - if hostname.startswith('v'): - if not re.match(r"\Av[a-fA-F0-9]+\..+\z", hostname): + if hostname.startswith(('v', 'V')): + if not re.match(r"\A[vV][a-fA-F0-9]+\..+\z", hostname): raise ValueError(f"IPvFuture address is invalid") else: ip = ipaddress.ip_address(hostname) # Throws Value Error if not IPv6 or IPv4 diff --git a/Lib/urllib/request.py b/Lib/urllib/request.py index 8d7470a2273..a8f40100b7b 100644 --- a/Lib/urllib/request.py +++ b/Lib/urllib/request.py @@ -1291,8 +1291,7 @@ def do_open(self, http_class, req, **http_conn_args): h.set_debuglevel(self._debuglevel) headers = dict(req.unredirected_hdrs) - headers.update({k: v for k, v in req.headers.items() - if k not in headers}) + headers.update(req.headers) # TODO(jhylton): Should this be redesigned to handle # persistent connections? diff --git a/Lib/urllib/robotparser.py b/Lib/urllib/robotparser.py index e70eae80036..d267ed00345 100644 --- a/Lib/urllib/robotparser.py +++ b/Lib/urllib/robotparser.py @@ -65,9 +65,17 @@ def read(self): f = urllib.request.urlopen(self.url) except urllib.error.HTTPError as err: if err.code in (401, 403): + # If access to robot.txt has the status Unauthorized/Forbidden, + # then most likely this applies to the entire site. self.disallow_all = True - elif err.code >= 400 and err.code < 500: + elif 400 <= err.code < 500: + # RFC 9309, Section 2.3.1.3: the crawler MAY access any + # resources on the server. self.allow_all = True + elif 500 <= err.code < 600: + # RFC 9309, Section 2.3.1.4: the crawler MUST assume + # complete disallow. + self.disallow_all = True err.close() else: raw = f.read() @@ -135,15 +143,15 @@ def parse(self, lines): # before trying to convert to int we need to make # sure that robots.txt has valid syntax otherwise # it will crash - if line[1].strip().isdigit(): + if line[1].strip().isdecimal(): entry.delay = int(line[1]) state = 2 elif line[0] == "request-rate": if state != 0: numbers = line[1].split('/') # check if all values are sane - if (len(numbers) == 2 and numbers[0].strip().isdigit() - and numbers[1].strip().isdigit()): + if (len(numbers) == 2 and numbers[0].strip().isdecimal() + and numbers[1].strip().isdecimal()): entry.req_rate = RequestRate(int(numbers[0]), int(numbers[1])) state = 2 elif line[0] == "sitemap": From b0b4d2f8a9dd997a8f8743d2b01cbde00976de14 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" <69878+youknowone@users.noreply.github.com> Date: Fri, 28 Aug 2026 16:20:39 +0900 Subject: [PATCH 4/5] lock: add an opt-in rwlock that detaches before it blocks (#8556) * lock: add an opt-in rwlock that detaches before it blocks A thread blocked acquiring a lock reaches no safepoint, so stop-the-world cannot stop it, and the lock it waits for is routinely one a thread the requester already suspended is holding. `RawDetachingRwLock` wraps the raw rwlock and hands the wait for a contended acquire to a hook that leaves the interpreter first; an acquire that takes the lock on its first try does not reach the hook. The vm installs the hook during interpreter init and implements it with `allow_threads`. The wait ends with the lock acquired while detached, so re-attaching can park the thread holding it. That is only safe where nothing reachable from a stop-the-world section takes the same lock, so it is opt-in per lock: `PyDetachingRwLock` is a separate type from `PyRwLock`, and `Traverse` is not implemented for it, so a payload holding one cannot derive `Traverse`. `PyByteArray::inner` takes it. `BorrowedValue`/`BorrowedValueMut` gain the matching mapped-guard variants. `a_thread_blocked_on_a_lock_does_not_stall_stop_the_world` blocks an interpreter thread on a `PyDetachingRwLock` and asserts stop-the-world still completes, running the stop on its own thread with a timeout so a stop that never completes fails rather than hangs. Without the hook installed it fails on the 10 s timeout; with it, it passes in 0.07 s. Assisted-by: Claude * lock: stop detaching where the lock may already be held `upgrade` runs with the upgradable lock held, and `lock_shared_recursive` may be the re-entrant take of a lock the calling thread holds; detaching there parks a thread holding the lock, which is what this type documents it must not do. They forward to the wrapped lock instead. `lock_upgradable` starts from holding nothing, but nothing takes an upgradable read of one of these, so it forwards too. `lock_shared` and `lock_exclusive` still detach. Also narrow two claims the comments overstated. Not implementing `Traverse` enforces the opt-in rule only against collections, not against the other things that stop the world. And the requester exemption the hook relies on is wider than `_PyEval_StopTheWorld` gives, so it is a local invariant. Assisted-by: Claude * os.readinto: read aside when the fd can wait `readinto` held the destination's write lock for the whole call, including the `read(2)` inside `allow_threads`. On a pipe, socket or terminal that read returns only when the other end writes, so a thread reaching the same object waited on a lock for an unbounded time, reaching no safepoint while it did. Take the fd that answers without waiting directly, as before, and otherwise read into scratch and take the lock only for the copy. This is what `FileIO.readinto`, `socket.recv_into` and `socket.recvfrom_into` already do; `os.readinto` was the site left over. The EINTR retry moves to `read_into_slice`, unchanged. Assisted-by: Claude * fcntl: detach around the calls that wait Every call in this module ran with the thread attached, so a thread inside one reached no safepoint until it returned. `flock(LOCK_EX)` and `lockf(F_LOCK)` return when whoever holds the lock gives it up, which may be never, and an ioctl on a terminal or socket answers when the device is ready to; the world could not be stopped for that long. `fcntl_fcntl_impl`, `fcntl_ioctl_impl`, `fcntl_flock_impl` and `fcntl_lockf_impl` all release around the call. `ioctl` with `mutate_flag` additionally held the target's write lock for the whole call, so a thread reaching the same object waited on a lock for as long as the device took. Its bytes now go in and come back through a buffer of our own, as `fcntl_ioctl_impl` copies through one of its own for anything up to IOCTL_BUFSZ. The export the argument holds is what keeps the length from changing in between. test_fcntl and test_ioctl pass. Assisted-by: Claude * openssl: read aside instead of holding the caller's buffer locked `SSLSocket.read` wrote straight into the destination buffer, holding the lock that reaching its bytes takes for the whole call. That read returns when the peer writes, which may be never, so a thread touching the same object waited on that lock for as long as the peer took, reaching no safepoint while it did. Read into a buffer of our own and take the destination's lock only for the copy. The rustls backend already reads this way, and `_ssl__SSLSocket_read_impl` works from a `Py_buffer` whose critical section ended before the read. `test_ssl` on this backend fails the same 16 tests before and after. Assisted-by: Claude * vm: add the inverse of allow_threads for callbacks A call that detaches hands the thread to stop-the-world, which counts it as parked. A callback that reaches Python from inside such a call would then run on a thread the requester believes is stopped, and nothing in the vm could stop it: `attach_thread` and `detach_thread` are private to `thread.rs`, and `allow_threads` only goes the one way. `attach_for_callback` attaches for the duration of the closure and returns the thread to where it was, the way `PyGILState_Ensure` and `PyGILState_Release` bracket `_servername_callback`. It tests for "not ATTACHED" rather than for DETACHED, so a thread a stop-the-world has already moved to SUSPENDED routes through `attach_thread` and parks there until the world starts again. `a_callback_inside_a_detached_call_waits_for_the_world` stops the world with a thread detached, then turns that thread loose at a callback and asserts it does not run until the world starts. With the transition disabled it fails. Assisted-by: Claude * openssl: attach around the callbacks that run Python `_servername_callback` and `_msg_callback` reach the interpreter from inside an SSL call -- they take a reference to the Python callback, build arguments and call it. That call is about to detach, and running Python from a detached thread runs it on a thread a stop-the-world requester counts as parked. `_servername_callback` opens with `PyGILState_Ensure()` for the same reason. Both now rejoin the interpreter for the duration of the callback and give the thread back afterwards. The reference to the callback moves inside that section, since taking it is itself an interpreter operation; the check for whether a callback is set at all stays outside, so a socket with none set never reaches the interpreter. No behavior change yet: nothing detaches around these calls, so `attach_for_callback` finds the thread already attached and just runs. Assisted-by: Claude * openssl: detach around the SSL calls that wait On a socket with a timeout the wait lands in `select` -> `sock_wait`, which already detaches. On a blocking socket there is no such return: `SSL_read` blocks in `recv(2)` through `impl Read for &PySocket`, with the thread attached and the connection's write lock held, so the world could not be stopped for as long as the peer stayed silent. `SSL_do_handshake`, `SSL_read_ex`, `SSL_write_ex` and `SSL_shutdown` all run between `Py_BEGIN_ALLOW_THREADS` and `Py_END_ALLOW_THREADS`; these now do too, in both socket and BIO mode as there. The connection lock stays held across the call and so becomes a detaching lock: a thread reaching the same socket gives up its interpreter rather than wait for it attached. `connection` is `#[pytraverse(skip)]`, so a collection does not walk into it and never takes that lock -- which is the rule for opting in, though the skip is what supplies it here rather than the missing `Traverse`. A server that completes a handshake and then says nothing used to deadlock the whole process: the collector suspended the main thread at a safepoint and then waited forever for the reader, so even the test's own timeout could not fire. It now collects in 3 ms. Verified separately that a Python `_msg_callback` still runs from inside the handshake -- 920 invocations across 40 handshakes with a collector looping, five runs clean. test_ssl on this backend fails the same 8 tests before and after, by name, and openssl.rs draws no clippy warning it did not draw before. Assisted-by: Claude * lock: assert the opt-in rule against every stop-the-world section Not implementing `Traverse` for `PyDetachingRwLock` states the rule to a collection: a payload holding one cannot derive `Traverse`, so a collection cannot walk into it. It says nothing to the other sections that stop the world -- fork, traceback dumps, frame enumeration -- and `#[pytraverse(skip)]` steps around it besides, which is how `_SSLSocket.connection` holds one. For those the rule was a comment. `set_world_stopped` records, on the one thread still running inside a stopped world, that it is that thread; `lock_shared` and `lock_exclusive` assert it is not set. A section that took one of these could block on a lock a parked thread holds and only that section can release, which is the deadlock the rule exists to prevent. Debug builds only; release builds track nothing. Nothing in the tree trips it: 180 rounds of collect, `sys._current_frames`, `faulthandler.dump_traceback` and 18 forks with four threads churning bytearrays, plus test_gc/test_bytes/test_threading/test_memoryview/test_buffer on a debug build, all clean. `taking_one_while_stopping_the_world_is_caught` takes one with the flag set and asserts the panic, so the guard is not dead code. Assisted-by: Claude * clippy: drop std_instead_of_core suppressions 1.98 no longer reports `std::io` items for the lint, so the six `expect` attributes for it are unfulfilled. Also drops `from_iter_instead_of_collect` from the workspace lint table, which 1.98 removed. Assisted-by: Claude * lock: spell out why the detaching rwlock exists States the motivation as the three-thread cycle it avoids: how a stop reaches a DETACHED thread but not an ATTACHED one, why a thread blocked on a lock reaches no safepoint, and why the waiter rather than the holder gives way. Puts it on `RawDetachingRwLock`, which is public and so rendered; the module doc is private and keeps only the hook description. Assisted-by: Claude * fcntl: raise the error the call returned The six detached calls discarded the `io::Error` and read `errno` again after `allow_threads`, which re-attaches in between and can park on the way. `lockf` already converts the returned error; these now do too. Assisted-by: Claude * openssl: allocate the read buffer through the vm Without a `buffer` argument `read_len` is whatever non-negative size the caller passed, so `vec![0u8; read_len]` aborts on allocation failure. `new_zeroed_bytes` raises `MemoryError` instead, as the two other reads in this file already do. Assisted-by: Claude * lock: leave no attached blocking acquire on a detaching lock `lock_upgradable` starts from holding nothing, so it detaches like `lock_shared`. A recursive read cannot: it may be the re-entrant take of a lock this thread holds, so detaching there parks a thread holding it, and staying attached stalls stop-the-world. `RawRwLockRecursive` is no longer implemented, which removes `read_recursive` from these locks; nothing took one. The assertion test no longer replaces the panic hook, which is process-wide and was suppressing panic output from whatever else ran beside it. Assisted-by: Claude * vm: wait for the actual block in the stop-the-world test The worker signalled before `read()` and the test slept 50ms, so a stop could complete with nothing blocked on the lock and the test would pass having checked nothing. It now publishes its thread id from inside the interpreter and the test waits for that slot to reach DETACHED, bounded so an acquire that never detaches fails rather than hangs. Assisted-by: Claude --- Cargo.toml | 1 - crates/common/src/borrow.rs | 16 +- crates/common/src/lock.rs | 16 ++ crates/common/src/lock/detaching.rs | 365 ++++++++++++++++++++++++++++ crates/stdlib/src/fcntl.rs | 52 ++-- crates/stdlib/src/openssl.rs | 329 ++++++++++++++----------- crates/vm/src/builtins/bytearray.rs | 29 +-- crates/vm/src/stdlib/os.rs | 47 +++- crates/vm/src/vm/interpreter.rs | 183 ++++++++++++++ crates/vm/src/vm/mod.rs | 15 ++ crates/vm/src/vm/thread.rs | 76 ++++++ 11 files changed, 937 insertions(+), 192 deletions(-) create mode 100644 crates/common/src/lock/detaching.rs diff --git a/Cargo.toml b/Cargo.toml index 3b489a88687..74bc202e2b2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -398,7 +398,6 @@ explicit_iter_loop = "warn" filter_map_next = "warn" flat_map_option = "warn" format_collect = "warn" -from_iter_instead_of_collect = "warn" inconsistent_struct_constructor = "warn" index_refutable_slice = "warn" inefficient_to_string = "warn" diff --git a/crates/common/src/borrow.rs b/crates/common/src/borrow.rs index 70d755ff155..ebf69fde71d 100644 --- a/crates/common/src/borrow.rs +++ b/crates/common/src/borrow.rs @@ -1,5 +1,6 @@ use crate::lock::{ - MapImmutable, PyImmutableMappedMutexGuard, PyMappedMutexGuard, PyMappedRwLockReadGuard, + MapImmutable, PyImmutableMappedMutexGuard, PyMappedDetachingRwLockReadGuard, + PyMappedDetachingRwLockWriteGuard, PyMappedMutexGuard, PyMappedRwLockReadGuard, PyMappedRwLockWriteGuard, PyMutexGuard, PyRwLockReadGuard, PyRwLockWriteGuard, }; use alloc::fmt; @@ -24,6 +25,7 @@ pub enum BorrowedValue<'a, T: ?Sized> { MappedMuLock(PyImmutableMappedMutexGuard<'a, T>), ReadLock(PyRwLockReadGuard<'a, T>), MappedReadLock(PyMappedRwLockReadGuard<'a, T>), + MappedDetachingReadLock(PyMappedDetachingRwLockReadGuard<'a, T>), } impl_from!('a, T, BorrowedValue<'a, T>, Ref(&'a T), @@ -31,6 +33,7 @@ impl_from!('a, T, BorrowedValue<'a, T>, MappedMuLock(PyImmutableMappedMutexGuard<'a, T>), ReadLock(PyRwLockReadGuard<'a, T>), MappedReadLock(PyMappedRwLockReadGuard<'a, T>), + MappedDetachingReadLock(PyMappedDetachingRwLockReadGuard<'a, T>), ); impl<'a, T: ?Sized> BorrowedValue<'a, T> { @@ -59,6 +62,9 @@ impl<'a, T: ?Sized> BorrowedValue<'a, T> { Self::MappedReadLock(m) => { BorrowedValue::MappedReadLock(PyMappedRwLockReadGuard::map(m, f)) } + Self::MappedDetachingReadLock(m) => { + BorrowedValue::MappedDetachingReadLock(PyMappedDetachingRwLockReadGuard::map(m, f)) + } } } } @@ -73,6 +79,7 @@ impl Deref for BorrowedValue<'_, T> { Self::MappedMuLock(m) => m, Self::ReadLock(r) => r, Self::MappedReadLock(m) => m, + Self::MappedDetachingReadLock(m) => m, } } } @@ -90,6 +97,7 @@ pub enum BorrowedValueMut<'a, T: ?Sized> { MappedMuLock(PyMappedMutexGuard<'a, T>), WriteLock(PyRwLockWriteGuard<'a, T>), MappedWriteLock(PyMappedRwLockWriteGuard<'a, T>), + MappedDetachingWriteLock(PyMappedDetachingRwLockWriteGuard<'a, T>), } impl_from!('a, T, BorrowedValueMut<'a, T>, @@ -98,6 +106,7 @@ impl_from!('a, T, BorrowedValueMut<'a, T>, MappedMuLock(PyMappedMutexGuard<'a, T>), WriteLock(PyRwLockWriteGuard<'a, T>), MappedWriteLock(PyMappedRwLockWriteGuard<'a, T>), + MappedDetachingWriteLock(PyMappedDetachingRwLockWriteGuard<'a, T>), ); impl<'a, T: ?Sized> BorrowedValueMut<'a, T> { @@ -113,6 +122,9 @@ impl<'a, T: ?Sized> BorrowedValueMut<'a, T> { Self::MappedWriteLock(m) => { BorrowedValueMut::MappedWriteLock(PyMappedRwLockWriteGuard::map(m, f)) } + Self::MappedDetachingWriteLock(m) => BorrowedValueMut::MappedDetachingWriteLock( + PyMappedDetachingRwLockWriteGuard::map(m, f), + ), } } } @@ -127,6 +139,7 @@ impl Deref for BorrowedValueMut<'_, T> { Self::MappedMuLock(m) => m, Self::WriteLock(w) => w, Self::MappedWriteLock(w) => w, + Self::MappedDetachingWriteLock(w) => w, } } } @@ -139,6 +152,7 @@ impl DerefMut for BorrowedValueMut<'_, T> { Self::MappedMuLock(m) => &mut *m, Self::WriteLock(w) => &mut *w, Self::MappedWriteLock(w) => &mut *w, + Self::MappedDetachingWriteLock(w) => &mut *w, } } } diff --git a/crates/common/src/lock.rs b/crates/common/src/lock.rs index 08fbc316599..134b7f2a4fa 100644 --- a/crates/common/src/lock.rs +++ b/crates/common/src/lock.rs @@ -8,6 +8,7 @@ use lock_api::{ cfg_select! { feature = "threading" => { + pub use detaching::{BlockingWaitHook, set_blocking_wait_hook, set_world_stopped}; pub use parking_lot::{RawMutex, RawRwLock, RawThreadId}; pub use std::sync::OnceLock as OnceCell; pub use core::cell::LazyCell; @@ -47,6 +48,8 @@ cfg_select! { } } +mod detaching; +pub use detaching::RawDetachingRwLock; mod immutable_mutex; pub use immutable_mutex::*; mod thread_mutex; @@ -60,6 +63,19 @@ pub type PyThreadMutex = ThreadMutex; pub type PyThreadMutexGuard<'a, T> = ThreadMutexGuard<'a, RawMutex, RawThreadId, T>; pub type PyMappedThreadMutexGuard<'a, T> = MappedThreadMutexGuard<'a, RawMutex, RawThreadId, T>; +/// A `PyRwLock` for data a thread may hold locked across a blocking call. +/// +/// Waiting for one of these leaves the interpreter first, so a thread blocked +/// on it is a thread stop-the-world can park. That is only safe where a +/// collection never takes the same lock — see [`RawDetachingRwLock`] — so this +/// is opt-in per lock rather than what every `PyRwLock` does. +pub type PyDetachingRwLock = RwLock; +pub type PyDetachingRwLockReadGuard<'a, T> = RwLockReadGuard<'a, RawDetachingRwLock, T>; +pub type PyDetachingRwLockWriteGuard<'a, T> = RwLockWriteGuard<'a, RawDetachingRwLock, T>; +pub type PyMappedDetachingRwLockReadGuard<'a, T> = MappedRwLockReadGuard<'a, RawDetachingRwLock, T>; +pub type PyMappedDetachingRwLockWriteGuard<'a, T> = + MappedRwLockWriteGuard<'a, RawDetachingRwLock, T>; + pub type PyRwLock = RwLock; pub type PyRwLockUpgradableReadGuard<'a, T> = RwLockUpgradableReadGuard<'a, RawRwLock, T>; pub type PyRwLockReadGuard<'a, T> = RwLockReadGuard<'a, RawRwLock, T>; diff --git a/crates/common/src/lock/detaching.rs b/crates/common/src/lock/detaching.rs new file mode 100644 index 00000000000..b662e5d85a1 --- /dev/null +++ b/crates/common/src/lock/detaching.rs @@ -0,0 +1,365 @@ +//! A reader-writer lock that lets a thread leave its interpreter before it +//! blocks. +//! +//! [`RawDetachingRwLock`] carries the reasoning: what goes wrong when a thread +//! waits for a lock while attached, why the waiter rather than the holder is +//! the one that has to give way, and the rule that comes with fixing it. +//! +//! The wait itself is handed to a hook, because this crate cannot depend on the +//! vm and so cannot detach a thread by itself. Whoever can installs it through +//! [`set_blocking_wait_hook`]; until then, and on any thread that is not +//! running an interpreter, a blocked acquire just blocks. Only the contended +//! path reaches any of this — an acquire that takes the lock on its first try +//! is the same atomic exchange it was. + +use super::RawRwLock; +#[cfg(feature = "threading")] +use core::cell::Cell; +use lock_api::{ + RawRwLock as RawRwLockTrait, RawRwLockDowngrade, RawRwLockUpgrade as RawRwLockUpgradeTrait, + RawRwLockUpgradeDowngrade, +}; +#[cfg(feature = "threading")] +use std::sync::OnceLock; + +/// Runs `wait` with the calling thread detached from its interpreter. +#[cfg(feature = "threading")] +pub type BlockingWaitHook = fn(wait: &dyn Fn()); + +#[cfg(feature = "threading")] +static BLOCKING_WAIT: OnceLock = OnceLock::new(); + +/// Install the hook that detaches a thread around a blocked lock acquire. +/// +/// Later calls are ignored, so every interpreter in a process can call this +/// during its own initialization. +#[cfg(feature = "threading")] +pub fn set_blocking_wait_hook(hook: BlockingWaitHook) { + let _ = BLOCKING_WAIT.set(hook); +} + +#[cfg(feature = "threading")] +std::thread_local! { + /// Set while this thread is inside the hook, so that a lock taken by the + /// hook itself — or by anything detaching and re-attaching runs — waits + /// plainly instead of recursing back into it. + static IN_HOOK: Cell = const { Cell::new(false) }; +} + +/// Clears [`IN_HOOK`] even if the hook unwinds. +#[cfg(feature = "threading")] +struct HookGuard; + +#[cfg(feature = "threading")] +impl Drop for HookGuard { + fn drop(&mut self) { + let _ = IN_HOOK.try_with(|in_hook| in_hook.set(false)); + } +} + +#[cfg(all(feature = "threading", debug_assertions))] +std::thread_local! { + /// Set on the one thread still running while the world is stopped. + static WORLD_STOPPED: Cell = const { Cell::new(false) }; +} + +/// Record whether this thread is the one running inside a stopped world. +/// +/// The rule for opting a lock into detaching is that nothing reachable from a +/// stop-the-world section takes it — a section that did could block on a lock +/// only that same section can release. Not implementing `Traverse` states the +/// rule to a collection; this states it to every other section, which is +/// otherwise unchecked. Debug builds only; release builds track nothing and +/// pay nothing. +#[cfg(feature = "threading")] +#[inline] +pub fn set_world_stopped(stopped: bool) { + #[cfg(debug_assertions)] + let _ = WORLD_STOPPED.try_with(|flag| flag.set(stopped)); + #[cfg(not(debug_assertions))] + let _ = stopped; +} + +/// Panics if a stop-the-world section is taking one of these locks. +#[cfg(all(feature = "threading", debug_assertions))] +#[track_caller] +fn assert_not_stopping_the_world() { + // `try_with` fails only once thread locals are being destroyed, which is + // not a point at which this thread is driving a stop. + let stopped = WORLD_STOPPED.try_with(Cell::get).unwrap_or(false); + assert!( + !stopped, + "a stop-the-world section took a detaching lock, which a parked thread \ + may be holding and only this section can release" + ); +} + +#[cfg(not(all(feature = "threading", debug_assertions)))] +#[inline(always)] +fn assert_not_stopping_the_world() {} + +/// Block on `wait`, detached from this thread's interpreter if there is one. +/// +/// Nothing spins on the way here. The lock underneath already spins before it +/// parks, and skips that spin once a waiter has parked — the same condition +/// `_PyMutex_LockTimed` spins under. A spin layered on top cannot read that +/// condition, and would go on retrying a `try_lock` that reports failure for as +/// long as a writer holds the writer bit, which it takes before it waits for +/// readers to drain: a yield per retry for the whole of exactly the wait this +/// exists to survive. +#[cfg(feature = "threading")] +#[cold] +#[inline(never)] +fn wait_detached(wait: impl Fn()) { + let Some(hook) = BLOCKING_WAIT.get() else { + wait(); + return; + }; + // `try_with` fails once the thread's locals are being destroyed, which is + // also a point at which there is no interpreter left to detach from. + let entered = IN_HOOK + .try_with(|in_hook| !in_hook.replace(true)) + .unwrap_or(false); + if !entered { + wait(); + return; + } + let _guard = HookGuard; + hook(&wait); +} + +/// Without threads there is no interpreter to leave and nothing to stop. +#[cfg(not(feature = "threading"))] +#[inline] +fn wait_detached(wait: impl Fn()) { + wait(); +} + +/// A reader-writer lock whose blocking acquires detach first, and which is the +/// raw lock it wraps in every other respect. +/// +/// Use through [`PyDetachingRwLock`](super::PyDetachingRwLock). +/// +/// # Why this exists +/// +/// Stopping the world means waiting until every other thread sits at +/// SUSPENDED, and there are two ways a thread gets there: +/// +/// - A DETACHED thread is not running interpreter code, so the requester moves +/// it to SUSPENDED itself. The thread never finds out. +/// - An ATTACHED thread can only suspend itself, at a safepoint — the check +/// `check_signals` makes between bytecodes. +/// +/// A thread blocked acquiring a lock runs no bytecode, so it reaches no +/// safepoint. While ATTACHED it is a thread the world cannot stop for as long +/// as it waits, and the requester waits without a bound. +/// +/// On its own that is a pause. It becomes a deadlock as soon as the lock being +/// waited for is held by a thread the same stop has already parked: +/// +/// ```text +/// A holds the lock, blocks inside allow_threads -> DETACHED +/// B requests a stop, and parks A -> A is SUSPENDED, holding the lock +/// C wants the same lock, and waits for it -> ATTACHED, blocked +/// +/// B waits for C to suspend C reaches no safepoint +/// C waits for A to release A is parked +/// A waits for B to start the world B is still waiting for C +/// ``` +/// +/// No thread in that cycle can break it, because none of them is running. It is +/// not hypothetical: an `SSLSocket.read` against a peer that completed a +/// handshake and then went quiet froze whole processes this way, the main +/// thread included, so not even a Python-level timeout could fire. +/// +/// The holder cannot be the one to give way. A lock is held across a blocking +/// call precisely because that is what the call needs. So the waiter gives way +/// instead: it leaves its interpreter for the duration of the wait, which is +/// what a blocking call does anyway, and a waiter that has left is a waiter the +/// requester can park. C detaches before it blocks, the stop completes, B +/// finishes, A resumes and releases, and C takes the lock and attaches again. +/// +/// # Only for locks a stop-the-world section never takes +/// +/// The wait acquires the lock while detached, so the thread comes back holding +/// it, and re-attaching is a point at which a stop-the-world in flight will +/// park the thread. It is therefore parked *holding the lock*. Everything that +/// stops the world must be able to finish without that lock: if a collection +/// were to take it, the collection would block on a thread only the collection +/// can release, and neither would move again. +/// +/// So this is opt-in per lock, and the rule for opting in is that nothing +/// reachable from a stop-the-world section takes the same lock. An object whose +/// payload holds no references — nothing for the collector to traverse into — +/// satisfies that; most do not. +/// +/// Not implementing the vm's `Traverse` for this lock enforces part of that: a +/// payload holding one cannot derive `Traverse`, so it cannot become something +/// a collection walks into. Only that part. A collection is not the only thing +/// that stops the world — dumping tracebacks, enumerating thread frames and +/// forking all do — and nothing checks what those reach. For them the rule is +/// still a convention. +#[repr(transparent)] +pub struct RawDetachingRwLock(RawRwLock); + +// SAFETY: every method forwards to the wrapped raw lock, which upholds the +// contract; the blocking acquires only add a wait that ends with the same lock +// acquired. +unsafe impl RawRwLockTrait for RawDetachingRwLock { + #[allow( + clippy::declare_interior_mutable_const, + reason = "raw lock initializer, as in the type it wraps" + )] + const INIT: Self = Self(::INIT); + + type GuardMarker = ::GuardMarker; + + #[inline] + fn lock_shared(&self) { + assert_not_stopping_the_world(); + if !self.0.try_lock_shared() { + wait_detached(|| self.0.lock_shared()); + } + } + + #[inline] + fn try_lock_shared(&self) -> bool { + self.0.try_lock_shared() + } + + #[inline] + unsafe fn unlock_shared(&self) { + unsafe { self.0.unlock_shared() } + } + + #[inline] + fn lock_exclusive(&self) { + assert_not_stopping_the_world(); + if !self.0.try_lock_exclusive() { + wait_detached(|| self.0.lock_exclusive()); + } + } + + #[inline] + fn try_lock_exclusive(&self) -> bool { + self.0.try_lock_exclusive() + } + + #[inline] + unsafe fn unlock_exclusive(&self) { + unsafe { self.0.unlock_exclusive() } + } + + #[inline] + fn is_locked(&self) -> bool { + self.0.is_locked() + } + + #[inline] + fn is_locked_exclusive(&self) -> bool { + self.0.is_locked_exclusive() + } +} + +// SAFETY: forwards to the wrapped raw lock. +unsafe impl RawRwLockDowngrade for RawDetachingRwLock { + #[inline] + unsafe fn downgrade(&self) { + unsafe { self.0.downgrade() } + } +} + +// SAFETY: forwards to the wrapped raw lock; `lock_upgradable` only adds a wait +// that ends with the same lock acquired. +// +// `lock_upgradable` detaches for the same reason `lock_shared` does: it starts +// from holding nothing, so the wait cannot park a thread that holds the lock. +// `upgrade` does not, because it runs with the upgradable lock already held. +unsafe impl RawRwLockUpgradeTrait for RawDetachingRwLock { + #[inline] + fn lock_upgradable(&self) { + assert_not_stopping_the_world(); + if !self.0.try_lock_upgradable() { + wait_detached(|| self.0.lock_upgradable()); + } + } + + #[inline] + fn try_lock_upgradable(&self) -> bool { + self.0.try_lock_upgradable() + } + + #[inline] + unsafe fn unlock_upgradable(&self) { + unsafe { self.0.unlock_upgradable() } + } + + #[inline] + unsafe fn upgrade(&self) { + // SAFETY: the caller holds the upgradable lock, as `upgrade` requires. + unsafe { self.0.upgrade() } + } + + #[inline] + unsafe fn try_upgrade(&self) -> bool { + unsafe { self.0.try_upgrade() } + } +} + +// SAFETY: forwards to the wrapped raw lock. +unsafe impl RawRwLockUpgradeDowngrade for RawDetachingRwLock { + #[inline] + unsafe fn downgrade_upgradable(&self) { + unsafe { self.0.downgrade_upgradable() } + } + + #[inline] + unsafe fn downgrade_to_upgradable(&self) { + unsafe { self.0.downgrade_to_upgradable() } + } +} + +// `RawRwLockRecursive` is deliberately not implemented, so that `read_recursive` +// does not exist on these locks. It is the one blocking acquire that cannot +// detach: a recursive read may be the re-entrant take of a lock this thread +// already holds, and detaching there parks a thread *holding* the lock, which is +// the deadlock this type exists to avoid. Leaving it implemented but attached +// would instead leave an acquire that stalls stop-the-world, so neither form of +// it belongs here. + +#[cfg(test)] +mod tests { + #[cfg(all(feature = "threading", debug_assertions))] + use super::set_world_stopped; + #[cfg(all(feature = "threading", debug_assertions))] + use crate::lock::PyDetachingRwLock; + + /// The opt-in rule holds for every stop-the-world section, not only the + /// collector that not implementing `Traverse` speaks to. + #[cfg(all(feature = "threading", debug_assertions))] + #[test] + fn taking_one_while_stopping_the_world_is_caught() { + let lock = PyDetachingRwLock::new(()); + + // Ordinary use, for contrast. + drop(lock.write()); + + // The panic below is the expected result, so it prints where an + // unexpected one would. Silencing it would mean replacing the panic hook, + // which is process-wide and would swallow the output of whatever else the + // test binary is running at the same time. + set_world_stopped(true); + let taken = std::panic::catch_unwind(core::panic::AssertUnwindSafe(|| { + let _guard = lock.read(); + })); + set_world_stopped(false); + + assert!( + taken.is_err(), + "a stop-the-world section took a detaching lock and nothing complained" + ); + + // The flag is per-thread and back to clear, so the lock still works. + drop(lock.write()); + } +} diff --git a/crates/stdlib/src/fcntl.rs b/crates/stdlib/src/fcntl.rs index 8e24f2b6e4a..dca53c104dd 100644 --- a/crates/stdlib/src/fcntl.rs +++ b/crates/stdlib/src/fcntl.rs @@ -78,15 +78,16 @@ mod fcntl { .ok_or_else(|| vm.new_value_error("fcntl string arg too long"))? .copy_from_slice(&s) } - host_fcntl::fcntl_with_bytes(fd, cmd, &mut buf[..arg_len]) - .map_err(|_| vm.new_last_errno_error())?; + vm.allow_threads(|| host_fcntl::fcntl_with_bytes(fd, cmd, &mut buf[..arg_len])) + .map_err(|err| err.to_pyexception(vm))?; return Ok(vm.ctx.new_bytes(buf[..arg_len].to_vec()).into()); } OptionalArg::Present(Either::B(i)) => i.as_u32_mask(), OptionalArg::Missing => 0, }; - let ret = - host_fcntl::fcntl_int(fd, cmd, int as i32).map_err(|_| vm.new_last_errno_error())?; + let ret = vm + .allow_threads(|| host_fcntl::fcntl_int(fd, cmd, int as i32)) + .map_err(|err| err.to_pyexception(vm))?; Ok(vm.new_pyobj(ret)) } @@ -114,26 +115,37 @@ mod fcntl { let buf_len = match buf_kind { Either::A(rw_arg) => { let mutate_flag = mutate_flag.unwrap_or(true); - let mut arg_buf = rw_arg.borrow_buf_mut(); if mutate_flag { - let ret = unsafe { - host_fcntl::ioctl_ptr(fd, request, arg_buf.as_mut_ptr().cast()) - } - .map_err(|_| vm.new_last_errno_error())?; + // A terminal or a socket answers an ioctl when it is + // ready to, so the call runs detached, and the target's + // bytes go in and come back through a buffer of our own + // rather than stay locked meanwhile -- `fcntl_ioctl_impl` + // copies through one the same way. + let mut scratch = vm.new_zeroed_bytes(rw_arg.len())?; + scratch.copy_from_slice(&rw_arg.borrow_buf_mut()); + let ret = vm + .allow_threads(|| unsafe { + host_fcntl::ioctl_ptr(fd, request, scratch.as_mut_ptr().cast()) + }) + .map_err(|err| err.to_pyexception(vm))?; + rw_arg.borrow_buf_mut().copy_from_slice(&scratch); return Ok(vm.ctx.new_int(ret).into()); } // treat like an immutable buffer - fill_buf(&arg_buf)? + fill_buf(&rw_arg.borrow_buf_mut())? } Either::B(ro_buf) => fill_buf(&ro_buf.borrow_bytes())?, }; - unsafe { host_fcntl::ioctl_ptr(fd, request, buf.as_mut_ptr().cast()) } - .map_err(|_| vm.new_last_errno_error())?; + vm.allow_threads(|| unsafe { + host_fcntl::ioctl_ptr(fd, request, buf.as_mut_ptr().cast()) + }) + .map_err(|err| err.to_pyexception(vm))?; Ok(vm.ctx.new_bytes(buf[..buf_len].to_vec()).into()) } Either::B(i) => { - let ret = - host_fcntl::ioctl_int(fd, request, i).map_err(|_| vm.new_last_errno_error())?; + let ret = vm + .allow_threads(|| host_fcntl::ioctl_int(fd, request, i)) + .map_err(|err| err.to_pyexception(vm))?; Ok(vm.ctx.new_int(ret).into()) } } @@ -143,7 +155,11 @@ mod fcntl { #[cfg(not(any(target_os = "wasi", target_os = "redox")))] #[pyfunction] fn flock(_io::Fildes(fd): _io::Fildes, operation: i32, vm: &VirtualMachine) -> PyResult { - let ret = host_fcntl::flock(fd, operation).map_err(|_| vm.new_last_errno_error())?; + // LOCK_EX without LOCK_NB waits for whoever holds the lock, which may + // be for good. + let ret = vm + .allow_threads(|| host_fcntl::flock(fd, operation)) + .map_err(|err| err.to_pyexception(vm))?; Ok(vm.ctx.new_int(ret).into()) } @@ -170,8 +186,10 @@ mod fcntl { OptionalArg::Present(w) => w, OptionalArg::Missing => 0, }; - let ret = - host_fcntl::lockf(fd, cmd, len, start, whence).map_err(|err| err.to_pyexception(vm))?; + // F_LOCK and F_TLOCK differ in exactly this: the first one waits. + let ret = vm + .allow_threads(|| host_fcntl::lockf(fd, cmd, len, start, whence)) + .map_err(|err| err.to_pyexception(vm))?; Ok(vm.ctx.new_int(ret).into()) } } diff --git a/crates/stdlib/src/openssl.rs b/crates/stdlib/src/openssl.rs index ee9d9ae84e0..6a35a30dc89 100644 --- a/crates/stdlib/src/openssl.rs +++ b/crates/stdlib/src/openssl.rs @@ -64,8 +64,8 @@ mod _ssl { }; use crate::{ common::lock::{ - LazyLock, PyMappedRwLockReadGuard, PyMutex, PyRwLock, PyRwLockReadGuard, - PyRwLockWriteGuard, + LazyLock, PyDetachingRwLock, PyMappedRwLockReadGuard, PyMutex, PyRwLock, + PyRwLockReadGuard, PyRwLockWriteGuard, }, socket::{self, PySocket, SockWaitKind, sock_wait}, vm::{ @@ -568,7 +568,9 @@ mod _ssl { } // Get SSL pointer - either from thread-local (during handshake) or from connection - fn get_ssl_ptr_for_context_change(connection: &PyRwLock) -> *mut sys::SSL { + fn get_ssl_ptr_for_context_change( + connection: &PyDetachingRwLock, + ) -> *mut sys::SSL { // First check if we're in a handshake callback (lock already held) if let Some(ptr) = HANDSHAKE_SSL_PTR.with(|cell| cell.get()) { return ptr; @@ -672,11 +674,10 @@ mod _ssl { unsafe { let ctx = &*(arg as *const PySslContext); - // Get the callback - let callback_opt = ctx.sni_callback.lock().clone(); - let Some(callback) = callback_opt else { + // Nothing to call: leave without reaching the interpreter at all. + if ctx.sni_callback.lock().is_none() { return SSL_TLSEXT_ERR_OK; - }; + } // Get callback data from SSL ex_data let idx = get_sni_ex_data_index(); @@ -695,66 +696,77 @@ mod _ssl { }; let vm = &*vm_ptr; - // Get server name - let servername = sys::SSL_get_servername(ssl_ptr, TLSEXT_NAMETYPE_host_name); - let server_name_arg = if servername.is_null() { - vm.ctx.none() - } else { - let name_cstr = core::ffi::CStr::from_ptr(servername); - match name_cstr.to_str() { - Ok(name_str) => vm.ctx.new_str(name_str).into(), - Err(_) => vm.ctx.none(), - } - }; + // The handshake this runs inside has left the interpreter, so + // everything below rejoins it first — taking a reference to the + // callback already counts — and gives the thread back after. + vm.attach_for_callback(|| { + // Get the callback + let callback_opt = ctx.sni_callback.lock().clone(); + let Some(callback) = callback_opt else { + return SSL_TLSEXT_ERR_OK; + }; - // Get SSL socket from callback data via weak reference - let ssl_socket_obj = callback_data - .ssl_socket_weak - .upgrade() - .unwrap_or_else(|| vm.ctx.none()); + // Get server name + let servername = sys::SSL_get_servername(ssl_ptr, TLSEXT_NAMETYPE_host_name); + let server_name_arg = if servername.is_null() { + vm.ctx.none() + } else { + let name_cstr = core::ffi::CStr::from_ptr(servername); + match name_cstr.to_str() { + Ok(name_str) => vm.ctx.new_str(name_str).into(), + Err(_) => vm.ctx.none(), + } + }; - // Call the Python callback - match callback.call( - ( - ssl_socket_obj, - server_name_arg, - callback_data.ssl_context.to_owned(), - ), - vm, - ) { - Ok(result) => { - // Check return value type (must be None or integer) - if vm.is_none(&result) { - // None is OK - SSL_TLSEXT_ERR_OK - } else { - // Try to convert to integer - match result.try_to_value::(vm) { - Ok(alert_code) => { - // Valid integer - use as alert code - *al = alert_code; - SSL_TLSEXT_ERR_ALERT_FATAL - } - Err(_) => { - // Type conversion failed - raise TypeError - let type_error = vm.new_type_error(format!( + // Get SSL socket from callback data via weak reference + let ssl_socket_obj = callback_data + .ssl_socket_weak + .upgrade() + .unwrap_or_else(|| vm.ctx.none()); + + // Call the Python callback + match callback.call( + ( + ssl_socket_obj, + server_name_arg, + callback_data.ssl_context.to_owned(), + ), + vm, + ) { + Ok(result) => { + // Check return value type (must be None or integer) + if vm.is_none(&result) { + // None is OK + SSL_TLSEXT_ERR_OK + } else { + // Try to convert to integer + match result.try_to_value::(vm) { + Ok(alert_code) => { + // Valid integer - use as alert code + *al = alert_code; + SSL_TLSEXT_ERR_ALERT_FATAL + } + Err(_) => { + // Type conversion failed - raise TypeError + let type_error = vm.new_type_error(format!( "servername callback must return None or an integer, not '{}'", result.class().name() )); - vm.run_unraisable(type_error, None, result); - *al = SSL_AD_INTERNAL_ERROR; - SSL_TLSEXT_ERR_ALERT_FATAL + vm.run_unraisable(type_error, None, result); + *al = SSL_AD_INTERNAL_ERROR; + SSL_TLSEXT_ERR_ALERT_FATAL + } } } } + Err(exc) => { + // Log the exception but don't propagate it + vm.run_unraisable(exc, None, vm.ctx.none()); + *al = SSL_AD_INTERNAL_ERROR; + SSL_TLSEXT_ERR_ALERT_FATAL + } } - Err(exc) => { - // Log the exception but don't propagate it - vm.run_unraisable(exc, None, vm.ctx.none()); - *al = SSL_AD_INTERNAL_ERROR; - SSL_TLSEXT_ERR_ALERT_FATAL - } - } + }) } } @@ -794,11 +806,10 @@ mod _ssl { // ssl_socket_ptr is a pointer to Box>, set in _wrap_socket/_wrap_bio let ssl_socket: &Py = &*(ssl_socket_ptr as *const Py); - // Get the callback from the context - let callback_opt = ssl_socket.ctx.read().msg_callback.lock().clone(); - let Some(callback) = callback_opt else { + // Nothing to call: leave without reaching the interpreter at all. + if ssl_socket.ctx.read().msg_callback.lock().is_none() { return; - }; + } // Get VM from thread-local storage (set by HandshakeVmGuard in do_handshake) let Some(vm_ptr) = HANDSHAKE_VM.with(|cell| cell.get()) else { @@ -807,63 +818,74 @@ mod _ssl { }; let vm = &*vm_ptr; - // Get SSL socket owner object - let ssl_socket_obj = ssl_socket - .owner - .read() - .as_ref() - .and_then(|weak| weak.upgrade()) - .unwrap_or_else(|| vm.ctx.none()); - - // Create the message bytes - let buf_slice = core::slice::from_raw_parts(buf as *const u8, len); - let msg_bytes = vm.ctx.new_bytes(buf_slice.to_vec()); - - // Determine direction string - let direction_str = if write_p != 0 { "write" } else { "read" }; - - // Calculate msg_type based on content_type (debughelpers.c behavior) - let msg_type = match content_type { - SSL3_RT_CHANGE_CIPHER_SPEC => SSL3_MT_CHANGE_CIPHER_SPEC, - SSL3_RT_ALERT if len >= 2 => { - // byte 1 is alert type - buf_slice[1] as i32 - } - SSL3_RT_HANDSHAKE if !buf_slice.is_empty() => { - // byte 0 is handshake type - buf_slice[0] as i32 - } - SSL3_RT_HEADER if len >= 3 => { - // Frame header: version in bytes 1..2, type in byte 0 - version = ((buf_slice[1] as i32) << 8) | (buf_slice[2] as i32); - buf_slice[0] as i32 - } - SSL3_RT_INNER_CONTENT_TYPE if !buf_slice.is_empty() => { - // Inner content type in byte 0 - buf_slice[0] as i32 - } - _ => -1, - }; + // The SSL call this reports from has left the interpreter; rejoin + // it for the duration of the callback, as `_servername_callback` + // does above. + vm.attach_for_callback(|| { + // Get the callback from the context + let callback_opt = ssl_socket.ctx.read().msg_callback.lock().clone(); + let Some(callback) = callback_opt else { + return; + }; - // Call the Python callback - // Signature: callback(conn, direction, version, content_type, msg_type, data) - match callback.call( - ( - ssl_socket_obj, - vm.ctx.new_str(direction_str), - vm.ctx.new_int(version), - vm.ctx.new_int(content_type), - vm.ctx.new_int(msg_type), - msg_bytes, - ), - vm, - ) { - Ok(_) => {} - Err(exc) => { - // Log the exception but don't propagate it - vm.run_unraisable(exc, None, vm.ctx.none()); + // Get SSL socket owner object + let ssl_socket_obj = ssl_socket + .owner + .read() + .as_ref() + .and_then(|weak| weak.upgrade()) + .unwrap_or_else(|| vm.ctx.none()); + + // Create the message bytes + let buf_slice = core::slice::from_raw_parts(buf as *const u8, len); + let msg_bytes = vm.ctx.new_bytes(buf_slice.to_vec()); + + // Determine direction string + let direction_str = if write_p != 0 { "write" } else { "read" }; + + // Calculate msg_type based on content_type (debughelpers.c behavior) + let msg_type = match content_type { + SSL3_RT_CHANGE_CIPHER_SPEC => SSL3_MT_CHANGE_CIPHER_SPEC, + SSL3_RT_ALERT if len >= 2 => { + // byte 1 is alert type + buf_slice[1] as i32 + } + SSL3_RT_HANDSHAKE if !buf_slice.is_empty() => { + // byte 0 is handshake type + buf_slice[0] as i32 + } + SSL3_RT_HEADER if len >= 3 => { + // Frame header: version in bytes 1..2, type in byte 0 + version = ((buf_slice[1] as i32) << 8) | (buf_slice[2] as i32); + buf_slice[0] as i32 + } + SSL3_RT_INNER_CONTENT_TYPE if !buf_slice.is_empty() => { + // Inner content type in byte 0 + buf_slice[0] as i32 + } + _ => -1, + }; + + // Call the Python callback + // Signature: callback(conn, direction, version, content_type, msg_type, data) + match callback.call( + ( + ssl_socket_obj, + vm.ctx.new_str(direction_str), + vm.ctx.new_int(version), + vm.ctx.new_int(content_type), + vm.ctx.new_int(msg_type), + msg_bytes, + ), + vm, + ) { + Ok(_) => {} + Err(exc) => { + // Log the exception but don't propagate it + vm.run_unraisable(exc, None, vm.ctx.none()); + } } - } + }) } } @@ -2157,7 +2179,7 @@ mod _ssl { let py_ssl_socket = PySslSocket { ctx: PyRwLock::new(zelf.clone()), - connection: PyRwLock::new(SslConnection::Socket(stream)), + connection: PyDetachingRwLock::new(SslConnection::Socket(stream)), socket_type, server_hostname, owner: PyRwLock::new(args.owner.map(|o| o.downgrade(None, vm)).transpose()?), @@ -2226,7 +2248,7 @@ mod _ssl { let py_ssl_socket = PySslSocket { ctx: PyRwLock::new(zelf.clone()), - connection: PyRwLock::new(SslConnection::Bio(stream)), + connection: PyDetachingRwLock::new(SslConnection::Bio(stream)), socket_type, server_hostname, owner: PyRwLock::new(args.owner.map(|o| o.downgrade(None, vm)).transpose()?), @@ -2527,7 +2549,7 @@ mod _ssl { struct PySslSocket { ctx: PyRwLock>, #[pytraverse(skip)] - connection: PyRwLock, + connection: PyDetachingRwLock, #[pytraverse(skip)] socket_type: SslServerOrClient, server_hostname: Option, @@ -2868,7 +2890,7 @@ mod _ssl { // BIO mode: just try shutdown once and raise SSLWantReadError if needed if stream.is_bio() { - let ret = unsafe { sys::SSL_shutdown(ssl_ptr) }; + let ret = vm.allow_threads(|| unsafe { sys::SSL_shutdown(ssl_ptr) }); if ret < 0 { let err = unsafe { sys::SSL_get_error(ssl_ptr, ret) }; if err == sys::SSL_ERROR_WANT_READ { @@ -2896,7 +2918,10 @@ mod _ssl { let mut zeros = 0; loop { - let ret = unsafe { sys::SSL_shutdown(ssl_ptr) }; + // Shutting down sends close-notify and waits for the peer's, + // which a peer that has gone away never sends. `SSL_shutdown` + // is released around for the same reason. + let ret = vm.allow_threads(|| unsafe { sys::SSL_shutdown(ssl_ptr) }); // ret > 0: complete shutdown if ret > 0 { @@ -3013,7 +3038,7 @@ mod _ssl { // BIO mode: no timeout/select logic, just do handshake if stream.is_bio() { - let result = stream.do_handshake().map_err(|e| { + let result = vm.allow_threads(|| stream.do_handshake()).map_err(|e| { let exc = convert_ssl_error(vm, e); // If it's a cert verification error, set verify info if exc.class().is(PySSLCertVerificationError::class(&vm.ctx)) { @@ -3033,7 +3058,13 @@ mod _ssl { .expect("handshake called in bio mode; should only be called in socket mode") .timeout_deadline(); loop { - let err = match stream.do_handshake() { + // On a blocking socket this waits for the peer, which may never + // answer. `SSL_do_handshake` runs between + // `Py_BEGIN_ALLOW_THREADS` and `Py_END_ALLOW_THREADS` for the + // same reason. The connection lock stays held across it, which + // is why it is a detaching lock: a thread reaching the same + // socket gives up its interpreter rather than wait attached. + let err = match vm.allow_threads(|| stream.do_handshake()) { Ok(()) => { // Clean up SNI ex_data after successful handshake // SAFETY: ssl_ptr is valid for the lifetime of stream @@ -3091,7 +3122,9 @@ mod _ssl { // BIO mode: no timeout/select logic if stream.is_bio() { - return stream.ssl_write(data).map_err(|e| convert_ssl_error(vm, e)); + return vm + .allow_threads(|| stream.ssl_write(data)) + .map_err(|e| convert_ssl_error(vm, e)); } // Socket mode: handle timeout and blocking @@ -3112,7 +3145,10 @@ mod _ssl { _ => {} } loop { - let err = match stream.ssl_write(data) { + // Sending waits for the peer to make room, which it need not + // ever do; `SSL_write_ex` is released around for the same + // reason. + let err = match vm.allow_threads(|| stream.ssl_write(data)) { Ok(len) => return Ok(len), Err(e) => e, }; @@ -3240,23 +3276,16 @@ mod _ssl { } let mut stream = self.connection.write(); - let mut inner_buffer = if let OptionalArg::Present(buffer) = &buffer { - Either::A(buffer.borrow_buf_mut()) - } else { - Either::B(vec![0u8; read_len]) - }; - let buf = match &mut inner_buffer { - Either::A(b) => &mut **b, - Either::B(b) => b.as_mut_slice(), - }; - let buf = match buf.get_mut(..read_len) { - Some(b) => b, - None => buf, - }; + // The read below answers when the peer writes, which may be never, + // and reaching the caller's buffer takes a lock that every other + // thread touching the same object waits on. Read aside and take + // that lock only for the copy. + let mut scratch = vm.new_zeroed_bytes(read_len)?; + let buf = scratch.as_mut_slice(); // BIO mode: no timeout/select logic let count = if stream.is_bio() { - match stream.ssl_read(buf) { + match vm.allow_threads(|| stream.ssl_read(buf)) { Ok(count) => count, Err(e) => { // Handle ZERO_RETURN (EOF) - raise SSLEOFError @@ -3278,7 +3307,10 @@ mod _ssl { .expect("read called in bio mode; should only be called in socket mode") .timeout_deadline(); loop { - let err = match stream.ssl_read(buf) { + // This is the wait the whole method is shaped around: it + // ends when the peer writes. `SSL_read_ex` is released + // around for the same reason. + let err = match vm.allow_threads(|| stream.ssl_read(buf)) { Ok(count) => break count, Err(e) => e, }; @@ -3312,12 +3344,15 @@ mod _ssl { return Err(convert_ssl_error(vm, err)); } }; - let ret = match inner_buffer { - Either::A(_buf) => vm.ctx.new_int(count).into(), - Either::B(mut buf) => { - buf.truncate(count); - buf.shrink_to_fit(); - vm.ctx.new_bytes(buf).into() + let ret = match &buffer { + OptionalArg::Present(buffer) => { + buffer.borrow_buf_mut()[..count].copy_from_slice(&scratch[..count]); + vm.ctx.new_int(count).into() + } + OptionalArg::Missing => { + scratch.truncate(count); + scratch.shrink_to_fit(); + vm.ctx.new_bytes(scratch).into() } }; Ok(ret) diff --git a/crates/vm/src/builtins/bytearray.rs b/crates/vm/src/builtins/bytearray.rs index 594ecc569d8..f063a1d08c9 100644 --- a/crates/vm/src/builtins/bytearray.rs +++ b/crates/vm/src/builtins/bytearray.rs @@ -18,8 +18,8 @@ use crate::{ common::{ atomic::{AtomicUsize, Ordering}, lock::{ - PyMappedRwLockReadGuard, PyMappedRwLockWriteGuard, PyMutex, PyRwLock, - PyRwLockReadGuard, PyRwLockWriteGuard, + PyDetachingRwLock, PyDetachingRwLockReadGuard, PyDetachingRwLockWriteGuard, + PyMappedDetachingRwLockReadGuard, PyMappedDetachingRwLockWriteGuard, PyMutex, }, }, convert::{ToPyObject, ToPyResult}, @@ -43,7 +43,7 @@ use core::mem::size_of; #[pyclass(module = false, name = "bytearray", unhashable = true)] #[derive(Debug, Default)] pub struct PyByteArray { - inner: PyRwLock, + inner: PyDetachingRwLock, exports: AtomicUsize, } @@ -81,17 +81,17 @@ impl PyByteArray { const fn from_inner(inner: PyBytesInner) -> Self { Self { - inner: PyRwLock::new(inner), + inner: PyDetachingRwLock::new(inner), exports: AtomicUsize::new(0), } } - pub fn borrow_buf(&self) -> PyMappedRwLockReadGuard<'_, [u8]> { - PyRwLockReadGuard::map(self.inner.read(), |inner| &*inner.elements) + pub fn borrow_buf(&self) -> PyMappedDetachingRwLockReadGuard<'_, [u8]> { + PyDetachingRwLockReadGuard::map(self.inner.read(), |inner| &*inner.elements) } - pub fn borrow_buf_mut(&self) -> PyMappedRwLockWriteGuard<'_, Vec> { - PyRwLockWriteGuard::map(self.inner.write(), |inner| &mut inner.elements) + pub fn borrow_buf_mut(&self) -> PyMappedDetachingRwLockWriteGuard<'_, Vec> { + PyDetachingRwLockWriteGuard::map(self.inner.write(), |inner| &mut inner.elements) } fn repeat(&self, value: isize, vm: &VirtualMachine) -> PyResult { @@ -194,11 +194,11 @@ impl PyByteArray { } #[inline] - fn inner(&self) -> PyRwLockReadGuard<'_, PyBytesInner> { + fn inner(&self) -> PyDetachingRwLockReadGuard<'_, PyBytesInner> { self.inner.read() } #[inline] - fn inner_mut(&self) -> PyRwLockWriteGuard<'_, PyBytesInner> { + fn inner_mut(&self) -> PyDetachingRwLockWriteGuard<'_, PyBytesInner> { self.inner.write() } @@ -739,9 +739,10 @@ impl Comparable for PyByteArray { static BUFFER_METHODS: BufferMethods = BufferMethods { obj_bytes: |buffer| buffer.obj_as::().borrow_buf().into(), obj_bytes_mut: |buffer| { - PyMappedRwLockWriteGuard::map(buffer.obj_as::().borrow_buf_mut(), |x| { - x.as_mut_slice() - }) + PyMappedDetachingRwLockWriteGuard::map( + buffer.obj_as::().borrow_buf_mut(), + |x| x.as_mut_slice(), + ) .into() }, release: |buffer| { @@ -783,7 +784,7 @@ impl AsBuffer for PyByteArray { } impl BufferResizeGuard for PyByteArray { - type Resizable<'a> = PyRwLockWriteGuard<'a, PyBytesInner>; + type Resizable<'a> = PyDetachingRwLockWriteGuard<'a, PyBytesInner>; fn try_resizable_opt(&self) -> Option> { // An export is a borrow someone else still holds, so it is answered diff --git a/crates/vm/src/stdlib/os.rs b/crates/vm/src/stdlib/os.rs index 70afdf827b8..523097ad971 100644 --- a/crates/vm/src/stdlib/os.rs +++ b/crates/vm/src/stdlib/os.rs @@ -344,24 +344,47 @@ pub(super) mod _os { } } + /// `read(2)` into `buf`, retrying on EINTR (PEP 475). + fn read_into_slice( + fd: crt_fd::Borrowed<'_>, + buf: &mut [u8], + vm: &VirtualMachine, + ) -> PyResult { + loop { + match vm.allow_threads(|| crt_fd::read(fd, buf)) { + Ok(n) => return Ok(n), + Err(e) if e.raw_os_error() == Some(libc::EINTR) => { + vm.check_signals()?; + continue; + } + Err(e) => return Err(e.into_pyexception(vm)), + } + } + } + #[pyfunction] fn readinto( fd: crt_fd::Borrowed<'_>, buffer: ArgMemoryBuffer, vm: &VirtualMachine, ) -> PyResult { - buffer.with_ref(|buf| { - loop { - match vm.allow_threads(|| crt_fd::read(fd, buf)) { - Ok(n) => return Ok(n), - Err(e) if e.raw_os_error() == Some(libc::EINTR) => { - vm.check_signals()?; - continue; - } - Err(e) => return Err(e.into_pyexception(vm)), - } - } - }) + if rustpython_host_env::io::reads_without_waiting(fd) { + // The read answers from the file itself, so it returns without + // waiting on anyone; write where the caller asked directly. + return buffer.with_ref(|buf| read_into_slice(fd, buf, vm)); + } + + // A pipe, socket or terminal answers only when the other end writes, + // which may be never. Holding the export for the whole call is what + // keeps the target from being resized meanwhile; but reaching its + // bytes takes a lock that every other thread touching the same object + // waits on, and a thread waiting on a lock never reaches a safepoint, + // so holding that one across the wait stops the world from being + // stopped at all. Read aside and take the lock for the copy. + let mut scratch = vm.new_zeroed_bytes(buffer.len())?; + let n = read_into_slice(fd, &mut scratch, vm)?; + buffer.borrow_buf_mut()[..n].copy_from_slice(&scratch[..n]); + Ok(n) } #[pyfunction] diff --git a/crates/vm/src/vm/interpreter.rs b/crates/vm/src/vm/interpreter.rs index 6d4e1f75a22..8545d6152df 100644 --- a/crates/vm/src/vm/interpreter.rs +++ b/crates/vm/src/vm/interpreter.rs @@ -76,6 +76,10 @@ where use core::sync::atomic::{AtomicBool, AtomicU64}; use crossbeam_utils::atomic::AtomicCell; + // Before any lock this interpreter's threads can contend on exists. + #[cfg(feature = "threading")] + thread::install_blocking_wait_hook(); + let (config, all_module_defs, frozen, hash_secret, int_max_str_digits) = if let Some(parent) = parent_state { // Subinterpreter: clone config and module tables from parent, fresh runtime state. @@ -1656,6 +1660,185 @@ for _ in range(40): worker.join().expect("nested worker panicked"); } + /// A thread blocked on a detaching lock must not stall stop-the-world. + /// + /// Blocking on a lock reaches no safepoint, so an interpreter thread that + /// waits while attached is a thread the world can never stop — and the + /// lock it waits for is routinely one a stopped thread holds, which is the + /// deadlock. The waiter therefore leaves its interpreter for the wait. + #[cfg(feature = "threading")] + #[test] + fn a_thread_blocked_on_a_lock_does_not_stall_stop_the_world() { + use super::super::thread::THREAD_DETACHED; + use crate::common::lock::PyDetachingRwLock; + use alloc::sync::Arc; + use core::{ + sync::atomic::{AtomicU64, Ordering}, + time::Duration, + }; + + let interp = Interpreter::without_stdlib(Default::default()); + let state = interp.enter(|vm| vm.state.clone()); + + let lock: Arc> = Arc::new(PyDetachingRwLock::new(())); + // The worker's thread id, published from inside the interpreter. No + // thread has id 0, so it doubles as "not registered yet". + let worker_ident = Arc::new(AtomicU64::new(0)); + + // Held for the whole test, so the worker below blocks and stays blocked. + let held = lock.write(); + + let worker_lock = Arc::clone(&lock); + let published_ident = Arc::clone(&worker_ident); + let worker = interp.enter(|vm| { + let thread_vm = vm.new_thread(); + std::thread::spawn(move || { + thread_vm.run(|_vm| { + published_ident.store(crate::stdlib::_thread::get_ident(), Ordering::Release); + let _read = worker_lock.read(); + }); + }) + }); + + // Wait for the worker to have blocked, not merely to have been scheduled + // to. It publishes its id while attached, so that slot reaching DETACHED + // is the contended acquire leaving the interpreter — the state this test + // is about. A sleep here would let the stop below complete with no + // blocked waiter at all, and pass without testing anything. + // + // Bounded, so an acquire that never detaches fails the test instead of + // hanging it, as the timeout on the stop below does. + let deadline = std::time::Instant::now() + Duration::from_secs(10); + let blocked_detached = |ident| { + state + .thread_frames + .lock() + .get(&ident) + .is_some_and(|slot| slot.state.load(Ordering::Acquire) == THREAD_DETACHED) + }; + loop { + match worker_ident.load(Ordering::Acquire) { + ident if ident != 0 && blocked_detached(ident) => break, + _ => assert!( + std::time::Instant::now() < deadline, + "the worker never detached for the contended acquire" + ), + } + std::thread::yield_now(); + } + + // Stop from a thread of its own so that a stop that never completes + // fails the test instead of hanging it. + let (tx, rx) = std::sync::mpsc::channel(); + let stop_state = state; + let stopper = std::thread::spawn(move || { + stop_state.stop_the_world.stop_the_world(&stop_state); + let stopped = tx.send(()); + stop_state.stop_the_world.start_the_world(&stop_state); + stopped + }); + + let stopped = rx.recv_timeout(Duration::from_secs(10)); + + // Release before any assertion: the worker has to finish for the + // stopper to be joinable, and for the test to end at all. + drop(held); + assert!( + stopped.is_ok(), + "stop-the-world did not complete while a thread was blocked on a lock" + ); + stopper.join().expect("stopper panicked").expect("send"); + worker.join().expect("worker panicked"); + } + + /// A callback reaching Python from inside a detached call waits for the + /// world to start again. + /// + /// Detaching for a blocking call is what lets stop-the-world count this + /// thread as parked. A callback that runs Python from in there — an SSL + /// handshake reaching a Python `sni_callback`, say — would run on a thread + /// the requester believes is stopped, so it has to attach first, and + /// attaching while the world is stopped means waiting. + #[cfg(feature = "threading")] + #[test] + fn a_callback_inside_a_detached_call_waits_for_the_world() { + use alloc::sync::Arc; + use core::{ + sync::atomic::{AtomicBool, Ordering}, + time::Duration, + }; + + let interp = Interpreter::without_stdlib(Default::default()); + let state = interp.enter(|vm| vm.state.clone()); + + let detached = Arc::new(AtomicBool::new(false)); + let ran = Arc::new(AtomicBool::new(false)); + let go = Arc::new(AtomicBool::new(false)); + + let worker_detached = Arc::clone(&detached); + let worker_ran = Arc::clone(&ran); + let worker_go = Arc::clone(&go); + let worker = interp.enter(|vm| { + let thread_vm = vm.new_thread(); + std::thread::spawn(move || { + thread_vm.run(|vm| { + vm.allow_threads(|| { + worker_detached.store(true, Ordering::Release); + // Spinning here is spinning *detached*, which is what a + // blocking call looks like to the requester: it marks + // this thread SUSPENDED and the stop completes. + while !worker_go.load(Ordering::Acquire) { + std::thread::yield_now(); + } + vm.attach_for_callback(|| worker_ran.store(true, Ordering::Release)); + }); + }); + }) + }); + + while !detached.load(Ordering::Acquire) { + std::thread::yield_now(); + } + + // Stop from a thread of its own so that a stop that never completes + // fails the test instead of hanging it. + let (stopped_tx, stopped_rx) = std::sync::mpsc::channel(); + let (release_tx, release_rx) = std::sync::mpsc::channel(); + let stop_state = state; + let stopper = std::thread::spawn(move || { + stop_state.stop_the_world.stop_the_world(&stop_state); + stopped_tx.send(()).expect("send"); + release_rx.recv().expect("recv"); + stop_state.stop_the_world.start_the_world(&stop_state); + }); + + stopped_rx + .recv_timeout(Duration::from_secs(10)) + .expect("stop-the-world did not complete"); + + // The world is stopped; turn the worker loose at its callback. It has + // to park instead of running it, so the flag stays clear — give it the + // time it needs to get there and fail to run. + go.store(true, Ordering::Release); + std::thread::sleep(Duration::from_millis(200)); + let ran_while_stopped = ran.load(Ordering::Acquire); + + // Release before asserting: the worker has to finish for the stopper to + // be joinable, and for the test to end at all. + release_tx.send(()).expect("send"); + stopper.join().expect("stopper panicked"); + worker.join().expect("worker panicked"); + + assert!( + !ran_while_stopped, + "a callback ran Python while the world was stopped" + ); + assert!( + ran.load(Ordering::Acquire), + "the callback never ran once the world started again" + ); + } + /// The process main id is recorded once and is stable across later creates. #[test] fn process_main_id_recorded_and_stable() { diff --git a/crates/vm/src/vm/mod.rs b/crates/vm/src/vm/mod.rs index 28e59a2f477..9f2d6b25013 100644 --- a/crates/vm/src/vm/mod.rs +++ b/crates/vm/src/vm/mod.rs @@ -441,6 +441,7 @@ impl StopTheWorldState { self.park_detached_threads(state); if initial_countdown == 0 || self.all_non_requester_suspended(state) { self.world_stopped.store(true, Ordering::Release); + crate::common::lock::set_world_stopped(true); #[cfg(debug_assertions)] self.debug_assert_all_non_requester_suspended(state); stw_trace(format_args!( @@ -488,6 +489,7 @@ impl StopTheWorldState { } } self.world_stopped.store(true, Ordering::Release); + crate::common::lock::set_world_stopped(true); #[cfg(debug_assertions)] self.debug_assert_all_non_requester_suspended(state); stw_trace(format_args!( @@ -508,6 +510,7 @@ impl StopTheWorldState { // thread-slot initialization. self.requested.store(false, Ordering::Release); self.world_stopped.store(false, Ordering::Release); + crate::common::lock::set_world_stopped(false); #[expect( clippy::iter_over_hash_type, @@ -545,6 +548,7 @@ impl StopTheWorldState { pub fn reset_after_fork(&self) { self.requested.store(false, Ordering::Relaxed); self.world_stopped.store(false, Ordering::Relaxed); + crate::common::lock::set_world_stopped(false); self.requester.store(0, Ordering::Relaxed); self.thread_countdown.store(0, Ordering::Relaxed); // The surviving child thread inherited the exclusion taken by the @@ -957,6 +961,17 @@ impl VirtualMachine { thread::allow_threads(self, f) } + /// Re-attach the current thread for the duration of `f`, then return it to + /// where it was. The inverse of [`allow_threads`](Self::allow_threads), for + /// a callback that runs Python from inside a call this thread detached for. + /// + /// Equivalent to `PyGILState_Ensure` / `PyGILState_Release` around such a + /// callback. + #[inline] + pub fn attach_for_callback(&self, f: impl FnOnce() -> R) -> R { + thread::attach_for_callback(self, f) + } + /// Check whether the current thread is the main thread. /// Mirrors `_Py_ThreadCanHandleSignals`. #[allow(dead_code)] diff --git a/crates/vm/src/vm/thread.rs b/crates/vm/src/vm/thread.rs index 4ba0d7ffada..7b9f5102c5b 100644 --- a/crates/vm/src/vm/thread.rs +++ b/crates/vm/src/vm/thread.rs @@ -605,6 +605,82 @@ pub fn allow_threads(_vm: &VirtualMachine, f: impl FnOnce() -> R) -> R { f() } +/// Run `f` with this thread attached, then return it to where it was. +/// +/// The inverse of [`allow_threads`], for a callback that has to run Python from +/// inside a call the thread detached for — a handshake callback reaching a +/// Python `sni_callback`, say. Running that detached would execute Python on a +/// thread a stop-the-world requester counts as parked. `PyGILState_Ensure` and +/// `PyGILState_Release` bracket such a callback for the same reason. +/// +/// A thread already attached, or one with no interpreter to attach to, just +/// runs `f`. A thread a stop-the-world has already moved to SUSPENDED parks +/// here until the world starts again, because [`attach_thread`] treats that +/// state as the wait it is; that is the point of routing through it rather than +/// testing for DETACHED alone. +#[cfg(feature = "threading")] +pub fn attach_for_callback(vm: &VirtualMachine, f: impl FnOnce() -> R) -> R { + let should_transition = CURRENT_THREAD_SLOT.with(|slot| { + slot.borrow() + .as_ref() + .is_some_and(|s| s.state.load(Ordering::Acquire) != THREAD_ATTACHED) + }); + if !should_transition { + return f(); + } + + attach_thread(vm); + // Detach again even if `f` unwinds, so the `allow_threads` this is nested + // inside still finds the state it left behind. + let redetach_guard = scopeguard::guard((), |()| detach_thread()); + let result = f(); + drop(redetach_guard); + result +} + +/// No-op on non-threading builds. +#[cfg(not(feature = "threading"))] +pub fn attach_for_callback(_vm: &VirtualMachine, f: impl FnOnce() -> R) -> R { + f() +} + +/// Wait for a lock the way a blocking call waits: detached, so a +/// stop-the-world requester never has to wait for this thread to reach a +/// safepoint it cannot reach while blocked. +/// +/// Threads with no interpreter to leave — a native thread, or one whose +/// locals are already being destroyed — simply block. +/// +/// Detaching cannot park the one thread that can start the world again: +/// [`park_detached_threads`](super::StopTheWorldState) skips the requester's +/// slot outright, by thread id, and [`suspend_if_needed`] keys off a stop bit +/// never set for it. That exemption is wider than the one `_PyEval_StopTheWorld` +/// gives, where only an ATTACHED requester is skipped and a DETACHED one is +/// suspended like any other thread — so this rests on a local invariant rather +/// than on the reference behavior. +#[cfg(feature = "threading")] +fn wait_detached_from_interpreter(wait: &dyn Fn()) { + // Read the VM out before waiting: attaching afterwards reaches for the + // same thread locals, which must not still be borrowed here. + let current = VM_STACK + .try_with(|vms| vms.try_borrow().ok()?.last().copied()) + .ok() + .flatten(); + match current { + // SAFETY: entries in VM_STACK either borrow a VM for the dynamic + // scope of a set_current_vm()/enter_vm() call or point at GILSTATE_VM. + Some(vm) => allow_threads(unsafe { vm.as_ref() }, wait), + None => wait(), + } +} + +/// Teach the lock types how to detach this thread. Idempotent, so every +/// interpreter can call it while initializing. +#[cfg(feature = "threading")] +pub(crate) fn install_blocking_wait_hook() { + rustpython_common::lock::set_blocking_wait_hook(wait_detached_from_interpreter); +} + /// Called from check_signals when stop-the-world is requested. /// Transitions ATTACHED → SUSPENDED and waits until released /// (like `_PyThreadState_Suspend` + `_PyThreadState_Attach`). From 86e7edeac4ebb623b7c42407ee0a41ea3d66bc4c Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" <69878+youknowone@users.noreply.github.com> Date: Sat, 29 Aug 2026 01:12:51 +0900 Subject: [PATCH 5/5] Update malachite to 0.11 (#8604) `malachite-bigint` 0.11.0 released while the workspace pinned 0.10.0. `pymath` requires `malachite-bigint = "0"`, so a fresh resolve picks 0.11 for it and leaves the workspace on 0.10, putting two incompatible copies of `BigInt` in one graph. `crates/stdlib/src/math.rs` then fails to compile, which is what the example projects do -- they carry no lockfile and resolve fresh on every run. `malachite-q` and `malachite-base` move with it; bumping `malachite-bigint` alone splits `malachite-nz` the same way. No source change is needed. Assisted-by: Claude --- Cargo.lock | 18 +++++++++--------- Cargo.toml | 6 +++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f448ef555ca..cc1ad30a8e5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2097,9 +2097,9 @@ dependencies = [ [[package]] name = "malachite-base" -version = "0.10.0" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c6b9d4679f346f85a8f466d0171478304dab8b0e944dd38086411ab5f6100a17" +checksum = "17073d2b5f3fe81b6abec0efcbdb6933d7adbda942854095f87c2f82633eafa5" dependencies = [ "hashbrown 0.16.1", "itertools 0.14.0", @@ -2109,9 +2109,9 @@ dependencies = [ [[package]] name = "malachite-bigint" -version = "0.10.0" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5064cf3abe01ff3b80b0349936ebad6c52f7c793182d9c7992bf79ece18c0d22" +checksum = "69c389baa355653795601ac65189e4ab21a1879fc1326a0244227f7757240edf" dependencies = [ "malachite-base", "malachite-nz", @@ -2122,9 +2122,9 @@ dependencies = [ [[package]] name = "malachite-nz" -version = "0.10.0" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8a6821ab988221c35d421ba16c4f8dca101efe5ae1cbfa9831f1fdb1596c755e" +checksum = "c2f37fc9ab5654d216d8ae22b57f0b8d9f1741ef160649d37ce1864a1e308993" dependencies = [ "itertools 0.14.0", "libm", @@ -2134,9 +2134,9 @@ dependencies = [ [[package]] name = "malachite-q" -version = "0.10.0" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3cf7894cd9617e43ef5d9824633f7dfc1bffd0298880dd668f20bdffb7a6e8ea" +checksum = "6542042c11f3d94433ed4262cf5e82eb43eff687fc5bf1fe1b4b5cde2215836e" dependencies = [ "itertools 0.14.0", "libm", @@ -4180,7 +4180,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" dependencies = [ "fastrand", - "getrandom 0.3.4", + "getrandom 0.4.3", "once_cell", "rustix", "windows-sys 0.61.2", diff --git a/Cargo.toml b/Cargo.toml index 74bc202e2b2..bf44e55d967 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -246,9 +246,9 @@ log = "0.4.30" lz4_flex = "0.13" nix = { version = "0.31", features = ["fs", "user", "process", "term", "time", "signal", "ioctl", "socket", "sched", "zerocopy", "dir", "hostname", "net", "poll"] } mac_address = "1.1.3" -malachite-bigint = "0.10.0" -malachite-q = "0.10.0" -malachite-base = "0.10.0" +malachite-bigint = "0.11.0" +malachite-q = "0.11.0" +malachite-base = "0.11.0" md-5 = "0.11" memchr = { version = "2.8.1", default-features = false, features = ["alloc"] } memmap2 = "0.9.10"