Skip to content

[BUG] [v0.0.7] save_session() in storage.rs unlocks file before BufWriter is flushed, risking partial writes #157

@katotomoya0105

Description

@katotomoya0105

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:

  1. The lock is released with potentially zero bytes written.
  2. A concurrent reader that acquires the shared lock immediately after will read an incomplete or empty file.
  3. 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_session to fail with an InvalidData error on the next read.
  • Multi-instance deployments (the exact scenario the file-locking was designed to protect) are most affected.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions