Skip to content

Commit 2e0c57a

Browse files
committed
Guard DuckDB definition lock cleanup
1 parent 0990208 commit 2e0c57a

1 file changed

Lines changed: 55 additions & 3 deletions

File tree

sidemantic-rs/src/ffi.rs

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,10 @@ use std::io::{self, Write};
1313
use std::os::raw::c_char;
1414
use std::path::{Path, PathBuf};
1515
use std::ptr;
16-
use std::sync::Mutex;
16+
use std::sync::{
17+
atomic::{AtomicU64, Ordering},
18+
Mutex,
19+
};
1720
use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
1821

1922
use once_cell::sync::Lazy;
@@ -38,6 +41,7 @@ struct FfiState {
3841
/// Semantic graph state keyed by DuckDB database/session context.
3942
static FFI_STATES: Lazy<Mutex<HashMap<String, FfiState>>> =
4043
Lazy::new(|| Mutex::new(HashMap::new()));
44+
static DEFINITIONS_LOCK_TOKEN_COUNTER: AtomicU64 = AtomicU64::new(1);
4145

4246
/// Result from rewrite operation
4347
#[repr(C)]
@@ -692,12 +696,15 @@ fn validate_definitions_content(content: &str) -> Result<(), String> {
692696
struct DefinitionsFileLock {
693697
path: PathBuf,
694698
file: Option<fs::File>,
699+
owner_token: String,
695700
}
696701

697702
impl Drop for DefinitionsFileLock {
698703
fn drop(&mut self) {
699704
self.file.take();
700-
let _ = fs::remove_file(&self.path);
705+
if definitions_lock_owned_by(&self.path, &self.owner_token) {
706+
let _ = fs::remove_file(&self.path);
707+
}
701708
}
702709
}
703710

@@ -710,6 +717,22 @@ fn definitions_lock_path(path: &Path) -> PathBuf {
710717
parent.join(format!(".{file_name}.lock"))
711718
}
712719

720+
fn definitions_lock_owner_token() -> String {
721+
let unique = SystemTime::now()
722+
.duration_since(UNIX_EPOCH)
723+
.map(|duration| duration.as_nanos())
724+
.unwrap_or_default();
725+
let sequence = DEFINITIONS_LOCK_TOKEN_COUNTER.fetch_add(1, Ordering::Relaxed);
726+
format!("pid={};time={unique};seq={sequence}", std::process::id())
727+
}
728+
729+
fn definitions_lock_owned_by(lock_path: &Path, owner_token: &str) -> bool {
730+
fs::read_to_string(lock_path)
731+
.ok()
732+
.and_then(|content| content.lines().next().map(str::to_owned))
733+
.is_some_and(|token| token == owner_token)
734+
}
735+
713736
fn lock_definitions_file(path: &Path) -> io::Result<DefinitionsFileLock> {
714737
let lock_path = definitions_lock_path(path);
715738
let deadline = Instant::now() + DEFINITIONS_LOCK_TIMEOUT;
@@ -721,11 +744,13 @@ fn lock_definitions_file(path: &Path) -> io::Result<DefinitionsFileLock> {
721744
.open(&lock_path)
722745
{
723746
Ok(mut file) => {
724-
writeln!(file, "pid={}", std::process::id())?;
747+
let owner_token = definitions_lock_owner_token();
748+
writeln!(file, "{owner_token}")?;
725749
file.sync_all()?;
726750
return Ok(DefinitionsFileLock {
727751
path: lock_path,
728752
file: Some(file),
753+
owner_token,
729754
});
730755
}
731756
Err(error) if error.kind() == io::ErrorKind::AlreadyExists => {
@@ -2318,6 +2343,33 @@ models:
23182343
let _ = fs::remove_file(definitions_path);
23192344
}
23202345

2346+
#[test]
2347+
fn test_lock_drop_does_not_remove_recreated_lock() {
2348+
let _guard = test_lock();
2349+
2350+
let db_path = unique_db_path("lock_recreated");
2351+
let db_path = CString::new(db_path.to_string_lossy().to_string()).unwrap();
2352+
let definitions_path = get_definitions_path(db_path.as_ptr()).unwrap();
2353+
let lock_path = definitions_lock_path(&definitions_path);
2354+
2355+
let original_lock = lock_definitions_file(&definitions_path).unwrap();
2356+
assert!(lock_path.exists());
2357+
2358+
fs::remove_file(&lock_path).unwrap();
2359+
let recreated_token = "pid=recreated;time=1;seq=1";
2360+
fs::write(&lock_path, format!("{recreated_token}\n")).unwrap();
2361+
2362+
drop(original_lock);
2363+
2364+
assert_eq!(
2365+
fs::read_to_string(&lock_path).unwrap(),
2366+
format!("{recreated_token}\n")
2367+
);
2368+
2369+
let _ = fs::remove_file(lock_path);
2370+
let _ = fs::remove_file(definitions_path);
2371+
}
2372+
23212373
#[test]
23222374
fn test_remove_model_from_file_handles_multiline_models() {
23232375
let _guard = test_lock();

0 commit comments

Comments
 (0)