Skip to content

Commit baf3ec1

Browse files
youknowoneyouknowone
authored
Merge pull request #1927 from palaviv/threading-stdlib-3
Convert more stdlib objects to thread safe
2 parents 559d91a + fc21d72 commit baf3ec1

File tree

5 files changed

+90
-55
lines changed

5 files changed

+90
-55
lines changed

‎vm/src/stdlib/socket.rs

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1-
use std::cell::{Cell, Ref, RefCell};
21
use std::io::{self, prelude::*};
32
use std::net::{Ipv4Addr, Shutdown, SocketAddr, ToSocketAddrs};
3+
use std::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard};
44
use std::time::Duration;
55

66
use byteorder::{BigEndian, ByteOrder};
7+
use crossbeam_utils::atomic::AtomicCell;
78
use gethostname::gethostname;
89
#[cfg(all(unix, not(target_os = "redox")))]
910
use nix::unistd::sethostname;
@@ -21,7 +22,8 @@ use crate::obj::objstr::{PyString, PyStringRef};
2122
use crate::obj::objtuple::PyTupleRef;
2223
use crate::obj::objtype::PyClassRef;
2324
use crate::pyobject::{
24-
Either, IntoPyObject, PyClassImpl, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject,
25+
Either, IntoPyObject, PyClassImpl, PyObjectRef, PyRef, PyResult, PyValue, ThreadSafe,
26+
TryFromObject,
2527
};
2628
use crate::vm::VirtualMachine;
2729

@@ -44,12 +46,14 @@ mod c {
4446
#[pyclass]
4547
#[derive(Debug)]
4648
pub struct PySocket {
47-
kind: Cell<i32>,
48-
family: Cell<i32>,
49-
proto: Cell<i32>,
50-
sock: RefCell<Socket>,
49+
kind: AtomicCell<i32>,
50+
family: AtomicCell<i32>,
51+
proto: AtomicCell<i32>,
52+
sock: RwLock<Socket>,
5153
}
5254

55+
impl ThreadSafe for PySocket {}
56+
5357
impl PyValue for PySocket {
5458
fn class(vm: &VirtualMachine) -> PyClassRef {
5559
vm.class("_socket", "socket")
@@ -60,17 +64,21 @@ pub type PySocketRef = PyRef<PySocket>;
6064

6165
#[pyimpl(flags(BASETYPE))]
6266
impl PySocket {
63-
fn sock(&self) -> Ref<Socket> {
64-
self.sock.borrow()
67+
fn sock(&self) -> RwLockReadGuard<'_, Socket> {
68+
self.sock.read().unwrap()
69+
}
70+
71+
fn sock_mut(&self) -> RwLockWriteGuard<'_, Socket> {
72+
self.sock.write().unwrap()
6573
}
6674

6775
#[pyslot]
6876
fn tp_new(cls: PyClassRef, _args: PyFuncArgs, vm: &VirtualMachine) -> PyResult<PyRef<Self>> {
6977
PySocket {
70-
kind: Cell::default(),
71-
family: Cell::default(),
72-
proto: Cell::default(),
73-
sock: RefCell::new(invalid_sock()),
78+
kind: AtomicCell::default(),
79+
family: AtomicCell::default(),
80+
proto: AtomicCell::default(),
81+
sock: RwLock::new(invalid_sock()),
7482
}
7583
.into_ref_with_type(vm, cls)
7684
}
@@ -103,12 +111,12 @@ impl PySocket {
103111
)
104112
.map_err(|err| convert_sock_error(vm, err))?;
105113

106-
self.family.set(family);
107-
self.kind.set(socket_kind);
108-
self.proto.set(proto);
114+
self.family.store(family);
115+
self.kind.store(socket_kind);
116+
self.proto.store(proto);
109117
sock
110118
};
111-
self.sock.replace(sock);
119+
*self.sock.write().unwrap() = sock;
112120
Ok(())
113121
}
114122

@@ -191,7 +199,7 @@ impl PySocket {
191199
#[pymethod]
192200
fn sendall(&self, bytes: PyBytesLike, vm: &VirtualMachine) -> PyResult<()> {
193201
bytes
194-
.with_ref(|b| self.sock.borrow_mut().write_all(b))
202+
.with_ref(|b| self.sock_mut().write_all(b))
195203
.map_err(|err| convert_sock_error(vm, err))
196204
}
197205

@@ -206,11 +214,11 @@ impl PySocket {
206214

207215
#[pymethod]
208216
fn close(&self) {
209-
self.sock.replace(invalid_sock());
217+
*self.sock_mut() = invalid_sock();
210218
}
211219
#[pymethod]
212220
fn detach(&self) -> RawSocket {
213-
into_sock_fileno(self.sock.replace(invalid_sock()))
221+
into_sock_fileno(std::mem::replace(&mut *self.sock_mut(), invalid_sock()))
214222
}
215223

216224
#[pymethod]
@@ -384,29 +392,29 @@ impl PySocket {
384392

385393
#[pyproperty(name = "type")]
386394
fn kind(&self) -> i32 {
387-
self.kind.get()
395+
self.kind.load()
388396
}
389397
#[pyproperty]
390398
fn family(&self) -> i32 {
391-
self.family.get()
399+
self.family.load()
392400
}
393401
#[pyproperty]
394402
fn proto(&self) -> i32 {
395-
self.proto.get()
403+
self.proto.load()
396404
}
397405
}
398406

399407
impl io::Read for PySocketRef {
400408
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
401-
<Socket as io::Read>::read(&mut self.sock.borrow_mut(), buf)
409+
<Socket as io::Read>::read(&mut self.sock_mut(), buf)
402410
}
403411
}
404412
impl io::Write for PySocketRef {
405413
fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
406-
<Socket as io::Write>::write(&mut self.sock.borrow_mut(), buf)
414+
<Socket as io::Write>::write(&mut self.sock_mut(), buf)
407415
}
408416
fn flush(&mut self) -> io::Result<()> {
409-
<Socket as io::Write>::flush(&mut self.sock.borrow_mut())
417+
<Socket as io::Write>::flush(&mut self.sock_mut())
410418
}
411419
}
412420

‎vm/src/stdlib/subprocess.rs

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,31 @@
1-
use std::cell::RefCell;
21
use std::ffi::OsString;
32
use std::fs::File;
43
use std::io::ErrorKind;
4+
use std::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard};
55
use std::time::Duration;
66

77
use crate::function::OptionalArg;
88
use crate::obj::objbytes::PyBytesRef;
99
use crate::obj::objlist::PyListRef;
1010
use crate::obj::objstr::{self, PyStringRef};
1111
use crate::obj::objtype::PyClassRef;
12-
use crate::pyobject::{Either, IntoPyObject, PyObjectRef, PyRef, PyResult, PyValue};
12+
use crate::pyobject::{Either, IntoPyObject, PyObjectRef, PyRef, PyResult, PyValue, ThreadSafe};
1313
use crate::stdlib::io::io_open;
1414
use crate::stdlib::os::{convert_io_error, raw_file_number, rust_file};
1515
use crate::vm::VirtualMachine;
1616

1717
#[derive(Debug)]
1818
struct Popen {
19-
process: RefCell<subprocess::Popen>,
19+
process: RwLock<subprocess::Popen>,
2020
args: PyObjectRef,
2121
}
2222

23+
// Remove once https://github.com/hniksic/rust-subprocess/issues/42 is resolved
24+
#[cfg(windows)]
25+
unsafe impl Sync for Popen {}
26+
27+
impl ThreadSafe for Popen {}
28+
2329
impl PyValue for Popen {
2430
fn class(vm: &VirtualMachine) -> PyClassRef {
2531
vm.class("_subprocess", "Popen")
@@ -103,6 +109,14 @@ fn convert_to_file_io(file: &Option<File>, mode: &str, vm: &VirtualMachine) -> P
103109
}
104110

105111
impl PopenRef {
112+
fn borrow_process(&self) -> RwLockReadGuard<'_, subprocess::Popen> {
113+
self.process.read().unwrap()
114+
}
115+
116+
fn borrow_process_mut(&self) -> RwLockWriteGuard<'_, subprocess::Popen> {
117+
self.process.write().unwrap()
118+
}
119+
106120
fn new(cls: PyClassRef, args: PopenArgs, vm: &VirtualMachine) -> PyResult<PopenRef> {
107121
let stdin = convert_redirection(args.stdin, vm)?;
108122
let stdout = convert_redirection(args.stdout, vm)?;
@@ -130,27 +144,26 @@ impl PopenRef {
130144
.map_err(|s| vm.new_os_error(format!("Could not start program: {}", s)))?;
131145

132146
Popen {
133-
process: RefCell::new(process),
147+
process: RwLock::new(process),
134148
args: args.args.into_object(),
135149
}
136150
.into_ref_with_type(vm, cls)
137151
}
138152

139153
fn poll(self) -> Option<subprocess::ExitStatus> {
140-
self.process.borrow_mut().poll()
154+
self.borrow_process_mut().poll()
141155
}
142156

143157
fn return_code(self) -> Option<subprocess::ExitStatus> {
144-
self.process.borrow().exit_status()
158+
self.borrow_process().exit_status()
145159
}
146160

147161
fn wait(self, args: PopenWaitArgs, vm: &VirtualMachine) -> PyResult<i64> {
148162
let timeout = match args.timeout {
149163
Some(timeout) => self
150-
.process
151-
.borrow_mut()
164+
.borrow_process_mut()
152165
.wait_timeout(Duration::new(timeout, 0)),
153-
None => self.process.borrow_mut().wait().map(Some),
166+
None => self.borrow_process_mut().wait().map(Some),
154167
}
155168
.map_err(|s| vm.new_os_error(format!("Could not start program: {}", s)))?;
156169
if let Some(exit) = timeout {
@@ -167,27 +180,25 @@ impl PopenRef {
167180
}
168181

169182
fn stdin(self, vm: &VirtualMachine) -> PyResult {
170-
convert_to_file_io(&self.process.borrow().stdin, "wb", vm)
183+
convert_to_file_io(&self.borrow_process().stdin, "wb", vm)
171184
}
172185

173186
fn stdout(self, vm: &VirtualMachine) -> PyResult {
174-
convert_to_file_io(&self.process.borrow().stdout, "rb", vm)
187+
convert_to_file_io(&self.borrow_process().stdout, "rb", vm)
175188
}
176189

177190
fn stderr(self, vm: &VirtualMachine) -> PyResult {
178-
convert_to_file_io(&self.process.borrow().stderr, "rb", vm)
191+
convert_to_file_io(&self.borrow_process().stderr, "rb", vm)
179192
}
180193

181194
fn terminate(self, vm: &VirtualMachine) -> PyResult<()> {
182-
self.process
183-
.borrow_mut()
195+
self.borrow_process_mut()
184196
.terminate()
185197
.map_err(|err| convert_io_error(vm, err))
186198
}
187199

188200
fn kill(self, vm: &VirtualMachine) -> PyResult<()> {
189-
self.process
190-
.borrow_mut()
201+
self.borrow_process_mut()
191202
.kill()
192203
.map_err(|err| convert_io_error(vm, err))
193204
}
@@ -202,7 +213,7 @@ impl PopenRef {
202213
OptionalArg::Present(ref bytes) => Some(bytes.get_value().to_vec()),
203214
OptionalArg::Missing => None,
204215
};
205-
let mut communicator = self.process.borrow_mut().communicate_start(bytes);
216+
let mut communicator = self.borrow_process_mut().communicate_start(bytes);
206217
if let OptionalArg::Present(timeout) = args.timeout {
207218
communicator = communicator.limit_time(Duration::new(timeout, 0));
208219
}
@@ -217,7 +228,7 @@ impl PopenRef {
217228
}
218229

219230
fn pid(self) -> Option<u32> {
220-
self.process.borrow().pid()
231+
self.borrow_process().pid()
221232
}
222233

223234
fn enter(self) -> Self {
@@ -230,7 +241,7 @@ impl PopenRef {
230241
_exception_value: PyObjectRef,
231242
_traceback: PyObjectRef,
232243
) {
233-
let mut process = self.process.borrow_mut();
244+
let mut process = self.borrow_process_mut();
234245
process.stdout.take();
235246
process.stdin.take();
236247
process.stderr.take();

‎vm/src/stdlib/symtable.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use rustpython_parser::parser;
55

66
use crate::obj::objstr::PyStringRef;
77
use crate::obj::objtype::PyClassRef;
8-
use crate::pyobject::{PyClassImpl, PyObjectRef, PyRef, PyResult, PyValue};
8+
use crate::pyobject::{PyClassImpl, PyObjectRef, PyRef, PyResult, PyValue, ThreadSafe};
99
use crate::vm::VirtualMachine;
1010

1111
pub fn make_module(vm: &VirtualMachine) -> PyObjectRef {
@@ -72,6 +72,8 @@ struct PySymbolTable {
7272
symtable: symboltable::SymbolTable,
7373
}
7474

75+
impl ThreadSafe for PySymbolTable {}
76+
7577
impl fmt::Debug for PySymbolTable {
7678
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
7779
write!(f, "SymbolTable()")
@@ -158,6 +160,8 @@ struct PySymbol {
158160
symbol: symboltable::Symbol,
159161
}
160162

163+
impl ThreadSafe for PySymbol {}
164+
161165
impl fmt::Debug for PySymbol {
162166
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
163167
write!(f, "Symbol()")

‎vm/src/stdlib/unicodedata.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
use crate::function::OptionalArg;
66
use crate::obj::objstr::PyStringRef;
77
use crate::obj::objtype::PyClassRef;
8-
use crate::pyobject::{PyClassImpl, PyObject, PyObjectRef, PyResult, PyValue};
8+
use crate::pyobject::{PyClassImpl, PyObject, PyObjectRef, PyResult, PyValue, ThreadSafe};
99
use crate::vm::VirtualMachine;
1010

1111
use itertools::Itertools;
@@ -60,6 +60,8 @@ struct PyUCD {
6060
unic_version: UnicodeVersion,
6161
}
6262

63+
impl ThreadSafe for PyUCD {}
64+
6365
impl PyValue for PyUCD {
6466
fn class(vm: &VirtualMachine) -> PyClassRef {
6567
vm.class("unicodedata", "UCD")

0 commit comments

Comments
 (0)