-
Notifications
You must be signed in to change notification settings - Fork 6
Description
Summary
In src/cortex-app-server/src/storage.rs, the save_session() method acquires an exclusive file lock, wraps the file in a BufWriter, serializes JSON into it, then calls file.unlock() before the BufWriter is dropped. Because BufWriter only flushes its internal buffer when it is dropped (or explicitly flushed), the actual bytes may not have been written to the file at the point the lock is released. Another process/thread that acquires the lock immediately after can therefore read a stale or empty file.
Affected Code
// src/cortex-app-server/src/storage.rs lines 88-103
pub fn save_session(&self, session: &StoredSession) -> std::io::Result<()> {
let path = self.session_path(&session.id);
let file = fs::File::create(&path)?;
// Acquire exclusive lock for writing
file.lock_exclusive()?;
let writer = BufWriter::new(&file);
let result = serde_json::to_writer_pretty(writer, session).map_err(std::io::Error::other);
// Lock is automatically released when file is dropped
file.unlock()?; // ← BufWriter not yet dropped/flushed here!
result?;
Ok(())
}Root Cause
BufWriter holds an internal buffer. serde_json::to_writer_pretty writes into that buffer. The buffer is only flushed to the underlying file when BufWriter is dropped or flush() is explicitly called. Here, file.unlock() is called while writer (the BufWriter) is still live on the stack — its destructor runs after unlock() returns. This means:
- The lock is released with potentially zero bytes written.
- A concurrent reader that acquires the shared lock immediately after will read an incomplete or empty file.
- The
result?check happens after unlock, so even an error path does not protect against this.
Expected Behaviour
The BufWriter should be explicitly flushed (or dropped) before file.unlock() is called, ensuring all buffered bytes reach the file descriptor before the lock is released.
Suggested Fix
pub fn save_session(&self, session: &StoredSession) -> std::io::Result<()> {
let path = self.session_path(&session.id);
let file = fs::File::create(&path)?;
file.lock_exclusive()?;
let mut writer = BufWriter::new(&file);
serde_json::to_writer_pretty(&mut writer, session).map_err(std::io::Error::other)?;
writer.flush()?; // ← flush before unlocking
drop(writer);
file.unlock()?;
Ok(())
}Impact
- Data corruption / data loss: session metadata written by one server instance can be silently lost or partially written, causing
load_sessionto fail with anInvalidDataerror on the next read. - Multi-instance deployments (the exact scenario the file-locking was designed to protect) are most affected.