Conversation
AndriiS-DevBrother
left a comment
AndriiS-DevBrother
left a comment
There was a problem hiding this comment.
lgtm
just 1 comment with potential issue
| if self.env.is_none() { | ||
| self.env = Some(self.create_env()?); | ||
| } | ||
| let env = self.env.as_ref().unwrap(); | ||
|
|
||
| // Check if database already exists | ||
| { | ||
| let dbs = self.dbs.lock().await; | ||
| if let Some(db) = dbs.get(&name) { | ||
| return Ok(Arc::new(LmdbKeyValueStore::new(env.clone(), db.clone()))); | ||
| } | ||
| } | ||
|
|
||
| // Create the database (heed v0.22 requires a write transaction) | ||
| let mut wtxn = env.write_txn()?; | ||
| let db = env.create_database(&mut wtxn, Some(&name))?; | ||
| wtxn.commit()?; | ||
|
|
||
| // Store database reference | ||
| { | ||
| let mut dbs = self.dbs.lock().await; | ||
| dbs.insert(name, db.clone()); | ||
| } |
There was a problem hiding this comment.
I don't Rust so deep, but AI comment it as potential issue:
LMDB env race and handle mix: concurrent
storecalls can each execute theis_nonebranch, create separateEnvs, and then overwriteself.envwhile prior DB handles remain tied to the first env. Subsequent operations may mix DB handles with a different env, which LMDB forbids and can manifest as corruption or panics. Guard env creation with a dedicated lock/OnceCell, or create env eagerly during construction.
Suggested fix: make env creation single-flight (e.g.,
OnceCell<Env<WithoutTls>>withget_or_try_initor an async mutex around creation) and optionally initialize eagerly innew. Ensure DB creation also uses that single env (consider a short mutex aroundcreate_database).
2 participants
Fix LMDB Environment Management and Test Infrastructure
Problem
Casper tests failing with infrastructure errors:
EnvAlreadyOpened- LMDB environments opened multiple timesRoot Causes
rspace-historyandrspace-rootsshare same LMDB path but code opened separate environmentsChanges
LMDB Fixes
rspace_store_manager.rs: Reuse single LMDB env for databases sharing same pathlmdb_store_manager.rs: Simplified env lifecycle managementlmdb_dir_store_manager.rs: Store and reuse managers by env name (matching Scala)heed0.11 → 0.22, addedread_txn_without_tls()forMDB_NOTLSflagTest Fixes
OnceCellgenesis caching aligned with Scala's per-class patternapprove_block_protocol_test.rs: Added#[serial]to prevent metrics race conditionmulti_parent_casper_merge_spec.rs: Fixed async runtime nesting panic