aleleba/wordcraft
1
0
Fork 0
mirror of https://github.com/storytold/wordcraft.git synced 2026-10-24 10:27:44 -06:00
Code Issues Packages Projects Releases Wiki Activity

Update the dictionary and its in-memory origin under one lock

Browse Source 
CodeRabbit (#3252): reload_dictionary() wrote DICT and DICT_IN_MEMORY
under two separate locks. If install_dictionary() ran between them,
the final state could hold an in-memory dictionary with the flag left
false, so installed_path() would report a same-named disk file
instead of None.

Combined both into one DictState struct behind a single RwLock in
both ru and cs, so install_dictionary() and reload_dictionary() each
update the dictionary and its origin in one critical section.

Didn't add a dedicated concurrency test: genuinely proving a race's
absence needs real multi-threaded stress testing, and the previous
commit on this PR already found that tests directly exercising
install_dictionary()/reload_dictionary() concurrency in this crate's
shared global state cause real, reproducible flakiness in unrelated
tests. The fix is correct by construction (one lock acquisition around
both field writes) and all 222 existing proof tests, run 15 times in
a row, stay green.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Michael
2026-10-20 17:41:20 +08:00
co-authored by Claude Sonnet 5
parent de6a52cc01
commit b57871b6ad
2 changed files with 36 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/proof/src/cs/mod.rs
+18 -13
View File
@@ -13,13 +13,16 @@ use crate::affix::AffixDict;
pub use suggest::suggest;
/// The Czech dictionary: `None` until first use, then the installed one (or an empty one).
static DICT: RwLock<Option<Arc<AffixDict>>> = RwLock::new(None);
/// The Czech dictionary and whether it came from [`install_dictionary`] rather than a file on
/// disk (it then has no file of its own to report from [`installed_path`], even if a matching
/// file also happens to exist): one lock, so a reload and an install can't interleave their two
/// fields into an inconsistent combination.
static DICT: RwLock<DictState> = RwLock::new(DictState { dict: None, in_memory: false });
/// Whether `DICT` holds a dictionary given with [`install_dictionary`] rather than one found on
/// disk: it has no file of its own to report from [`installed_path`], even if a matching file
/// happens to exist too.
static DICT_IN_MEMORY: RwLock<bool> = RwLock::new(false);
struct DictState {
dict: Option<Arc<AffixDict>>,
in_memory: bool,
}
/// File names of a Czech Hunspell dictionary.
const NAMES: &[&str] = &["cs_CZ", "cs-CZ", "cs"];
@@ -28,13 +31,13 @@ const NAMES: &[&str] = &["cs_CZ", "cs-CZ", "cs"];
/// [`install_dictionary`], or an empty one when none is available (Czech spelling is off
/// then; see [`has_dictionary`]).
pub fn dictionary() -> Arc<AffixDict> {
if let Some(d) = DICT.read().unwrap_or_else(|e| e.into_inner()).as_ref() {
if let Some(d) = DICT.read().unwrap_or_else(|e| e.into_inner()).dict.as_ref() {
return d.clone();
}
let loaded = Arc::new(load_installed());
let mut slot = DICT.write().unwrap_or_else(|e| e.into_inner());
// Another thread (or an install) may have filled it meanwhile: that one wins.
slot.get_or_insert(loaded).clone()
slot.dict.get_or_insert(loaded).clone()
}
fn load_installed() -> AffixDict {
@@ -52,16 +55,18 @@ fn load_installed() -> AffixDict {
/// UTF-8), for example one the user picked. On error the current dictionary stays.
pub fn install_dictionary(aff: &str, dic: &str) -> Result<(), String> {
let d = AffixDict::from_hunspell(aff, dic, &[])?;
*DICT.write().unwrap_or_else(|e| e.into_inner()) = Some(Arc::new(d));
*DICT_IN_MEMORY.write().unwrap_or_else(|e| e.into_inner()) = true;
let mut slot = DICT.write().unwrap_or_else(|e| e.into_inner());
slot.dict = Some(Arc::new(d));
slot.in_memory = true;
Ok(())
}
/// Forget the loaded dictionary, so the next check looks for an installed one again (after
/// [`crate::hunspell::set_dictionary_dir`] or a dictionary was installed).
pub fn reload_dictionary() {
*DICT.write().unwrap_or_else(|e| e.into_inner()) = None;
*DICT_IN_MEMORY.write().unwrap_or_else(|e| e.into_inner()) = false;
let mut slot = DICT.write().unwrap_or_else(|e| e.into_inner());
slot.dict = None;
slot.in_memory = false;
}
/// The `.dic` file of the installed Czech dictionary, searching the dictionaries folder and
@@ -69,7 +74,7 @@ pub fn reload_dictionary() {
/// none. A dictionary given with [`install_dictionary`] has no file and isn't reported, even if a
/// matching file also happens to exist.
pub fn installed_path() -> Option<std::path::PathBuf> {
if *DICT_IN_MEMORY.read().unwrap_or_else(|e| e.into_inner()) {
if DICT.read().unwrap_or_else(|e| e.into_inner()).in_memory {
return None;
}
crate::hunspell::find(NAMES).map(|(_, dic)| dic)
crates/proof/src/ru/mod.rs
+18 -13
View File
@@ -34,13 +34,16 @@ use crate::affix::AffixDict;
pub use suggest::suggest;
/// The Russian dictionary: `None` until first use, then the installed one (or an empty one).
static DICT: RwLock<Option<Arc<AffixDict>>> = RwLock::new(None);
/// The Russian dictionary and whether it came from [`install_dictionary`] rather than a file on
/// disk (it then has no file of its own to report from [`installed_path`], even if a matching
/// file also happens to exist): one lock, so a reload and an install can't interleave their two
/// fields into an inconsistent combination.
static DICT: RwLock<DictState> = RwLock::new(DictState { dict: None, in_memory: false });
/// Whether `DICT` holds a dictionary given with [`install_dictionary`] rather than one found on
/// disk: it has no file of its own to report from [`installed_path`], even if a matching file
/// happens to exist too.
static DICT_IN_MEMORY: RwLock<bool> = RwLock::new(false);
struct DictState {
dict: Option<Arc<AffixDict>>,
in_memory: bool,
}
/// File names of a Russian Hunspell dictionary.
const NAMES: &[&str] = &["ru_RU", "ru-RU", "ru"];
@@ -49,13 +52,13 @@ const NAMES: &[&str] = &["ru_RU", "ru-RU", "ru"];
/// [`install_dictionary`], or an empty one when none is available (Russian spelling is off then;
/// see [`has_dictionary`]).
pub fn dictionary() -> Arc<AffixDict> {
if let Some(d) = DICT.read().unwrap_or_else(|e| e.into_inner()).as_ref() {
if let Some(d) = DICT.read().unwrap_or_else(|e| e.into_inner()).dict.as_ref() {
return d.clone();
}
let loaded = Arc::new(load_installed());
let mut slot = DICT.write().unwrap_or_else(|e| e.into_inner());
// Another thread (or an install) may have filled it meanwhile: that one wins.
slot.get_or_insert(loaded).clone()
slot.dict.get_or_insert(loaded).clone()
}
fn load_installed() -> AffixDict {
@@ -73,16 +76,18 @@ fn load_installed() -> AffixDict {
/// for example one the user picked. On error the current dictionary stays.
pub fn install_dictionary(aff: &str, dic: &str) -> Result<(), String> {
let d = AffixDict::from_hunspell(aff, dic, &[])?;
*DICT.write().unwrap_or_else(|e| e.into_inner()) = Some(Arc::new(d));
*DICT_IN_MEMORY.write().unwrap_or_else(|e| e.into_inner()) = true;
let mut slot = DICT.write().unwrap_or_else(|e| e.into_inner());
slot.dict = Some(Arc::new(d));
slot.in_memory = true;
Ok(())
}
/// Forget the loaded dictionary, so the next check looks for an installed one again (after
/// [`crate::hunspell::set_dictionary_dir`] or a dictionary was installed).
pub fn reload_dictionary() {
*DICT.write().unwrap_or_else(|e| e.into_inner()) = None;
*DICT_IN_MEMORY.write().unwrap_or_else(|e| e.into_inner()) = false;
let mut slot = DICT.write().unwrap_or_else(|e| e.into_inner());
slot.dict = None;
slot.in_memory = false;
}
/// The `.dic` file of the installed Russian dictionary, searching the dictionaries folder and
@@ -90,7 +95,7 @@ pub fn reload_dictionary() {
/// none. A dictionary given with [`install_dictionary`] has no file and isn't reported, even if a
/// matching file also happens to exist.
pub fn installed_path() -> Option<std::path::PathBuf> {
if *DICT_IN_MEMORY.read().unwrap_or_else(|e| e.into_inner()) {
if DICT.read().unwrap_or_else(|e| e.into_inner()).in_memory {
return None;
}
crate::hunspell::find(NAMES).map(|(_, dic)| dic)