fix deadlock and clear up

This commit is contained in:
Kangzhi Shi
2021-09-23 11:27:03 +02:00
parent 4676dd902e
commit 1cd3d03951
4 changed files with 48 additions and 58 deletions

View File

@@ -587,7 +587,9 @@ impl Iterator for DictIter {
type Item = (PyObjectRef, PyObjectRef);
fn next(&mut self) -> Option<Self::Item> {
self.dict.entries.next_entry(&mut self.position)
let (position, key, value) = self.dict.entries.next_entry(self.position)?;
self.position = position;
Some((key, value))
}
fn size_hint(&self) -> (usize, Option<usize>) {
@@ -735,20 +737,21 @@ macro_rules! dict_iterator {
impl SlotIterator for $iter_name {
#[allow(clippy::redundant_closure_call)]
fn next(zelf: &PyRef<Self>, vm: &VirtualMachine) -> PyResult {
let mut status = PyRwLockWriteGuard::map(zelf.internal.write(), |x| &mut x.status);
let mut position =
PyRwLockWriteGuard::map(zelf.internal.write(), |x| &mut x.position);
if let IterStatus::Active(dict) = &*status {
let mut internal = zelf.internal.write();
if let IterStatus::Active(dict) = &internal.status {
if dict.entries.has_changed_size(&zelf.size) {
*status = IterStatus::Exhausted;
internal.status = IterStatus::Exhausted;
return Err(vm.new_runtime_error(
"dictionary changed size during iteration".to_owned(),
));
}
match dict.entries.next_entry(&mut *position) {
Some((key, value)) => Ok(($result_fn)(vm, key, value)),
match dict.entries.next_entry(internal.position) {
Some((position, key, value)) => {
internal.position = position;
Ok(($result_fn)(vm, key, value))
}
None => {
*status = IterStatus::Exhausted;
internal.status = IterStatus::Exhausted;
Err(vm.new_stop_iteration())
}
}
@@ -794,20 +797,25 @@ macro_rules! dict_iterator {
impl SlotIterator for $reverse_iter_name {
#[allow(clippy::redundant_closure_call)]
fn next(zelf: &PyRef<Self>, vm: &VirtualMachine) -> PyResult {
let mut status = PyRwLockWriteGuard::map(zelf.internal.write(), |x| &mut x.status);
let mut position =
PyRwLockWriteGuard::map(zelf.internal.write(), |x| &mut x.position);
if let IterStatus::Active(dict) = &*status {
let mut internal = zelf.internal.write();
if let IterStatus::Active(dict) = &internal.status {
if dict.entries.has_changed_size(&zelf.size) {
*status = IterStatus::Exhausted;
internal.status = IterStatus::Exhausted;
return Err(vm.new_runtime_error(
"dictionary changed size during iteration".to_owned(),
));
}
match dict.entries.prev_entry(&mut *position) {
Some((key, value)) => Ok(($result_fn)(vm, key, value)),
match dict.entries.prev_entry(internal.position) {
Some((position, key, value)) => {
if internal.position == position {
internal.status = IterStatus::Exhausted;
} else {
internal.position = position;
}
Ok(($result_fn)(vm, key, value))
}
None => {
*status = IterStatus::Exhausted;
internal.status = IterStatus::Exhausted;
Err(vm.new_stop_iteration())
}
}

View File

@@ -240,10 +240,13 @@ impl PyCallableIterator {
impl IteratorIterable for PyCallableIterator {}
impl SlotIterator for PyCallableIterator {
fn next(zelf: &PyRef<Self>, vm: &VirtualMachine) -> PyResult {
if let IterStatus::Active(callable) = &*zelf.status.read() {
// let mut status = zelf.status.write();
let status = zelf.status.upgradable_read();
if let IterStatus::Active(callable) = &*status {
let ret = callable.invoke((), vm)?;
if vm.bool_eq(&ret, &zelf.sentinel)? {
*zelf.status.write() = IterStatus::Exhausted;
// *status = IterStatus::Exhausted;
*PyRwLockUpgradableReadGuard::upgrade(status) = IterStatus::Exhausted;
Err(vm.new_stop_iteration())
} else {
Ok(ret)

View File

@@ -16,7 +16,7 @@ use crate::{
IdProtocol, PyClassImpl, PyComparisonValue, PyContext, PyObjectRef, PyRef, PyResult, PyValue,
TryFromObject, TypeProtocol,
};
use rustpython_common::lock::{PyRwLock, PyRwLockWriteGuard};
use rustpython_common::lock::PyRwLock;
use std::fmt;
pub type SetContentType = dictdatatype::Dict<()>;
@@ -842,11 +842,6 @@ impl PySetIterator {
self.internal
.read()
.length_hint(|_| Some(self.size.entries_size), vm)
// if let IterStatus::Exhausted = self.status.load() {
// 0
// } else {
// self.dict.len_from_entry_index(self.position.load())
// }
}
#[pymethod(magic)]
@@ -867,41 +862,25 @@ impl PySetIterator {
impl IteratorIterable for PySetIterator {}
impl SlotIterator for PySetIterator {
fn next(zelf: &PyRef<Self>, vm: &VirtualMachine) -> PyResult {
let mut status = PyRwLockWriteGuard::map(zelf.internal.write(), |x| &mut x.status);
let mut position = PyRwLockWriteGuard::map(zelf.internal.write(), |x| &mut x.position);
if let IterStatus::Active(dict) = &*status {
let mut internal = zelf.internal.write();
if let IterStatus::Active(dict) = &internal.status {
if dict.has_changed_size(&zelf.size) {
*status = IterStatus::Exhausted;
internal.status = IterStatus::Exhausted;
return Err(vm.new_runtime_error("set changed size during iteration".to_owned()));
}
match dict.next_entry(&mut *position) {
Some((key, _)) => Ok(key),
match dict.next_entry(internal.position) {
Some((position, key, _)) => {
internal.position = position;
Ok(key)
}
None => {
*status = IterStatus::Exhausted;
internal.status = IterStatus::Exhausted;
Err(vm.new_stop_iteration())
}
}
} else {
Err(vm.new_stop_iteration())
}
// match zelf.status.load() {
// IterStatus::Exhausted => Err(vm.new_stop_iteration()),
// IterStatus::Active => {
// if zelf.dict.has_changed_size(&zelf.size) {
// zelf.status.store(IterStatus::Exhausted);
// return Err(
// vm.new_runtime_error("set changed size during iteration".to_owned())
// );
// }
// match zelf.dict.next_entry_atomic(&zelf.position) {
// Some((key, _)) => Ok(key),
// None => {
// zelf.status.store(IterStatus::Exhausted);
// Err(vm.new_stop_iteration())
// }
// }
// }
// }
}
}

View File

@@ -478,24 +478,24 @@ impl<T: Clone> Dict<T> {
self.read().size()
}
pub fn next_entry(&self, position: &mut EntryIndex) -> Option<(PyObjectRef, T)> {
pub fn next_entry(&self, mut position: EntryIndex) -> Option<(usize, PyObjectRef, T)> {
let inner = self.read();
loop {
let entry = inner.entries.get(*position)?;
*position += 1;
let entry = inner.entries.get(position)?;
position += 1;
if let Some(entry) = entry {
break Some((entry.key.clone(), entry.value.clone()));
break Some((position, entry.key.clone(), entry.value.clone()));
}
}
}
pub fn prev_entry(&self, position: &mut EntryIndex) -> Option<(PyObjectRef, T)> {
pub fn prev_entry(&self, mut position: EntryIndex) -> Option<(usize, PyObjectRef, T)> {
let inner = self.read();
loop {
let entry = inner.entries.get(*position)?;
*position = position.checked_sub(1)?;
let entry = inner.entries.get(position)?;
position = position.saturating_sub(1);
if let Some(entry) = entry {
break Some((entry.key.clone(), entry.value.clone()));
break Some((position, entry.key.clone(), entry.value.clone()));
}
}
}