Skip to content

Commit f8d47c6

Browse files
coolreader18coolreader18
authored
Merge pull request RustPython#2324 from RustPython/coolreader18/fix-nothreading
Fix cell_lock, remove the RwLock<BufferOptions> from bytearray
2 parents f390f61 + a6805a5 commit f8d47c6

File tree

7 files changed

+121
-131
lines changed

7 files changed

+121
-131
lines changed

‎common/src/lock/cell_lock.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ unsafe impl RawRwLock for RawCellRwLock {
9393
#[inline]
9494
fn lock_exclusive(&self) {
9595
if !self.try_lock_exclusive() {
96-
deadlock("exclusively", "RwLock")
96+
deadlock("exclusively ", "RwLock")
9797
}
9898
self.state.set(WRITER_BIT)
9999
}
@@ -126,7 +126,7 @@ unsafe impl RawRwLockDowngrade for RawCellRwLock {
126126
unsafe impl RawRwLockUpgrade for RawCellRwLock {
127127
#[inline]
128128
fn lock_upgradable(&self) {
129-
if self.try_lock_upgradable() {
129+
if !self.try_lock_upgradable() {
130130
deadlock("upgradably+sharedly ", "RwLock")
131131
}
132132
}
@@ -176,7 +176,7 @@ unsafe impl RawRwLockRecursive for RawCellRwLock {
176176
#[inline]
177177
fn lock_shared_recursive(&self) {
178178
if !self.try_lock_shared_recursive() {
179-
deadlock("recursively+sharedly", "RwLock")
179+
deadlock("recursively+sharedly ", "RwLock")
180180
}
181181
}
182182

‎vm/src/builtins/bytearray.rs

Lines changed: 25 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,7 @@ use crate::bytesinner::{
1414
};
1515
use crate::byteslike::PyBytesLike;
1616
use crate::common::borrow::{BorrowedValue, BorrowedValueMut};
17-
use crate::common::lock::{
18-
PyRwLock, PyRwLockReadGuard, PyRwLockUpgradableReadGuard, PyRwLockWriteGuard,
19-
};
17+
use crate::common::lock::{PyRwLock, PyRwLockReadGuard, PyRwLockWriteGuard};
2018
use crate::function::{OptionalArg, OptionalOption};
2119
use crate::pyobject::{
2220
BorrowValue, Either, IdProtocol, IntoPyObject, PyClassImpl, PyComparisonValue, PyContext,
@@ -45,7 +43,6 @@ use std::mem::size_of;
4543
pub struct PyByteArray {
4644
inner: PyRwLock<PyBytesInner>,
4745
exports: AtomicCell<usize>,
48-
buffer_options: PyRwLock<Option<Box<BufferOptions>>>,
4946
}
5047

5148
pub type PyByteArrayRef = PyRef<PyByteArray>;
@@ -63,7 +60,6 @@ impl PyByteArray {
6360
PyByteArray {
6461
inner: PyRwLock::new(inner),
6562
exports: AtomicCell::new(0),
66-
buffer_options: PyRwLock::new(None),
6763
}
6864
}
6965

@@ -667,40 +663,42 @@ impl Comparable for PyByteArray {
667663
impl BufferProtocol for PyByteArray {
668664
fn get_buffer(zelf: &PyRef<Self>, _vm: &VirtualMachine) -> PyResult<Box<dyn Buffer>> {
669665
zelf.exports.fetch_add(1);
670-
Ok(Box::new(zelf.clone()))
666+
let buf = ByteArrayBuffer {
667+
bytearray: zelf.clone(),
668+
options: BufferOptions {
669+
readonly: false,
670+
len: zelf.len(),
671+
..Default::default()
672+
},
673+
};
674+
Ok(Box::new(buf))
671675
}
672676
}
673677

674-
impl Buffer for PyByteArrayRef {
678+
#[derive(Debug)]
679+
struct ByteArrayBuffer {
680+
bytearray: PyByteArrayRef,
681+
options: BufferOptions,
682+
}
683+
684+
impl Buffer for ByteArrayBuffer {
675685
fn obj_bytes(&self) -> BorrowedValue<[u8]> {
676-
PyRwLockReadGuard::map(self.borrow_value(), |x| x.elements.as_slice()).into()
686+
PyRwLockReadGuard::map(self.bytearray.borrow_value(), |x| x.elements.as_slice()).into()
677687
}
678688

679689
fn obj_bytes_mut(&self) -> BorrowedValueMut<[u8]> {
680-
PyRwLockWriteGuard::map(self.borrow_value_mut(), |x| x.elements.as_mut_slice()).into()
690+
PyRwLockWriteGuard::map(self.bytearray.borrow_value_mut(), |x| {
691+
x.elements.as_mut_slice()
692+
})
693+
.into()
681694
}
682695

683696
fn release(&self) {
684-
let mut w = self.buffer_options.write();
685-
if self.exports.fetch_sub(1) == 1 {
686-
*w = None;
687-
}
697+
self.bytearray.exports.fetch_sub(1);
688698
}
689699

690-
fn get_options(&self) -> BorrowedValue<BufferOptions> {
691-
let guard = self.buffer_options.upgradable_read();
692-
let guard = if guard.is_none() {
693-
let mut w = PyRwLockUpgradableReadGuard::upgrade(guard);
694-
*w = Some(Box::new(BufferOptions {
695-
readonly: false,
696-
len: self.len(),
697-
..Default::default()
698-
}));
699-
PyRwLockWriteGuard::downgrade(w)
700-
} else {
701-
PyRwLockUpgradableReadGuard::downgrade(guard)
702-
};
703-
PyRwLockReadGuard::map(guard, |x| x.as_ref().unwrap().as_ref()).into()
700+
fn get_options(&self) -> &BufferOptions {
701+
&self.options
704702
}
705703
}
706704

‎vm/src/builtins/bytes.rs

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
use bstr::ByteSlice;
22
use crossbeam_utils::atomic::AtomicCell;
3-
use rustpython_common::{
4-
borrow::{BorrowedValue, BorrowedValueMut},
5-
lock::OnceCell,
6-
};
3+
use rustpython_common::borrow::{BorrowedValue, BorrowedValueMut};
74
use std::mem::size_of;
85
use std::ops::Deref;
96

@@ -44,7 +41,6 @@ use crate::builtins::memory::{Buffer, BufferOptions};
4441
#[derive(Clone, Debug)]
4542
pub struct PyBytes {
4643
inner: PyBytesInner,
47-
buffer_options: OnceCell<Box<BufferOptions>>,
4844
}
4945

5046
pub type PyBytesRef = PyRef<PyBytes>;
@@ -61,17 +57,13 @@ impl From<Vec<u8>> for PyBytes {
6157
fn from(elements: Vec<u8>) -> Self {
6258
Self {
6359
inner: PyBytesInner { elements },
64-
buffer_options: OnceCell::new(),
6560
}
6661
}
6762
}
6863

6964
impl From<PyBytesInner> for PyBytes {
7065
fn from(inner: PyBytesInner) -> Self {
71-
Self {
72-
inner,
73-
buffer_options: OnceCell::new(),
74-
}
66+
Self { inner }
7567
}
7668
}
7769

@@ -499,13 +491,26 @@ impl PyBytes {
499491

500492
impl BufferProtocol for PyBytes {
501493
fn get_buffer(zelf: &PyRef<Self>, _vm: &VirtualMachine) -> PyResult<Box<dyn Buffer>> {
502-
Ok(Box::new(zelf.clone()))
494+
let buf = BytesBuffer {
495+
bytes: zelf.clone(),
496+
options: BufferOptions {
497+
len: zelf.len(),
498+
..Default::default()
499+
},
500+
};
501+
Ok(Box::new(buf))
503502
}
504503
}
505504

506-
impl Buffer for PyBytesRef {
505+
#[derive(Debug)]
506+
struct BytesBuffer {
507+
bytes: PyBytesRef,
508+
options: BufferOptions,
509+
}
510+
511+
impl Buffer for BytesBuffer {
507512
fn obj_bytes(&self) -> BorrowedValue<[u8]> {
508-
self.inner.elements.as_slice().into()
513+
self.bytes.borrow_value().into()
509514
}
510515

511516
fn obj_bytes_mut(&self) -> BorrowedValueMut<[u8]> {
@@ -514,16 +519,8 @@ impl Buffer for PyBytesRef {
514519

515520
fn release(&self) {}
516521

517-
fn get_options(&self) -> BorrowedValue<BufferOptions> {
518-
self.buffer_options
519-
.get_or_init(|| {
520-
Box::new(BufferOptions {
521-
len: self.len(),
522-
..Default::default()
523-
})
524-
})
525-
.as_ref()
526-
.into()
522+
fn get_options(&self) -> &BufferOptions {
523+
&self.options
527524
}
528525
}
529526

‎vm/src/builtins/memory.rs

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ impl Drop for RcBuffer {
7171
}
7272
}
7373
impl Buffer for RcBuffer {
74-
fn get_options(&self) -> BorrowedValue<BufferOptions> {
74+
fn get_options(&self) -> &BufferOptions {
7575
self.0.get_options()
7676
}
7777
fn obj_bytes(&self) -> BorrowedValue<[u8]> {
@@ -93,22 +93,20 @@ impl Buffer for RcBuffer {
9393
}
9494

9595
pub trait Buffer: Debug + PyThreadingConstraint {
96-
fn get_options(&self) -> BorrowedValue<BufferOptions>;
96+
fn get_options(&self) -> &BufferOptions;
9797
fn obj_bytes(&self) -> BorrowedValue<[u8]>;
9898
fn obj_bytes_mut(&self) -> BorrowedValueMut<[u8]>;
9999
fn release(&self);
100100

101101
fn as_contiguous(&self) -> Option<BorrowedValue<[u8]>> {
102-
let options = self.get_options();
103-
if !options.contiguous {
102+
if !self.get_options().contiguous {
104103
return None;
105104
}
106105
Some(self.obj_bytes())
107106
}
108107

109108
fn as_contiguous_mut(&self) -> Option<BorrowedValueMut<[u8]>> {
110-
let options = self.get_options();
111-
if !options.contiguous {
109+
if !self.get_options().contiguous {
112110
return None;
113111
}
114112
Some(self.obj_bytes_mut())
@@ -716,7 +714,7 @@ impl PyMemoryView {
716714
let other = try_buffer_from_object(vm, other)?;
717715

718716
let a_options = &zelf.options;
719-
let b_options = &*other.get_options();
717+
let b_options = other.get_options();
720718

721719
if a_options.len != b_options.len
722720
|| a_options.ndim != b_options.ndim
@@ -788,8 +786,8 @@ impl BufferProtocol for PyMemoryView {
788786
}
789787

790788
impl Buffer for PyMemoryViewRef {
791-
fn get_options(&self) -> BorrowedValue<BufferOptions> {
792-
(&self.options).into()
789+
fn get_options(&self) -> &BufferOptions {
790+
&self.options
793791
}
794792

795793
fn obj_bytes(&self) -> BorrowedValue<[u8]> {
@@ -807,8 +805,7 @@ impl Buffer for PyMemoryViewRef {
807805
}
808806

809807
fn as_contiguous(&self) -> Option<BorrowedValue<[u8]>> {
810-
let options = self.get_options();
811-
if !options.contiguous {
808+
if !self.options.contiguous {
812809
return None;
813810
}
814811
Some(BorrowedValue::map(self.obj_bytes(), |x| {
@@ -817,8 +814,7 @@ impl Buffer for PyMemoryViewRef {
817814
}
818815

819816
fn as_contiguous_mut(&self) -> Option<BorrowedValueMut<[u8]>> {
820-
let options = self.get_options();
821-
if !options.contiguous {
817+
if !self.options.contiguous {
822818
return None;
823819
}
824820
Some(BorrowedValueMut::map(self.obj_bytes_mut(), |x| {

‎vm/src/byteslike.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,10 @@ pub fn try_rw_bytes_like<R>(
9797
impl PyRwBytesLike {
9898
pub fn new(vm: &VirtualMachine, obj: &PyObjectRef) -> PyResult<Self> {
9999
let buffer = try_buffer_from_object(vm, &obj)?;
100-
if !buffer.get_options().contiguous {
100+
let options = buffer.get_options();
101+
if !options.contiguous {
101102
Err(vm.new_type_error("non-contiguous buffer is not a bytes-like object".to_owned()))
102-
} else if buffer.get_options().readonly {
103+
} else if options.readonly {
103104
Err(vm.new_type_error("buffer is not a read-write bytes-like object".to_owned()))
104105
} else {
105106
Ok(Self(buffer))

‎vm/src/stdlib/array.rs

Lines changed: 25 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use crate::builtins::slice::PySliceRef;
88
use crate::common::borrow::{BorrowedValue, BorrowedValueMut};
99
use crate::common::lock::{
1010
PyMappedRwLockReadGuard, PyMappedRwLockWriteGuard, PyRwLock, PyRwLockReadGuard,
11-
PyRwLockUpgradableReadGuard, PyRwLockWriteGuard,
11+
PyRwLockWriteGuard,
1212
};
1313
use crate::function::OptionalArg;
1414
use crate::pyobject::{
@@ -467,7 +467,6 @@ fn f64_try_into_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult<f
467467
pub struct PyArray {
468468
array: PyRwLock<ArrayContentType>,
469469
exports: AtomicCell<usize>,
470-
buffer_options: PyRwLock<Option<Box<BufferOptions>>>,
471470
}
472471

473472
pub type PyArrayRef = PyRef<PyArray>;
@@ -483,7 +482,6 @@ impl From<ArrayContentType> for PyArray {
483482
PyArray {
484483
array: PyRwLock::new(array),
485484
exports: AtomicCell::new(0),
486-
buffer_options: PyRwLock::new(None),
487485
}
488486
}
489487
}
@@ -852,43 +850,42 @@ impl Comparable for PyArray {
852850
impl BufferProtocol for PyArray {
853851
fn get_buffer(zelf: &PyRef<Self>, _vm: &VirtualMachine) -> PyResult<Box<dyn Buffer>> {
854852
zelf.exports.fetch_add(1);
855-
Ok(Box::new(zelf.clone()))
853+
let array = zelf.borrow_value();
854+
let buf = ArrayBuffer {
855+
array: zelf.clone(),
856+
options: BufferOptions {
857+
readonly: false,
858+
len: array.len(),
859+
itemsize: array.itemsize(),
860+
format: array.typecode_str().into(),
861+
..Default::default()
862+
},
863+
};
864+
Ok(Box::new(buf))
856865
}
857866
}
858867

859-
impl Buffer for PyArrayRef {
868+
#[derive(Debug)]
869+
struct ArrayBuffer {
870+
array: PyArrayRef,
871+
options: BufferOptions,
872+
}
873+
874+
impl Buffer for ArrayBuffer {
860875
fn obj_bytes(&self) -> BorrowedValue<[u8]> {
861-
self.get_bytes().into()
876+
self.array.get_bytes().into()
862877
}
863878

864879
fn obj_bytes_mut(&self) -> BorrowedValueMut<[u8]> {
865-
self.get_bytes_mut().into()
880+
self.array.get_bytes_mut().into()
866881
}
867882

868883
fn release(&self) {
869-
let mut w = self.buffer_options.write();
870-
if self.exports.fetch_sub(1) == 1 {
871-
*w = None;
872-
}
884+
self.array.exports.fetch_sub(1);
873885
}
874886

875-
fn get_options(&self) -> BorrowedValue<BufferOptions> {
876-
let guard = self.buffer_options.upgradable_read();
877-
let guard = if guard.is_none() {
878-
let mut w = PyRwLockUpgradableReadGuard::upgrade(guard);
879-
let array = &*self.borrow_value();
880-
*w = Some(Box::new(BufferOptions {
881-
readonly: false,
882-
len: array.len(),
883-
itemsize: array.itemsize(),
884-
format: array.typecode_str().into(),
885-
..Default::default()
886-
}));
887-
PyRwLockWriteGuard::downgrade(w)
888-
} else {
889-
PyRwLockUpgradableReadGuard::downgrade(guard)
890-
};
891-
PyRwLockReadGuard::map(guard, |x| x.as_ref().unwrap().as_ref()).into()
887+
fn get_options(&self) -> &BufferOptions {
888+
&self.options
892889
}
893890
}
894891

0 commit comments

Comments
 (0)