From 13cbff7eab750e2514a5303f87a26a7f3bf28162 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Bennet=20Ble=C3=9Fmann?= Date: Mon, 28 Aug 2023 23:04:30 +0200 Subject: [PATCH] [WIP] fix deadlock in AtomTable::build_with --- src/arena.rs | 7 +- src/atom_table.rs | 164 +++++++++++++++++----------- src/codegen.rs | 9 +- src/forms.rs | 6 +- src/heap_iter.rs | 56 +++------- src/heap_print.rs | 8 +- src/indexing.rs | 9 +- src/machine/compile.rs | 10 +- src/machine/copier.rs | 14 +-- src/machine/gc.rs | 14 +-- src/machine/heap.rs | 8 +- src/machine/loader.rs | 24 ++--- src/machine/machine_state.rs | 8 +- src/machine/machine_state_impl.rs | 8 +- src/machine/mod.rs | 4 +- src/machine/partial_string.rs | 73 ++++--------- src/machine/preprocessor.rs | 28 ++--- src/machine/system_calls.rs | 170 +++++++++++------------------- src/parser/ast.rs | 4 +- src/parser/lexer.rs | 9 +- src/parser/parser.rs | 42 +++----- src/read.rs | 11 +- src/repl_helper.rs | 6 +- tests/scryer/issues.rs | 14 --- 24 files changed, 294 insertions(+), 412 deletions(-) diff --git a/src/arena.rs b/src/arena.rs index 0ecec650..b2e43466 100644 --- a/src/arena.rs +++ b/src/arena.rs @@ -1003,11 +1003,8 @@ mod tests { // complete string - let pstr_var_cell = put_partial_string( - &mut wam.machine_st.heap, - "ronan", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + let pstr_var_cell = + put_partial_string(&mut wam.machine_st.heap, "ronan", &wam.machine_st.atom_tbl); let pstr_cell = wam.machine_st.heap[pstr_var_cell.get_value() as usize]; assert_eq!(pstr_cell.get_tag(), HeapCellValueTag::PStr); diff --git a/src/atom_table.rs b/src/atom_table.rs index 704bc523..b0042e50 100644 --- a/src/atom_table.rs +++ b/src/atom_table.rs @@ -46,16 +46,16 @@ impl From for Atom { } } -impl indexmap::Equivalent for str { - fn equivalent(&self, atom: &Atom) -> bool { - &*atom.as_str() == self +impl indexmap::Equivalent for LookupKey<'_, '_> { + fn equivalent(&self, key: &Atom) -> bool { + &*key.as_str_with_table(self.0) == self.1 } } const ATOM_TABLE_INIT_SIZE: usize = 1 << 16; const ATOM_TABLE_ALIGN: usize = 8; -pub fn global_atom_table() -> &'static RwLock>> { +fn global_atom_table() -> &'static RwLock>> { #[cfg(feature = "rust_beta_channel")] { // const Weak::new will be stabilized in 1.73 which is currently in beta, @@ -71,6 +71,22 @@ pub fn global_atom_table() -> &'static RwLock>> { } } +fn owned_atom_table_read_guard() -> Option> { + let atom_table = global_atom_table().blocking_read().upgrade()?; + + let guard = { + // some test don't start a Runtime + if let Ok(handle) = Handle::try_current() { + handle.block_on(atom_table.read_owned()) + } else { + tokio::runtime::Runtime::new() + .unwrap() + .block_on(atom_table.read_owned()) + } + }; + Some(guard) +} + impl RawBlockTraits for AtomTable { #[inline] fn init_size() -> usize { @@ -184,36 +200,28 @@ impl Atom { } #[inline(always)] - pub fn as_ptr(self) -> Option> { + pub fn as_ptr_with_table<'at>(&self, atom_table: &'at AtomTable) -> Option<&'at u8> { if self.is_static() { None } else { - let atom_table = global_atom_table() - .blocking_read() - .upgrade() - .expect("We should only have an Atom while there is an AtomTable"); - - #[cfg(not(test))] - let guard = Handle::current().block_on(atom_table.read_owned()); - - #[cfg(test)] - let guard = { - if let Ok(handle) = Handle::try_current() { - handle.block_on(atom_table.read_owned()) - } else { - tokio::runtime::Runtime::new() - .unwrap() - .block_on(atom_table.read_owned()) - } - }; - - Some(OwnedRwLockReadGuard::map(guard, |atom_table| unsafe { + unsafe { atom_table .buf() .offset(((self.index as usize) - (STRINGS.len() << 3)) as isize) .as_ref() - .unwrap() - })) + } + } + } + + #[inline(always)] + pub fn as_ptr(self) -> Option> { + if self.is_static() { + None + } else { + let guard = owned_atom_table_read_guard() + .expect("We should only have an Atom while there is an AtomTable"); + OwnedRwLockReadGuard::try_map(guard, |atom_table| self.as_ptr_with_table(atom_table)) + .ok() } } @@ -227,10 +235,9 @@ impl Atom { if self.is_static() { STRINGS[(self.index >> 3) as usize].len() } else { - unsafe { - ptr::read(self.as_ptr().unwrap().deref() as *const u8 as *const AtomHeader).len() - as _ - } + let ptr = self.as_ptr().unwrap(); + let ptr = ptr.deref() as *const u8 as *const AtomHeader; + unsafe { ptr::read(ptr) }.len() as _ } } @@ -253,34 +260,46 @@ impl Atom { } } + #[inline(always)] + pub fn as_str_with_table<'at>(&self, atom_table: &'at AtomTable) -> &'at str { + if let Some(ptr) = self.as_ptr_with_table(atom_table) { + let header = unsafe { ptr::read::(ptr as *const u8 as *const AtomHeader) }; + let len = header.len() as usize; + let buf = (unsafe { (ptr as *const u8).offset(mem::size_of::() as isize) }) + as *mut u8; + + unsafe { str::from_utf8_unchecked(slice::from_raw_parts(buf, len)) } + } else { + &STRINGS[(self.index >> 3) as usize] + } + } + + #[track_caller] #[inline] pub fn as_str(&self) -> AtomString<'static> { - if let Some(ptr_guard) = self.as_ptr() { - AtomString::Dynamic(OwnedRwLockReadGuard::map(ptr_guard, |ptr| { - let header = - unsafe { ptr::read::(ptr as *const u8 as *const AtomHeader) }; - let len = header.len() as usize; - let buf = - (unsafe { (ptr as *const u8).offset(mem::size_of::() as isize) }) - as *mut u8; - - unsafe { str::from_utf8_unchecked(slice::from_raw_parts(buf, len)) } - })) + if self.is_static() { + AtomString::Static(STRINGS[(self.index >> 3) as usize]) } else { - return AtomString::Static(STRINGS[(self.index >> 3) as usize]); + let guard = owned_atom_table_read_guard() + .expect("We should only have an Atom while there is an AtomTable"); + AtomString::Dynamic(OwnedRwLockReadGuard::map(guard, |atom_table| { + self.as_str_with_table(atom_table) + })) } } - pub fn defrock_brackets(&self, atom_tbl: &mut AtomTable) -> Self { + pub fn defrock_brackets(&self, atom_tbl: &Arc>) -> Self { let s = self.as_str(); - let s = if s.starts_with('(') && s.ends_with(')') { + let sub_str = if s.starts_with('(') && s.ends_with(')') { &s['('.len_utf8()..s.len() - ')'.len_utf8()] } else { return *self; }; - atom_tbl.build_with(s) + let val = sub_str.to_string(); + drop(s); // wee need to drop s as it holds a read lock on the AtomTable and build_with may need to acquire a write lock + AtomTable::build_with(&atom_tbl, &val) } } @@ -307,7 +326,7 @@ impl Ord for Atom { #[derive(Debug)] pub struct AtomTable { block: RawBlock, - pub table: IndexSet, + pub table: RwLock>, } impl Drop for AtomTable { @@ -316,11 +335,19 @@ impl Drop for AtomTable { } } +struct LookupKey<'table, 'key>(&'table AtomTable, &'key str); + +impl Hash for LookupKey<'_, '_> { + fn hash(&self, state: &mut H) { + self.1.hash(state); + } +} + impl AtomTable { #[inline] pub fn new() -> Arc> { let upgraded = global_atom_table().blocking_read().upgrade(); - // don't inline upgraded, temporary will be dropped too late in case of None + // don't inline upgraded, otherwise temporary will be dropped too late in case of None if let Some(atom_table) = upgraded { atom_table } else { @@ -331,7 +358,7 @@ impl AtomTable { } else { let atom_table = Arc::new(RwLock::new(Self { block: RawBlock::new(), - table: IndexSet::new(), + table: RwLock::new(IndexSet::new()), })); *guard = Arc::downgrade(&atom_table); atom_table @@ -350,15 +377,23 @@ impl AtomTable { } #[inline(always)] - fn lookup_str(&self, string: &str) -> Option { - STATIC_ATOMS_MAP - .get(string) - .or_else(|| self.table.get(string)) - .cloned() + fn lookup_str(self: &AtomTable, string: &str) -> Option { + STATIC_ATOMS_MAP.get(string).cloned().or_else(|| { + self.table + .blocking_read() + .get(&LookupKey(self, string)) + .cloned() + }) } - pub fn build_with(&mut self, string: &str) -> Atom { - if let Some(atom) = self.lookup_str(string) { + pub fn build_with(atom_table: &RwLock, string: &str) -> Atom { + let mut atom_table = loop { + if let Ok(guard) = atom_table.try_write() { + break guard; + } + }; + + if let Some(atom) = atom_table.lookup_str(string) { return atom; } @@ -368,16 +403,16 @@ impl AtomTable { let size = (size & !(align_offset - 1)) + align_offset; let len_ptr = loop { - let ptr = self.block.alloc(size); + let ptr = atom_table.block.alloc(size); if ptr.is_null() { - self.block.grow(); + atom_table.block.grow(); } else { break ptr; } }; - let ptr_base = self.block.base as usize; + let ptr_base = atom_table.block.base as usize; write_to_ptr(string, len_ptr); @@ -385,7 +420,16 @@ impl AtomTable { index: ((STRINGS.len() << 3) + len_ptr as usize - ptr_base) as u64, }; - self.table.insert(atom); + // we need to downgrade to a read so that Atom::hash can read from the AtomTable, + // so that it can calculate the hash for inserting the atom + // we can't just drop the guard as otherwise another thread could race us with another atom insertion + let atom_table = atom_table.downgrade(); + + // NOTE: there is no race between downgrade and blocking write as table is only accessed writable in this function + // and only after the write lock is acquired as we have the guard and just convert it from a write to a read guard no writer can race us + atom_table.table.blocking_write().insert(atom); + + drop(atom_table); // we need to keep the guard around till after the insert atom } diff --git a/src/codegen.rs b/src/codegen.rs index ca4e2604..65dd7817 100644 --- a/src/codegen.rs +++ b/src/codegen.rs @@ -511,7 +511,7 @@ impl<'b> CodeGenerator<'b> { TermRef::PartialString(lvl, cell, string, tail) => { self.marker .mark_non_var::(lvl, term_loc, cell, &mut target); - let atom = self.atom_tbl.blocking_write().build_with(&string); + let atom = AtomTable::build_with(&self.atom_tbl, &string); target.push_back(Target::to_pstr(lvl, atom, cell.get(), true)); self.subterm_to_instr::(tail, term_loc, &mut target); @@ -1242,12 +1242,7 @@ impl<'b> CodeGenerator<'b> { let index = code.len(); if clauses_len > 1 || self.settings.is_extensible { - code_offsets.index_term( - arg, - index, - &mut clause_index_info, - &mut self.atom_tbl.blocking_write(), - ); + code_offsets.index_term(arg, index, &mut clause_index_info, self.atom_tbl); } } diff --git a/src/forms.rs b/src/forms.rs index 54e5dd45..5f2a6ec7 100644 --- a/src/forms.rs +++ b/src/forms.rs @@ -16,6 +16,8 @@ use fxhash::FxBuildHasher; use indexmap::{IndexMap, IndexSet}; use ordered_float::OrderedFloat; +use tokio::sync::RwLock; + use std::cell::Cell; use std::collections::VecDeque; use std::convert::TryFrom; @@ -481,10 +483,10 @@ pub enum AtomOrString { impl AtomOrString { #[inline] - pub fn as_atom(&self, atom_tbl: &mut AtomTable) -> Atom { + pub fn as_atom(&self, atom_tbl: &RwLock) -> Atom { match self { &AtomOrString::Atom(atom) => atom, - AtomOrString::String(string) => atom_tbl.build_with(&string), + AtomOrString::String(string) => AtomTable::build_with(atom_tbl, &string), } } diff --git a/src/heap_iter.rs b/src/heap_iter.rs index e1a2d0a1..fc5d3416 100644 --- a/src/heap_iter.rs +++ b/src/heap_iter.rs @@ -708,11 +708,8 @@ mod tests { // first a 'dangling' partial string, later modified to be a two-part complete string, // then a three-part cyclic string involving an uncompacted list of chars. - let pstr_var_cell = put_partial_string( - &mut wam.machine_st.heap, - "abc ", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + let pstr_var_cell = + put_partial_string(&mut wam.machine_st.heap, "abc ", &wam.machine_st.atom_tbl); let pstr_cell = wam.machine_st.heap[pstr_var_cell.get_value() as usize]; { @@ -733,11 +730,8 @@ mod tests { wam.machine_st.heap.pop(); wam.machine_st.heap.push(pstr_loc_as_cell!(2)); - let pstr_second_var_cell = put_partial_string( - &mut wam.machine_st.heap, - "def", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + let pstr_second_var_cell = + put_partial_string(&mut wam.machine_st.heap, "def", &wam.machine_st.atom_tbl); let pstr_second_cell = wam.machine_st.heap[pstr_second_var_cell.get_value() as usize]; @@ -1776,11 +1770,8 @@ mod tests { // two-part complete string, then a three-part cyclic string // involving an uncompacted list of chars. - let pstr_var_cell = put_partial_string( - &mut wam.machine_st.heap, - "abc ", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + let pstr_var_cell = + put_partial_string(&mut wam.machine_st.heap, "abc ", &wam.machine_st.atom_tbl); let pstr_cell = wam.machine_st.heap[pstr_var_cell.get_value() as usize]; { @@ -1804,11 +1795,8 @@ mod tests { wam.machine_st.heap.pop(); wam.machine_st.heap.push(heap_loc_as_cell!(2)); - let pstr_second_var_cell = put_partial_string( - &mut wam.machine_st.heap, - "def", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + let pstr_second_var_cell = + put_partial_string(&mut wam.machine_st.heap, "def", &wam.machine_st.atom_tbl); let pstr_second_cell = wam.machine_st.heap[pstr_second_var_cell.get_value() as usize]; { @@ -2375,11 +2363,8 @@ mod tests { // two-part complete string, then a three-part cyclic string // involving an uncompacted list of chars. - let pstr_var_cell = put_partial_string( - &mut wam.machine_st.heap, - "abc ", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + let pstr_var_cell = + put_partial_string(&mut wam.machine_st.heap, "abc ", &wam.machine_st.atom_tbl); let pstr_cell = wam.machine_st.heap[pstr_var_cell.get_value() as usize]; { @@ -2402,11 +2387,8 @@ mod tests { wam.machine_st.heap.pop(); wam.machine_st.heap.push(pstr_loc_as_cell!(2)); - let pstr_second_var_cell = put_partial_string( - &mut wam.machine_st.heap, - "def", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + let pstr_second_var_cell = + put_partial_string(&mut wam.machine_st.heap, "def", &wam.machine_st.atom_tbl); let pstr_second_cell = wam.machine_st.heap[pstr_second_var_cell.get_value() as usize]; { @@ -2838,11 +2820,8 @@ mod tests { // two-part complete string, then a three-part cyclic string // involving an uncompacted list of chars. - let pstr_var_cell = put_partial_string( - &mut wam.machine_st.heap, - "abc ", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + let pstr_var_cell = + put_partial_string(&mut wam.machine_st.heap, "abc ", &wam.machine_st.atom_tbl); let pstr_cell = wam.machine_st.heap[pstr_var_cell.get_value() as usize]; { @@ -2862,11 +2841,8 @@ mod tests { wam.machine_st.heap.pop(); wam.machine_st.heap.push(pstr_loc_as_cell!(2)); - let pstr_second_var_cell = put_partial_string( - &mut wam.machine_st.heap, - "def", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + let pstr_second_var_cell = + put_partial_string(&mut wam.machine_st.heap, "def", &wam.machine_st.atom_tbl); let pstr_second_cell = wam.machine_st.heap[pstr_second_var_cell.get_value() as usize]; diff --git a/src/heap_print.rs b/src/heap_print.rs index 8cd175fa..c6e81da5 100644 --- a/src/heap_print.rs +++ b/src/heap_print.rs @@ -1588,7 +1588,7 @@ impl<'a, Outputter: HCValueOutputter> HCPrinter<'a, Outputter> { print_struct(self, name, arity); } (HeapCellValueTag::Char, c) => { - let name = self.atom_tbl.blocking_write().build_with(&String::from(c)); + let name = AtomTable::build_with(&self.atom_tbl, &String::from(c)); print_struct(self, name, 0); } (HeapCellValueTag::Str, s) => { @@ -1950,11 +1950,7 @@ mod tests { wam.machine_st.heap.clear(); - put_partial_string( - &mut wam.machine_st.heap, - "abc", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + put_partial_string(&mut wam.machine_st.heap, "abc", &wam.machine_st.atom_tbl); { let printer = HCPrinter::new( diff --git a/src/indexing.rs b/src/indexing.rs index 9370d557..94ae266b 100644 --- a/src/indexing.rs +++ b/src/indexing.rs @@ -6,6 +6,7 @@ use crate::instructions::*; use fxhash::FxBuildHasher; use indexmap::IndexMap; +use tokio::sync::RwLock; use std::collections::VecDeque; use std::hash::Hash; @@ -1093,7 +1094,7 @@ fn uncap_choice_seq_with_try(prelude: &mut [IndexedChoiceInstruction]) { pub(crate) fn constant_key_alternatives( constant: Literal, - atom_tbl: &mut AtomTable, + atom_tbl: &RwLock, // arena: &mut Arena, ) -> Vec { let mut constants = vec![]; @@ -1105,7 +1106,7 @@ pub(crate) fn constant_key_alternatives( } } Literal::Char(c) => { - let atom = atom_tbl.build_with(&c.to_string()); + let atom = AtomTable::build_with(&atom_tbl, &c.to_string()); constants.push(Literal::Atom(atom)); } /* @@ -1454,7 +1455,7 @@ impl CodeOffsets { fn index_constant( &mut self, - atom_tbl: &mut AtomTable, + atom_tbl: &RwLock, constant: Literal, index: usize, ) -> Vec { @@ -1511,7 +1512,7 @@ impl CodeOffsets { optimal_arg: &Term, index: usize, clause_index_info: &mut ClauseIndexInfo, - atom_tbl: &mut AtomTable, + atom_tbl: &RwLock, ) { match optimal_arg { &Term::Clause(_, atom!("."), ref terms) if terms.len() == 2 => { diff --git a/src/machine/compile.rs b/src/machine/compile.rs index c8bf3451..170bffaf 100644 --- a/src/machine/compile.rs +++ b/src/machine/compile.rs @@ -1237,12 +1237,10 @@ impl<'a, LS: LoadState<'a>> Loader<'a, LS> { if let Some(path_str) = load_context.path.to_str() { if !path_str.is_empty() { - return Some( - LS::machine_st(&mut self.payload) - .atom_tbl - .blocking_write() - .build_with(path_str), - ); + return Some(AtomTable::build_with( + &LS::machine_st(&mut self.payload).atom_tbl, + path_str, + )); } } } diff --git a/src/machine/copier.rs b/src/machine/copier.rs index 6b15298f..01f7e8bd 100644 --- a/src/machine/copier.rs +++ b/src/machine/copier.rs @@ -406,21 +406,15 @@ mod tests { wam.machine_st.heap.clear(); - let pstr_var_cell = put_partial_string( - &mut wam.machine_st.heap, - "abc ", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + let pstr_var_cell = + put_partial_string(&mut wam.machine_st.heap, "abc ", &wam.machine_st.atom_tbl); let pstr_cell = wam.machine_st.heap[pstr_var_cell.get_value() as usize]; wam.machine_st.heap.pop(); wam.machine_st.heap.push(pstr_loc_as_cell!(2)); - let pstr_second_var_cell = put_partial_string( - &mut wam.machine_st.heap, - "def", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + let pstr_second_var_cell = + put_partial_string(&mut wam.machine_st.heap, "def", &wam.machine_st.atom_tbl); let pstr_second_cell = wam.machine_st.heap[pstr_second_var_cell.get_value() as usize]; wam.machine_st.heap.pop(); diff --git a/src/machine/gc.rs b/src/machine/gc.rs index 2160fc59..5b4895bd 100644 --- a/src/machine/gc.rs +++ b/src/machine/gc.rs @@ -644,11 +644,8 @@ mod tests { // two-part complete string, then a three-part cyclic string // involving an uncompacted list of chars. - let pstr_var_cell = put_partial_string( - &mut wam.machine_st.heap, - "abc ", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + let pstr_var_cell = + put_partial_string(&mut wam.machine_st.heap, "abc ", &wam.machine_st.atom_tbl); let pstr_cell = wam.machine_st.heap[pstr_var_cell.get_value() as usize]; mark_cells(&mut wam.machine_st.heap, pstr_loc_as_cell!(0)); @@ -669,11 +666,8 @@ mod tests { wam.machine_st.heap.push(pstr_loc_as_cell!(2)); - let pstr_second_var_cell = put_partial_string( - &mut wam.machine_st.heap, - "def", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + let pstr_second_var_cell = + put_partial_string(&mut wam.machine_st.heap, "def", &wam.machine_st.atom_tbl); let pstr_second_cell = wam.machine_st.heap[pstr_second_var_cell.get_value() as usize]; mark_cells(&mut wam.machine_st.heap, pstr_loc_as_cell!(0)); diff --git a/src/machine/heap.rs b/src/machine/heap.rs index 1cdfeb74..142b6359 100644 --- a/src/machine/heap.rs +++ b/src/machine/heap.rs @@ -1,3 +1,5 @@ +use tokio::sync::RwLock; + use crate::arena::*; use crate::atom_table::*; use crate::forms::*; @@ -133,7 +135,7 @@ pub fn print_heap_terms<'a, I: Iterator>(heap: I, h: u pub(crate) fn put_complete_string( heap: &mut Heap, s: &str, - atom_tbl: &mut AtomTable, + atom_tbl: &RwLock, ) -> HeapCellValue { match allocate_pstr(heap, s, atom_tbl) { Some(h) => { @@ -160,7 +162,7 @@ pub(crate) fn put_complete_string( pub(crate) fn put_partial_string( heap: &mut Heap, s: &str, - atom_tbl: &mut AtomTable, + atom_tbl: &RwLock, ) -> HeapCellValue { match allocate_pstr(heap, s, atom_tbl) { Some(h) => { @@ -176,7 +178,7 @@ pub(crate) fn put_partial_string( pub(crate) fn allocate_pstr( heap: &mut Heap, mut src: &str, - atom_tbl: &mut AtomTable, + atom_tbl: &RwLock, ) -> Option { let orig_h = heap.len(); diff --git a/src/machine/loader.rs b/src/machine/loader.rs index d42f923c..6301fe85 100644 --- a/src/machine/loader.rs +++ b/src/machine/loader.rs @@ -1076,7 +1076,7 @@ impl<'a, LS: LoadState<'a>> Loader<'a, LS> { let export_list = machine_st.read_term_from_heap(cell)?; let atom_tbl = &mut LS::machine_st(&mut self.payload).atom_tbl; - let export_list = setup_module_export_list(export_list, &mut atom_tbl.blocking_write())?; + let export_list = setup_module_export_list(export_list, &atom_tbl)?; Ok(export_list.into_iter().collect()) } @@ -1420,7 +1420,7 @@ impl MachineState { term_stack.push(Term::PartialString(Cell::default(), string, tail)); } Ok((string, None)) => { - let atom = self.atom_tbl.blocking_write().build_with(&string); + let atom = AtomTable::build_with(&self.atom_tbl, &string); term_stack.push(Term::CompleteString(Cell::default(), atom)); } Err(cons_term) => term_stack.push(cons_term), @@ -1917,11 +1917,7 @@ impl Machine { pub(crate) fn load_context_source(&mut self) { if let Some(load_context) = self.load_contexts.last() { let path_str = load_context.path.to_str().unwrap(); - let path_atom = self - .machine_st - .atom_tbl - .blocking_write() - .build_with(path_str); + let path_atom = AtomTable::build_with(&self.machine_st.atom_tbl, path_str); self.machine_st .unify_atom(path_atom, self.machine_st.registers[1]); @@ -1935,11 +1931,8 @@ impl Machine { match load_context.path.file_name() { Some(file_name) if load_context.path.is_file() => { let file_name_str = file_name.to_str().unwrap(); - let file_name_atom = self - .machine_st - .atom_tbl - .blocking_write() - .build_with(file_name_str); + let file_name_atom = + AtomTable::build_with(&self.machine_st.atom_tbl, file_name_str); self.machine_st .unify_atom(file_name_atom, self.machine_st.registers[1]); @@ -1958,11 +1951,8 @@ impl Machine { if let Some(load_context) = self.load_contexts.last() { if let Some(directory) = load_context.path.parent() { let directory_str = directory.to_str().unwrap(); - let directory_atom = self - .machine_st - .atom_tbl - .blocking_write() - .build_with(directory_str); + let directory_atom = + AtomTable::build_with(&self.machine_st.atom_tbl, directory_str); self.machine_st .unify_atom(directory_atom, self.machine_st.registers[1]); diff --git a/src/machine/machine_state.rs b/src/machine/machine_state.rs index c5ae21a4..e099e894 100644 --- a/src/machine/machine_state.rs +++ b/src/machine/machine_state.rs @@ -205,12 +205,12 @@ pub fn pstr_loc_and_offset(heap: &[HeapCellValue], index: usize) -> (usize, Fixn fn push_var_eq_functors<'a>( heap: &mut Heap, iter: impl Iterator, - atom_tbl: &mut AtomTable, + atom_tbl: &RwLock, ) -> Vec { let mut list_of_var_eqs = vec![]; for (var, binding) in iter { - let var_atom = atom_tbl.build_with(&var.to_string()); + let var_atom = AtomTable::build_with(atom_tbl, &var.to_string()); let h = heap.len(); heap.push(atom_as_cell!(atom!("="), 2)); @@ -531,7 +531,7 @@ impl MachineState { Some((var_name, var)) } }), - &mut self.atom_tbl.blocking_write(), + &self.atom_tbl, ); let singleton_addr = self.registers[3]; @@ -617,7 +617,7 @@ impl MachineState { false } }), - &mut self.atom_tbl.blocking_write(), + &self.atom_tbl, ); for var in term_write_result.var_dict.values_mut() { diff --git a/src/machine/machine_state_impl.rs b/src/machine/machine_state_impl.rs index 991cccfa..8fd74279 100644 --- a/src/machine/machine_state_impl.rs +++ b/src/machine/machine_state_impl.rs @@ -1003,9 +1003,9 @@ impl MachineState { self.s_offset = 0; self.mode = MachineMode::Read; - put_partial_string(&mut self.heap, pstr, &mut self.atom_tbl.blocking_write()) + put_partial_string(&mut self.heap, pstr, &self.atom_tbl) } else { - put_complete_string(&mut self.heap, pstr, &mut self.atom_tbl.blocking_write()) + put_complete_string(&mut self.heap, pstr, &self.atom_tbl) } } @@ -1097,7 +1097,7 @@ impl MachineState { (name, 0, 0) } (HeapCellValueTag::Char, c) => { - (self.atom_tbl.blocking_write().build_with(&c.to_string()), 0, 0) + (AtomTable::build_with(&self.atom_tbl, &c.to_string()), 0, 0) } (HeapCellValueTag::Var | HeapCellValueTag::AttrVar | HeapCellValueTag::StackVar) => { let stub = functor_stub(atom!("call"), arity + 1); @@ -1436,7 +1436,7 @@ impl MachineState { } } (HeapCellValueTag::Char, c) => { - let c = self.atom_tbl.blocking_write().build_with(&c.to_string()); + let c = AtomTable::build_with(&self.atom_tbl, &c.to_string()); self.try_functor_fabricate_struct( c, diff --git a/src/machine/mod.rs b/src/machine/mod.rs index d67fc861..a7d80e9f 100644 --- a/src/machine/mod.rs +++ b/src/machine/mod.rs @@ -231,7 +231,7 @@ impl Machine { pub fn load_file(&mut self, path: &str, stream: Stream) { self.machine_st.registers[1] = stream_as_cell!(stream); self.machine_st.registers[2] = - atom_as_cell!(self.machine_st.atom_tbl.blocking_write().build_with(path)); + atom_as_cell!(AtomTable::build_with(&self.machine_st.atom_tbl, path)); self.run_module_predicate(atom!("loader"), (atom!("file_load"), 2)); } @@ -296,7 +296,7 @@ impl Machine { arg_pstrs.push(put_complete_string( &mut self.machine_st.heap, &arg, - &mut self.machine_st.atom_tbl.blocking_write(), + &self.machine_st.atom_tbl, )); } diff --git a/src/machine/partial_string.rs b/src/machine/partial_string.rs index 5605474d..1260ebdc 100644 --- a/src/machine/partial_string.rs +++ b/src/machine/partial_string.rs @@ -1,3 +1,5 @@ +use tokio::sync::RwLock; + use crate::atom_table::*; use crate::parser::ast::*; @@ -43,10 +45,9 @@ impl Into for PartialString { impl PartialString { #[inline] - pub(super) fn new<'a>(src: &'a str, atom_tbl: &mut AtomTable) -> Option<(Self, &'a str)> { + pub(super) fn new<'a>(src: &'a str, atom_tbl: &RwLock) -> Option<(Self, &'a str)> { let terminator_idx = scan_for_terminator(src.chars()); - let pstr = PartialString(atom_tbl.build_with(&src[..terminator_idx])); - + let pstr = PartialString(AtomTable::build_with(&atom_tbl, &src[..terminator_idx])); Some(if terminator_idx < src.as_bytes().len() { (pstr, &src[terminator_idx + 1..]) } else { @@ -805,11 +806,8 @@ mod test { fn pstr_iter_tests() { let mut wam = MockWAM::new(); - let pstr_var_cell = put_partial_string( - &mut wam.machine_st.heap, - "abc ", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + let pstr_var_cell = + put_partial_string(&mut wam.machine_st.heap, "abc ", &wam.machine_st.atom_tbl); let pstr_cell = wam.machine_st.heap[pstr_var_cell.get_value() as usize]; @@ -828,11 +826,8 @@ mod test { wam.machine_st.heap.pop(); wam.machine_st.heap.push(pstr_loc_as_cell!(2)); - let pstr_second_var_cell = put_partial_string( - &mut wam.machine_st.heap, - "def", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + let pstr_second_var_cell = + put_partial_string(&mut wam.machine_st.heap, "def", &wam.machine_st.atom_tbl); let pstr_second_cell = wam.machine_st.heap[pstr_second_var_cell.get_value() as usize]; @@ -913,21 +908,13 @@ mod test { // construct a structurally similar but different cyclic partial string // matching the one beginning at wam.machine_st.heap[0]. - put_partial_string( - &mut wam.machine_st.heap, - "ab", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + put_partial_string(&mut wam.machine_st.heap, "ab", &wam.machine_st.atom_tbl); wam.machine_st.heap.pop(); wam.machine_st.heap.push(pstr_loc_as_cell!(second_h + 2)); - put_partial_string( - &mut wam.machine_st.heap, - "c ", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + put_partial_string(&mut wam.machine_st.heap, "c ", &wam.machine_st.atom_tbl); wam.machine_st.heap.pop(); @@ -952,11 +939,7 @@ mod test { wam.machine_st.heap.clear(); - put_partial_string( - &mut wam.machine_st.heap, - "abc ", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + put_partial_string(&mut wam.machine_st.heap, "abc ", &wam.machine_st.atom_tbl); let pstr_cell = wam.machine_st.heap[0]; @@ -986,11 +969,8 @@ mod test { wam.machine_st.heap.clear(); - let cstr_var_cell = put_complete_string( - &mut wam.machine_st.heap, - "abc", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + let cstr_var_cell = + put_complete_string(&mut wam.machine_st.heap, "abc", &wam.machine_st.atom_tbl); wam.machine_st.heap.push(list_loc_as_cell!(2)); wam.machine_st.heap.push(heap_loc_as_cell!(2)); @@ -1015,11 +995,8 @@ mod test { wam.machine_st.heap.clear(); - let cstr_var_cell = put_complete_string( - &mut wam.machine_st.heap, - "abc", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + let cstr_var_cell = + put_complete_string(&mut wam.machine_st.heap, "abc", &wam.machine_st.atom_tbl); wam.machine_st.heap.push(list_loc_as_cell!(2)); wam.machine_st.heap.push(heap_loc_as_cell!(2)); // X @@ -1048,11 +1025,8 @@ mod test { wam.machine_st.heap.clear(); - let cstr_var_cell = put_complete_string( - &mut wam.machine_st.heap, - "d", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + let cstr_var_cell = + put_complete_string(&mut wam.machine_st.heap, "d", &wam.machine_st.atom_tbl); wam.machine_st.heap.push(list_loc_as_cell!(2)); wam.machine_st.heap.push(char_as_cell!('d')); @@ -1066,11 +1040,8 @@ mod test { wam.machine_st.heap.clear(); - let cstr_var_cell = put_complete_string( - &mut wam.machine_st.heap, - "abc", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + let cstr_var_cell = + put_complete_string(&mut wam.machine_st.heap, "abc", &wam.machine_st.atom_tbl); wam.machine_st.heap.push(list_loc_as_cell!(2)); wam.machine_st.heap.push(heap_loc_as_cell!(2)); @@ -1097,11 +1068,7 @@ mod test { wam.machine_st.heap.clear(); - put_complete_string( - &mut wam.machine_st.heap, - "abcdef", - &mut wam.machine_st.atom_tbl.blocking_write(), - ); + put_complete_string(&mut wam.machine_st.heap, "abcdef", &wam.machine_st.atom_tbl); wam.machine_st.heap.push(pstr_as_cell!(atom!("abc"))); wam.machine_st.heap.push(heap_loc_as_cell!(2)); diff --git a/src/machine/preprocessor.rs b/src/machine/preprocessor.rs index f867ec70..79946a49 100644 --- a/src/machine/preprocessor.rs +++ b/src/machine/preprocessor.rs @@ -8,6 +8,7 @@ use crate::machine::machine_errors::*; use crate::parser::ast::*; use indexmap::IndexSet; +use tokio::sync::RwLock; use std::cell::Cell; use std::convert::TryFrom; @@ -27,17 +28,17 @@ pub(crate) fn to_op_decl(prec: u16, spec: Atom, name: Atom) -> Result, - atom_tbl: &mut AtomTable, + atom_tbl: &RwLock, ) -> Result { let name = match terms.pop().unwrap() { Term::Literal(_, Literal::Atom(name)) => name, - Term::Literal(_, Literal::Char(c)) => atom_tbl.build_with(&c.to_string()), + Term::Literal(_, Literal::Char(c)) => AtomTable::build_with(atom_tbl, &c.to_string()), _ => return Err(CompilationError::InconsistentEntry), }; let spec = match terms.pop().unwrap() { Term::Literal(_, Literal::Atom(name)) => name, - Term::Literal(_, Literal::Char(c)) => atom_tbl.build_with(&c.to_string()), + Term::Literal(_, Literal::Char(c)) => AtomTable::build_with(atom_tbl, &c.to_string()), _ => return Err(CompilationError::InconsistentEntry), }; @@ -85,7 +86,7 @@ fn setup_predicate_indicator(term: &mut Term) -> Result, ) -> Result { setup_predicate_indicator(&mut term) .map(ModuleExport::PredicateKey) @@ -111,7 +112,7 @@ pub(crate) fn build_rule_body(vars: &[Term], body_term: Term) -> Term { pub(super) fn setup_module_export_list( mut export_list: Term, - atom_tbl: &mut AtomTable, + atom_tbl: &RwLock, ) -> Result, CompilationError> { let mut exports = vec![]; @@ -131,7 +132,7 @@ pub(super) fn setup_module_export_list( fn setup_module_decl( mut terms: Vec, - atom_tbl: &mut AtomTable, + atom_tbl: &RwLock, ) -> Result { let export_list = terms.pop().unwrap(); let name = terms.pop().unwrap(); @@ -164,7 +165,7 @@ type UseModuleExport = (ModuleSource, IndexSet); fn setup_qualified_import( mut terms: Vec, - atom_tbl: &mut AtomTable, + atom_tbl: &RwLock, ) -> Result { let mut export_list = terms.pop().unwrap(); let module_src = match terms.pop().unwrap() { @@ -313,17 +314,11 @@ pub(super) fn setup_declaration<'a, LS: LoadState<'a>>( } (atom!("module"), 2) => { let atom_tbl = &mut LS::machine_st(&mut loader.payload).atom_tbl; - Ok(Declaration::Module(setup_module_decl( - terms, - &mut atom_tbl.blocking_write(), - )?)) + Ok(Declaration::Module(setup_module_decl(terms, &atom_tbl)?)) } (atom!("op"), 3) => { let atom_tbl = &mut LS::machine_st(&mut loader.payload).atom_tbl; - Ok(Declaration::Op(setup_op_decl( - terms, - &mut atom_tbl.blocking_write(), - )?)) + Ok(Declaration::Op(setup_op_decl(terms, &atom_tbl)?)) } (atom!("non_counted_backtracking"), 1) => { let (name, arity) = setup_predicate_indicator(&mut terms.pop().unwrap())?; @@ -332,8 +327,7 @@ pub(super) fn setup_declaration<'a, LS: LoadState<'a>>( (atom!("use_module"), 1) => Ok(Declaration::UseModule(setup_use_module_decl(terms)?)), (atom!("use_module"), 2) => { let atom_tbl = &mut LS::machine_st(&mut loader.payload).atom_tbl; - let (name, exports) = - setup_qualified_import(terms, &mut atom_tbl.blocking_write())?; + let (name, exports) = setup_qualified_import(terms, &atom_tbl)?; Ok(Declaration::UseQualifiedModule(name, exports)) } diff --git a/src/machine/system_calls.rs b/src/machine/system_calls.rs index 561fdc07..47d203fd 100644 --- a/src/machine/system_calls.rs +++ b/src/machine/system_calls.rs @@ -1529,7 +1529,7 @@ impl Machine { } } (HeapCellValueTag::Char, c) => { - let name = self.machine_st.atom_tbl.blocking_write().build_with(&c.to_string()); + let name = AtomTable::build_with(&self.machine_st.atom_tbl,&c.to_string()); let h = self.machine_st.heap.len(); self.machine_st.heap.push(atom_as_cell!(name)); @@ -1804,7 +1804,7 @@ impl Machine { match hostname::get().ok() { Some(host) => match host.to_str() { Some(host) => { - let hostname = self.machine_st.atom_tbl.blocking_write().build_with(host); + let hostname = AtomTable::build_with(&self.machine_st.atom_tbl, host); let a1 = self.deref_register(1); self.machine_st.unify_atom(hostname, a1); @@ -1903,7 +1903,7 @@ impl Machine { for entry in entries { if let Ok(entry) = entry { if let Some(name) = entry.file_name().to_str() { - let name = self.machine_st.atom_tbl.blocking_write().build_with(name); + let name = AtomTable::build_with(&self.machine_st.atom_tbl, name); files.push(atom_as_cstr_cell!(name)); continue; @@ -2144,11 +2144,7 @@ impl Machine { } }; - let current_atom = self - .machine_st - .atom_tbl - .blocking_write() - .build_with(¤t); + let current_atom = AtomTable::build_with(&self.machine_st.atom_tbl, ¤t); let a1 = self.deref_register(1); self.machine_st.unify_complete_string(current_atom, a1); @@ -2189,7 +2185,7 @@ impl Machine { } }; - let canonical_atom = self.machine_st.atom_tbl.blocking_write().build_with(cs); + let canonical_atom = AtomTable::build_with(&self.machine_st.atom_tbl, cs); let a2 = self.deref_register(2); self.machine_st.unify_complete_string(canonical_atom, a2); @@ -2248,13 +2244,13 @@ impl Machine { let atom_cell = match str_like { AtomOrString::Atom(atom) => { atom_as_cell!(if atom == atom!("[]") { - self.machine_st.atom_tbl.blocking_write().build_with("") + AtomTable::build_with(&self.machine_st.atom_tbl, "") } else { atom }) } AtomOrString::String(string) => { - atom_as_cell!(self.machine_st.atom_tbl.blocking_write().build_with(&string)) + atom_as_cell!(AtomTable::build_with(&self.machine_st.atom_tbl, &string)) } }; @@ -2316,7 +2312,7 @@ impl Machine { match self.machine_st.try_from_list(self.machine_st.registers[2], stub_gen) { Ok(addrs) => { let string = self.machine_st.codes_to_string(addrs.into_iter(), stub_gen)?; - let atom = self.machine_st.atom_tbl.blocking_write().build_with(&string); + let atom = AtomTable::build_with(&self.machine_st.atom_tbl, &string); self.machine_st.bind(a1.as_var().unwrap(), atom_as_cell!(atom)); } @@ -2810,11 +2806,7 @@ impl Machine { } }; - let chars_atom = self - .machine_st - .atom_tbl - .blocking_write() - .build_with(&string.trim()); + let chars_atom = AtomTable::build_with(&self.machine_st.atom_tbl, &string.trim()); self.machine_st.unify_complete_string(chars_atom, chs); } @@ -3040,14 +3032,14 @@ impl Machine { match (name, arity) { (atom!("to_upper"), 1) => { let reg = self.machine_st.deref(self.machine_st.heap[s+1]); - let atom = self.machine_st.atom_tbl.blocking_write().build_with(&c.to_uppercase().to_string()); + let atom = AtomTable::build_with(&self.machine_st.atom_tbl, &c.to_uppercase().to_string()); let upper_str = string_as_cstr_cell!(atom); unify!(self.machine_st, reg, upper_str); self.machine_st.fail = false; } (atom!("to_lower"), 1) => { let reg = self.machine_st.deref(self.machine_st.heap[s+1]); - let atom = self.machine_st.atom_tbl.blocking_write().build_with(&c.to_lowercase().to_string()); + let atom = AtomTable::build_with(&self.machine_st.atom_tbl, &c.to_lowercase().to_string()); let lower_str = string_as_cstr_cell!(atom); unify!(self.machine_st, reg, lower_str); self.machine_st.fail = false; @@ -3570,11 +3562,7 @@ impl Machine { }; let output = self.deref_register(3); - let atom = self - .machine_st - .atom_tbl - .blocking_write() - .build_with(&string); + let atom = AtomTable::build_with(&self.machine_st.atom_tbl, &string); self.machine_st.unify_complete_string(atom, output); Ok(()) @@ -4069,7 +4057,7 @@ impl Machine { cell_as_atom!(self.machine_st.heap[s]) } (HeapCellValueTag::Char, c) => { - self.machine_st.atom_tbl.blocking_write().build_with(&c.to_string()) + AtomTable::build_with(&self.machine_st.atom_tbl, &c.to_string()) } _ => { unreachable!() @@ -4431,15 +4419,14 @@ impl Machine { let h = self.machine_st.heap.len(); let header_term = functor!( - self.machine_st - .atom_tbl - .blocking_write() - .build_with(header_name.as_str()), - [cell(string_as_cstr_cell!(self - .machine_st - .atom_tbl - .blocking_write() - .build_with(header_value.to_str().unwrap())))] + AtomTable::build_with( + &self.machine_st.atom_tbl, + header_name.as_str() + ), + [cell(string_as_cstr_cell!(AtomTable::build_with( + &self.machine_st.atom_tbl, + header_value.to_str().unwrap() + )))] ); self.machine_st.heap.extend(header_term.into_iter()); @@ -4458,10 +4445,7 @@ impl Machine { let reader = resp.bytes().unwrap().reader(); let mut stream = Stream::from_http_stream( - self.machine_st - .atom_tbl - .blocking_write() - .build_with(&address_string), + AtomTable::build_with(&self.machine_st.atom_tbl, &address_string), Box::new(reader), &mut self.machine_st.arena, ); @@ -4578,14 +4562,14 @@ impl Machine { Method::HEAD => atom!("head"), _ => unreachable!(), }; - let path_atom = self.machine_st.atom_tbl.blocking_write().build_with(request.request.uri().path()); + let path_atom = AtomTable::build_with(&self.machine_st.atom_tbl, request.request.uri().path()); let path_cell = atom_as_cstr_cell!(path_atom); let headers: Vec = request.request.headers().iter().map(|(header_name, header_value)| { let h = self.machine_st.heap.len(); let header_term = functor!( - self.machine_st.atom_tbl.blocking_write().build_with(header_name.as_str()), - [cell(string_as_cstr_cell!(self.machine_st.atom_tbl.blocking_write().build_with(header_value.to_str().unwrap())))] + AtomTable::build_with(&self.machine_st.atom_tbl, header_name.as_str()), + [cell(string_as_cstr_cell!(AtomTable::build_with(&self.machine_st.atom_tbl, header_value.to_str().unwrap())))] ); self.machine_st.heap.extend(header_term.into_iter()); @@ -4595,7 +4579,7 @@ impl Machine { let headers_list = iter_to_heap_list(&mut self.machine_st.heap, headers.into_iter()); let query_str = request.request.uri().query().unwrap_or(""); - let query_atom = self.machine_st.atom_tbl.blocking_write().build_with(query_str); + let query_atom = AtomTable::build_with(&self.machine_st.atom_tbl, query_str); let query_cell = string_as_cstr_cell!(query_atom); let hyper_req = request.request; @@ -4825,11 +4809,10 @@ impl Machine { unify!(self.machine_st, return_value, struct_value); } Value::CString(cstr) => { - let cstr = self - .machine_st - .atom_tbl - .blocking_write() - .build_with(cstr.to_str().unwrap()); + let cstr = AtomTable::build_with( + &self.machine_st.atom_tbl, + cstr.to_str().unwrap(), + ); self.machine_st.unify_complete_string(cstr, return_value); } } @@ -4858,11 +4841,10 @@ impl Machine { .map(|val| match val { Value::Int(n) => fixnum_as_cell!(Fixnum::build_with(n)), Value::Float(n) => HeapCellValue::from(float_alloc!(n, self.machine_st.arena)), - Value::CString(cstr) => atom_as_cell!(self - .machine_st - .atom_tbl - .blocking_write() - .build_with(&cstr.into_string().unwrap())), + Value::CString(cstr) => atom_as_cell!(AtomTable::build_with( + &self.machine_st.atom_tbl, + &cstr.into_string().unwrap() + )), Value::Struct(name, struct_args) => self.build_struct(&name, struct_args), }) .collect(); @@ -4918,7 +4900,7 @@ impl Machine { let src_sink = self.deref_register(1); if let Some(file_spec) = self.machine_st.value_to_str_like(src_sink) { - let file_spec = file_spec.as_atom(&mut self.machine_st.atom_tbl.blocking_write()); + let file_spec = file_spec.as_atom(&*self.machine_st.atom_tbl); let mut stream = self.machine_st @@ -4961,7 +4943,7 @@ impl Machine { let op = read_heap_cell!(self.deref_register(3), (HeapCellValueTag::Char, c) => { - self.machine_st.atom_tbl.blocking_write().build_with(&c.to_string()) + AtomTable::build_with(&self.machine_st.atom_tbl, &c.to_string()) } (HeapCellValueTag::Atom, (name, _arity)) => { name @@ -6128,11 +6110,7 @@ impl Machine { .read_term(&op_dir, Tokens::Default) .map_err(|err| error_after_read_term(err, 0, &parser)) .and_then(|term| { - write_term_to_heap( - &term, - &mut self.machine_st.heap, - &mut self.machine_st.atom_tbl.blocking_write(), - ) + write_term_to_heap(&term, &mut self.machine_st.heap, &self.machine_st.atom_tbl) }); match term_write_result { @@ -6296,7 +6274,7 @@ impl Machine { name } _ => { - self.machine_st.atom_tbl.blocking_write().build_with(&match Number::try_from(port) { + AtomTable::build_with(&self.machine_st.atom_tbl, &match Number::try_from(port) { Ok(Number::Fixnum(n)) => n.get_num().to_string(), Ok(Number::Integer(n)) => n.to_string(), _ => { @@ -6310,10 +6288,7 @@ impl Machine { atom!("127.0.0.1:80") } else { let buffer = format!("{}:{}", socket_atom.as_str(), port.as_str()); - self.machine_st - .atom_tbl - .blocking_write() - .build_with(&buffer) + AtomTable::build_with(&self.machine_st.atom_tbl, &buffer) }; let alias = self.machine_st.registers[4]; @@ -6495,7 +6470,7 @@ impl Machine { (ArenaHeaderTag::TcpListener, tcp_listener) => { match tcp_listener.accept().ok() { Some((tcp_stream, socket_addr)) => { - let client = self.machine_st.atom_tbl.blocking_write().build_with(&socket_addr.to_string()); + let client = AtomTable::build_with(&self.machine_st.atom_tbl, &socket_addr.to_string()); let mut tcp_stream = Stream::from_tcp_stream( client, @@ -7119,7 +7094,7 @@ impl Machine { let chars = put_complete_string( &mut self.machine_st.heap, &result, - &mut self.machine_st.atom_tbl.blocking_write(), + &self.machine_st.atom_tbl, ); let result_addr = self.deref_register(1); @@ -7138,7 +7113,7 @@ impl Machine { use git_version::git_version; let buffer = git_version!(cargo_prefix = "cargo:", fallback = "unknown"); - let buffer_atom = self.machine_st.atom_tbl.blocking_write().build_with(buffer); + let buffer_atom = AtomTable::build_with(&self.machine_st.atom_tbl, buffer); let a1 = self.deref_register(1); self.machine_st.unify_complete_string(buffer_atom, a1); @@ -7503,11 +7478,7 @@ impl Machine { if buffer.len() == 0 { empty_list_as_cell!() } else { - atom_as_cstr_cell!(self - .machine_st - .atom_tbl - .blocking_write() - .build_with(&buffer)) + atom_as_cstr_cell!(AtomTable::build_with(&self.machine_st.atom_tbl, &buffer)) } }; @@ -7644,11 +7615,8 @@ impl Machine { if let Some(string) = self.machine_st.value_to_str_like(addr) { for c in string.as_str().chars() { if c as u32 > 255 { - let non_octet = self - .machine_st - .atom_tbl - .blocking_write() - .build_with(&c.to_string()); + let non_octet = + AtomTable::build_with(&self.machine_st.atom_tbl, &c.to_string()); self.machine_st .unify_atom(non_octet, self.machine_st.registers[2]); return; @@ -7705,7 +7673,7 @@ impl Machine { let cstr = put_complete_string( &mut self.machine_st.heap, &value, - &mut self.machine_st.atom_tbl.blocking_write(), + &self.machine_st.atom_tbl, ); unify!(self.machine_st, self.machine_st.registers[2], cstr); @@ -7893,11 +7861,8 @@ impl Machine { path_buf.push(&*library_name.as_str()); let library_path_str = path_buf.to_str().unwrap(); - let library_path = self - .machine_st - .atom_tbl - .blocking_write() - .build_with(library_path_str); + let library_path = + AtomTable::build_with(&self.machine_st.atom_tbl, library_path_str); self.machine_st .unify_atom(library_path, self.machine_st.registers[3]); @@ -7994,11 +7959,8 @@ impl Machine { if path.is_dir() { if let Some(path) = path.to_str() { - let path_string = put_complete_string( - &mut self.machine_st.heap, - path, - &mut self.machine_st.atom_tbl.blocking_write(), - ); + let path_string = + put_complete_string(&mut self.machine_st.heap, path, &self.machine_st.atom_tbl); unify!(self.machine_st, self.machine_st.registers[1], path_string); return; @@ -8044,7 +8006,7 @@ impl Machine { fstr.push_str("finis]."); let s = datetime.format(&fstr).to_string(); - self.machine_st.atom_tbl.blocking_write().build_with(&s) + AtomTable::build_with(&self.machine_st.atom_tbl, &s) } pub(super) fn string_encoding_bytes( @@ -8068,21 +8030,17 @@ impl Machine { put_complete_string( &mut self.machine_st.heap, node.text().unwrap(), - &mut self.machine_st.atom_tbl.blocking_write(), + &self.machine_st.atom_tbl, ) } else { let mut avec = Vec::new(); for attr in node.attributes() { - let name = self - .machine_st - .atom_tbl - .blocking_write() - .build_with(attr.name()); + let name = AtomTable::build_with(&self.machine_st.atom_tbl, attr.name()); let value = put_complete_string( &mut self.machine_st.heap, &attr.value(), - &mut self.machine_st.atom_tbl.blocking_write(), + &self.machine_st.atom_tbl, ); avec.push(str_loc_as_cell!(self.machine_st.heap.len())); @@ -8108,11 +8066,7 @@ impl Machine { cvec.into_iter() )); - let tag = self - .machine_st - .atom_tbl - .blocking_write() - .build_with(node.tag_name().name()); + let tag = AtomTable::build_with(&self.machine_st.atom_tbl, node.tag_name().name()); let result = str_loc_as_cell!(self.machine_st.heap.len()); @@ -8132,17 +8086,17 @@ impl Machine { None => put_complete_string( &mut self.machine_st.heap, &node.text(), - &mut self.machine_st.atom_tbl.blocking_write(), + &self.machine_st.atom_tbl, ), Some(name) => { let mut avec = Vec::new(); for attr in node.attrs() { - let name = self.machine_st.atom_tbl.blocking_write().build_with(attr.0); + let name = AtomTable::build_with(&self.machine_st.atom_tbl, attr.0); let value = put_complete_string( &mut self.machine_st.heap, &attr.1, - &mut self.machine_st.atom_tbl.blocking_write(), + &self.machine_st.atom_tbl, ); avec.push(str_loc_as_cell!(self.machine_st.heap.len())); @@ -8168,7 +8122,7 @@ impl Machine { cvec.into_iter() )); - let tag = self.machine_st.atom_tbl.blocking_write().build_with(name); + let tag = AtomTable::build_with(&self.machine_st.atom_tbl, name); let result = str_loc_as_cell!(self.machine_st.heap.len()); self.machine_st @@ -8189,11 +8143,7 @@ impl Machine { if buffer.len() == 0 { empty_list_as_cell!() } else { - atom_as_cstr_cell!(self - .machine_st - .atom_tbl - .blocking_write() - .build_with(&buffer)) + atom_as_cstr_cell!(AtomTable::build_with(&self.machine_st.atom_tbl, &buffer)) } } } diff --git a/src/parser/ast.rs b/src/parser/ast.rs index 2458e81c..9df6f50c 100644 --- a/src/parser/ast.rs +++ b/src/parser/ast.rs @@ -10,6 +10,7 @@ use std::hash::{Hash, Hasher}; use std::io::{Error as IOError, ErrorKind}; use std::ops::{Deref, Neg}; use std::rc::Rc; +use std::sync::Arc; use std::vec::Vec; use crate::parser::dashu::{Integer, Rational}; @@ -18,6 +19,7 @@ use fxhash::FxBuildHasher; use indexmap::IndexMap; use modular_bitfield::error::OutOfBounds; use modular_bitfield::prelude::*; +use tokio::sync::RwLock; pub type Specifier = u32; @@ -631,7 +633,7 @@ impl fmt::Display for Literal { } impl Literal { - pub fn to_atom(&self, atom_tbl: &mut AtomTable) -> Option { + pub fn to_atom(&self, atom_tbl: &Arc>) -> Option { match self { Literal::Atom(atom) => Some(atom.defrock_brackets(atom_tbl)), _ => None, diff --git a/src/parser/lexer.rs b/src/parser/lexer.rs index dde0f03d..b73c8028 100644 --- a/src/parser/lexer.rs +++ b/src/parser/lexer.rs @@ -637,9 +637,10 @@ impl<'a, R: CharRead> Lexer<'a, R> { if token.as_str() == "[]" { Ok(Token::Literal(Literal::Atom(atom!("[]")))) } else { - Ok(Token::Literal(Literal::Atom( - self.machine_st.atom_tbl.blocking_write().build_with(&token), - ))) + Ok(Token::Literal(Literal::Atom(AtomTable::build_with( + &self.machine_st.atom_tbl, + &token, + )))) } } @@ -1083,7 +1084,7 @@ impl<'a, R: CharRead> Lexer<'a, R> { if c == '"' { let s = self.char_code_list_token(c)?; - let atom = self.machine_st.atom_tbl.blocking_write().build_with(&s); + let atom = AtomTable::build_with(&self.machine_st.atom_tbl, &s); return if let DoubleQuotes::Atom = self.machine_st.flags.double_quotes { Ok(Token::Literal(Literal::Atom(atom))) diff --git a/src/parser/parser.rs b/src/parser/parser.rs index d1ba7941..462bcf24 100644 --- a/src/parser/parser.rs +++ b/src/parser/parser.rs @@ -1,5 +1,6 @@ use dashu::Integer; use dashu::Rational; +use tokio::sync::RwLock; use crate::arena::*; use crate::atom_table::*; @@ -285,17 +286,17 @@ fn read_tokens(lexer: &mut Lexer) -> Result, ParserEr Ok(tokens) } -fn atomize_term(atom_tbl: &mut AtomTable, term: &Term) -> Option { +fn atomize_term(atom_tbl: &RwLock, term: &Term) -> Option { match term { Term::Literal(_, ref c) => atomize_constant(atom_tbl, *c), _ => None, } } -fn atomize_constant(atom_tbl: &mut AtomTable, c: Literal) -> Option { +fn atomize_constant(atom_tbl: &RwLock, c: Literal) -> Option { match c { Literal::Atom(ref name) => Some(*name), - Literal::Char(c) => Some(atom_tbl.build_with(&c.to_string())), + Literal::Char(c) => Some(AtomTable::build_with(atom_tbl, &c.to_string())), _ => None, } } @@ -568,19 +569,16 @@ impl<'a, R: CharRead> Parser<'a, R> { let idx = self.terms.len() - arity; if TokenType::Term == self.stack[stack_len].tt { - if atomize_term( - &mut self.lexer.machine_st.atom_tbl.blocking_write(), - &self.terms[idx - 1], - ) - .is_some() - { + if atomize_term(&self.lexer.machine_st.atom_tbl, &self.terms[idx - 1]).is_some() { self.stack.truncate(stack_len + 1); let mut subterms: Vec<_> = self.terms.drain(idx..).collect(); - if let Some(name) = self.terms.pop().and_then(|t| { - atomize_term(&mut self.lexer.machine_st.atom_tbl.blocking_write(), &t) - }) { + if let Some(name) = self + .terms + .pop() + .and_then(|t| atomize_term(&self.lexer.machine_st.atom_tbl, &t)) + { // reduce the '.' functor to a cons cell if it applies. if name == atom!(".") && subterms.len() == 2 { let tail = subterms.pop().unwrap(); @@ -591,12 +589,10 @@ impl<'a, R: CharRead> Parser<'a, R> { Term::PartialString(Cell::default(), string_buf, tail) } Ok((string_buf, None)) => { - let atom = self - .lexer - .machine_st - .atom_tbl - .blocking_write() - .build_with(&string_buf); + let atom = AtomTable::build_with( + &self.lexer.machine_st.atom_tbl, + &string_buf, + ); Term::CompleteString(Cell::default(), atom) } Err(term) => term, @@ -763,12 +759,7 @@ impl<'a, R: CharRead> Parser<'a, R> { Term::PartialString(Cell::default(), string_buf, tail) } Ok((string_buf, None)) => { - let atom = self - .lexer - .machine_st - .atom_tbl - .blocking_write() - .build_with(&string_buf); + let atom = AtomTable::build_with(&self.lexer.machine_st.atom_tbl, &string_buf); Term::CompleteString(Cell::default(), atom) } Err(term) => term, @@ -984,8 +975,7 @@ impl<'a, R: CharRead> Parser<'a, R> { |n, arena| Literal::from(float_alloc!(n, arena)), ), Token::Literal(c) => { - let atomized = - atomize_constant(&mut self.lexer.machine_st.atom_tbl.blocking_write(), c); + let atomized = atomize_constant(&self.lexer.machine_st.atom_tbl, c); if let Some(name) = atomized { if !self.shift_op(name, op_dir)? { self.shift(Token::Literal(c), 0, TERM); diff --git a/src/read.rs b/src/read.rs index b5ed91bc..17fa3257 100644 --- a/src/read.rs +++ b/src/read.rs @@ -81,7 +81,7 @@ impl MachineState { }; inner.add_lines_read(num_lines_read); - write_term_to_heap(&term, &mut self.heap, &mut self.atom_tbl.blocking_write()) + write_term_to_heap(&term, &mut self.heap, &self.atom_tbl) } } @@ -291,7 +291,7 @@ impl CharRead for ReadlineStream { pub(crate) fn write_term_to_heap<'a, 'b>( term: &'a Term, heap: &'b mut Heap, - atom_tbl: &mut AtomTable, + atom_tbl: &RwLock, ) -> Result { let term_writer = TermWriter::new(heap, atom_tbl); term_writer.write_term_to_heap(term) @@ -300,7 +300,7 @@ pub(crate) fn write_term_to_heap<'a, 'b>( #[derive(Debug)] struct TermWriter<'a, 'b> { heap: &'a mut Heap, - atom_tbl: &'b mut AtomTable, + atom_tbl: &'b RwLock, queue: SubtermDeque, var_dict: HeapVarDict, } @@ -313,7 +313,7 @@ pub struct TermWriteResult { impl<'a, 'b> TermWriter<'a, 'b> { #[inline] - fn new(heap: &'a mut Heap, atom_tbl: &'b mut AtomTable) -> Self { + fn new(heap: &'a mut Heap, atom_tbl: &'b RwLock) -> Self { TermWriter { heap, atom_tbl, @@ -435,7 +435,8 @@ impl<'a, 'b> TermWriter<'a, 'b> { continue; } &TermRef::CompleteString(_, _, ref src) => { - put_complete_string(self.heap, &src.as_str(), self.atom_tbl); + let src = src.as_str().to_owned(); + put_complete_string(self.heap, &src, self.atom_tbl); } &TermRef::PartialString(lvl, _, ref src, _) => { if let Level::Root = lvl { diff --git a/src/repl_helper.rs b/src/repl_helper.rs index a61d711a..5414651d 100644 --- a/src/repl_helper.rs +++ b/src/repl_helper.rs @@ -70,11 +70,13 @@ impl Completer for Helper { if let Some(idx) = start_of_prefix { let sub_str = line.get(idx..pos).unwrap(); - let guard = tokio::runtime::Handle::current() - .block_on(self.atoms.upgrade().unwrap().read_owned()); + let atom_table = self.atoms.upgrade().unwrap(); + println!("locking atom table to load completions"); + let guard = atom_table.blocking_read(); let mut matching = guard .table + .blocking_read() .iter() .chain(STATIC_ATOMS_MAP.values()) .map(|a| a.as_str()) diff --git a/tests/scryer/issues.rs b/tests/scryer/issues.rs index 1f0e2737..94aa37c7 100644 --- a/tests/scryer/issues.rs +++ b/tests/scryer/issues.rs @@ -1,5 +1,4 @@ use crate::helper::{load_module_test, run_top_level_test_no_args, run_top_level_test_with_args}; -use scryer_prolog::machine::Machine; use serial_test::serial; // issue #857 @@ -171,16 +170,3 @@ fn call_0() { " error(existence_error(procedure,call/0),call/0).\n", ); } - -// issue #1206 -#[serial] -#[test] -#[should_panic(expected = "Overwriting atom table base pointer")] -fn atomtable_is_not_concurrency_safe() { - // this is basically the same test as scryer_prolog::atom_table::atomtable_is_not_concurrency_safe - // but for this integration test scryer_prolog is compiled with cfg!(not(test)) while for the unit test it is compiled with cfg!(test) - // as the atom table implementation differ between cfg!(test) and cfg!(not(test)) both test serve a pourpose - // Note: this integration test itself is compiled with cfg!(test) independent of scryer_prolog itself - let _machine_a = Machine::with_test_streams(); - let _machine_b = Machine::with_test_streams(); -} -- 2.54.0