Skip to content

Commit abdbca0

Browse files
eldpswp99eldpswp99
committed
resolving deadlock and redundant lookup, grammatic mistakes remove unreachable
1 parent b49a1e9 commit abdbca0

3 files changed

Lines changed: 33 additions & 49 deletions

File tree

‎vm/src/builtins/dict.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ impl PyDict {
360360

361361
#[pymethod]
362362
fn popitem(&self, vm: &VirtualMachine) -> PyResult {
363-
if let Some((key, value)) = self.entries.pop_back(vm) {
363+
if let Some((key, value)) = self.entries.pop_back() {
364364
Ok(vm.ctx.new_tuple(vec![key, value]))
365365
} else {
366366
let err_msg = vm.ctx.new_str("popitem(): dictionary is empty");

‎vm/src/builtins/set.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ impl PySetInner {
226226

227227
fn pop(&self, vm: &VirtualMachine) -> PyResult {
228228
// TODO: should be pop_front, but that requires rearranging every index
229-
if let Some((key, _)) = self.content.pop_back(vm) {
229+
if let Some((key, _)) = self.content.pop_back() {
230230
Ok(key)
231231
} else {
232232
let err_msg = vm.ctx.new_str("pop from an empty set");

‎vm/src/dictdatatype.rs‎

Lines changed: 31 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,6 @@ pub struct DictSize {
110110
entries_size: usize,
111111
used: usize,
112112
filled: usize,
113-
nentires: usize,
114113
}
115114

116115
struct GenIndexes {
@@ -207,7 +206,6 @@ impl<T> DictInner<T> {
207206
entries_size: self.entries.len(),
208207
used: self.used,
209208
filled: self.filled,
210-
nentires: self.nentries,
211209
}
212210
}
213211

@@ -242,7 +240,9 @@ impl<T: Clone> Dict<T> {
242240
let mut inner = self.write();
243241
// Update existing key
244242
if let Some(entry) = inner.entries.get_mut(index) {
245-
let entry = entry.as_mut().unwrap();
243+
let entry = entry
244+
.as_mut()
245+
.expect("The dict was changed since we did lookup. Let's try again.");
246246
if entry.index == index_index {
247247
let removed = std::mem::replace(&mut entry.value, value);
248248
// defer dec RC
@@ -286,7 +286,7 @@ impl<T: Clone> Dict<T> {
286286
if let IndexEntry::Index(index) = entry {
287287
let inner = self.read();
288288
if let Some(entry) = inner.entries.get(index) {
289-
let entry = entry.as_ref().unwrap();
289+
let entry = extract_dict_entry(entry);
290290
if entry.index == index_index {
291291
break Some(entry.value.clone());
292292
} else {
@@ -399,7 +399,7 @@ impl<T: Clone> Dict<T> {
399399
if let IndexEntry::Index(index) = entry {
400400
let inner = self.read();
401401
if let Some(entry) = inner.entries.get(index) {
402-
let entry = entry.as_ref().unwrap();
402+
let entry = extract_dict_entry(entry);
403403
if entry.index == index_index {
404404
break entry.value.clone();
405405
} else {
@@ -442,7 +442,7 @@ impl<T: Clone> Dict<T> {
442442
if let IndexEntry::Index(index) = entry {
443443
let inner = self.read();
444444
if let Some(entry) = inner.entries.get(index) {
445-
let entry = entry.as_ref().unwrap();
445+
let entry = extract_dict_entry(entry);
446446
if entry.index == index_index {
447447
break (entry.key.clone(), entry.value.clone());
448448
} else {
@@ -479,14 +479,10 @@ impl<T: Clone> Dict<T> {
479479
pub fn next_entry(&self, position: &mut EntryIndex) -> Option<(PyObjectRef, T)> {
480480
let inner = self.read();
481481
loop {
482-
let entry = inner.entries.get(*position);
482+
let entry = inner.entries.get(*position)?;
483+
*position += 1;
483484
if let Some(entry) = entry {
484-
*position += 1;
485-
if let Some(entry) = entry {
486-
break Some((entry.key.clone(), entry.value.clone()));
487-
}
488-
} else {
489-
break None;
485+
break Some((entry.key.clone(), entry.value.clone()));
490486
}
491487
}
492488
}
@@ -504,8 +500,7 @@ impl<T: Clone> Dict<T> {
504500
self.read()
505501
.entries
506502
.iter()
507-
.filter(|v| v.is_some())
508-
.map(|v| v.as_ref().unwrap().key.clone())
503+
.filter_map(|v| v.as_ref().map(|v| v.key.clone()))
509504
.collect()
510505
}
511506

@@ -577,7 +572,7 @@ impl<T: Clone> Dict<T> {
577572
return Ok(None);
578573
};
579574
let mut inner = self.write();
580-
if matches!(inner.entries.get(entry_index), Some(entry) if entry.as_ref().unwrap().index == index_index)
575+
if matches!(inner.entries.get(entry_index), Some(Some(entry)) if entry.index == index_index)
581576
{
582577
// all good
583578
} else {
@@ -586,10 +581,8 @@ impl<T: Clone> Dict<T> {
586581
};
587582
inner.indices[index_index] = IndexEntry::DUMMY;
588583
inner.used -= 1;
589-
let removed = inner.entries[entry_index].clone();
590-
inner.entries[entry_index] = None;
591-
592-
Ok(Some(removed.unwrap()))
584+
let removed = std::mem::take(&mut inner.entries[entry_index]);
585+
Ok(removed)
593586
}
594587

595588
/// Retrieve and delete a key
@@ -606,35 +599,20 @@ impl<T: Clone> Dict<T> {
606599
Ok(removed)
607600
}
608601

609-
pub fn pop_back(&self, vm: &VirtualMachine) -> Option<(PyObjectRef, T)> {
610-
let inner = self.read();
611-
let mut idx: i64 = inner.nentries as i64 - 1;
612-
613-
while idx >= 0 && inner.entries.get(idx as usize).unwrap().is_none() {
614-
idx -= 1;
602+
pub fn pop_back(&self) -> Option<(PyObjectRef, T)> {
603+
let mut inner = self.write();
604+
let mut idx = inner.nentries.checked_sub(1)?;
605+
while inner.entries.get(idx).unwrap().is_none() {
606+
idx = idx.checked_sub(1)?;
615607
}
616608

617-
if idx < 0 {
618-
None
619-
} else {
620-
let idx = idx as usize;
621-
let entry = inner.entries.get(idx).unwrap().as_ref().unwrap().clone();
622-
623-
let removed_key = entry.key;
624-
let removed_item = entry.value;
625-
if let Ok(lookup) = self.lookup(vm, &removed_key, entry.hash, Some(inner)) {
626-
match self.pop_inner(lookup) {
627-
Ok(_) => {
628-
let mut inner = self.write();
629-
inner.nentries = idx as usize;
630-
Some((removed_key, removed_item))
631-
}
632-
Err(_) => None,
633-
}
634-
} else {
635-
None
636-
}
637-
}
609+
let entry = std::mem::take(&mut inner.entries[idx]).unwrap();
610+
let removed_key = entry.key;
611+
let removed_item = entry.value;
612+
613+
inner.used -= 1;
614+
inner.indices[entry.index] = IndexEntry::DUMMY;
615+
Some((removed_key, removed_item))
638616
}
639617

640618
pub fn sizeof(&self) -> usize {
@@ -755,6 +733,12 @@ fn str_exact<'a>(obj: &'a PyObjectRef, vm: &VirtualMachine) -> Option<&'a PyStr>
755733
}
756734
}
757735

736+
fn extract_dict_entry<T>(option_entry: &Option<DictEntry<T>>) -> &DictEntry<T> {
737+
option_entry
738+
.as_ref()
739+
.expect("The dict was changed since we did lookup. Let's try again.")
740+
}
741+
758742
#[cfg(test)]
759743
mod tests {
760744
use super::{Dict, DictKey};

0 commit comments

Comments
 (0)