Let sessions outlive the backend, and never resume one twice
Three `claude` processes ended up running against this checkout on 2026-08-29, and the account hit its session limit. One cause, several ways in. An agent imported the Claude Code session it was *itself* running in. That is an ordinary import, and importing runs `--resume` -- so a second CLI attached to a file the first was still writing. The whole 65 MB conversation, 154 embedded screenshots included, was re-appended to the transcript under a new prompt id; both copies then read each other's writes as work done elsewhere, and the adopted one was billed for re-reading all of it. Meanwhile `shutdown_all` asked each session to stop and the process exited immediately, so the SIGKILL timer died with the runtime, the stop was unreliable, and whatever survived was orphaned with nothing written down to find it by. The processes leaked either way. So leak them on purpose, and be able to pick them back up. A session's process now outlives the backend and is adopted again on the way up, which is worth having for its own sake: restarting the server no longer ends a turn somebody is waiting on. Its stdio lives in the session directory -- a fifo opened read-write so the process is its own last writer and never reads EOF, plus stdout/stderr logs read from a byte offset. `session::process` records the pid *and* the kernel's start time for it, because a pid alone is reused and adopting a stranger's would mean never resuming the real conversation. That makes the fix structural rather than a check: everything goes through `ClaudeDriver::launch`, which adopts if it can and starts if it cannot, and `--resume` is reachable only on the second path. `Driver` gains two ways out where it had one -- `detach` (coming back) and `stop` (the session is being deleted, so the process must not survive). Importing a session that is open is now refused outright. Claude Code keeps `~/.claude/sessions/<pid>.json` for every live session, so this is a measurement rather than a guess; it reports no/yes/unknown, because a machine that keeps no such record cannot answer and "could not check" is not "nobody is using it". `SessionStatus` gains `Unknown` for the same reason. Also here, found on the way: - A reconnecting phone was sent the entire backlog. Opening a session was bounded to a page but reconnecting was not, so a long disconnect delivered thousands of events one frame at a time. Past `CATCH_UP_LIMIT` the stream sends a `reset` frame and the newest window, and the client rebuilds from it as it does on open -- without the reset the window is spliced onto rows no longer adjacent to it. - A session's status was assumed idle at launch. Read from the transcript instead, so a restart stops claiming an exited session is waiting for you. - `llama-server`'s stdout was piped and never drained, so a chatty one blocked on a full pipe buffer mid-load. It goes to a log now. - A turn that exited or errored never emitted `Idle`, so the queue stayed "running" for good: every later message was held forever and, since a message is only recorded when taken, vanished with nothing on screen. - Two doc comments had drifted onto the wrong functions. Verified by killing the server mid-turn: the process survived, finished its turn unattended (12.8 KB of output nothing was reading), and the restarted server adopted it -- one process, all 700 lines in the transcript, no hole, and it still took a new message afterwards. Deleting a session stops its process; a 266-event backlog resets while a 16-event one streams. 46 tests, clippy and rustfmt clean, app compiles and lints. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VETa8afmpWaYezLCqJhDB8
This commit is contained in:
1 parent
9791afcfd6
commit
362d436d4f
23 files changed
+1934
-345
No files matched your search
+188
-40
@@ -26,15 +26,16 @@
|
||||
//! inconsistency should resolve it in this direction.
|
||||
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::Arc;
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
use std::sync::{Arc, Mutex};
|
||||
|
||||
use anyhow::{Context, Result, bail};
|
||||
use serde::{Deserialize, Serialize};
|
||||
use serde_json::json;
|
||||
|
||||
use super::driver::{Driver, Event, EventSink, ImageRef, SessionStatus};
|
||||
use super::transport::{Launch, Transport};
|
||||
use super::process;
|
||||
use super::transport::{Launch, Streams, Transport};
|
||||
use crate::config::{ProviderConfig, SessionConfig};
|
||||
|
||||
/// How long to wait for a model to load before giving up on it. Loading
|
||||
@@ -61,17 +62,27 @@ pub struct LlamaDriver {
|
||||
/// Set by [`Driver::interrupt`]; the streaming loop checks it between
|
||||
/// chunks and stops, leaving what was generated in the transcript.
|
||||
cancel: Arc<AtomicBool>,
|
||||
/// Taken by shutdown to stop the server.
|
||||
kill: Mutex<Option<tokio::sync::oneshot::Sender<()>>>,
|
||||
/// Where this session's process record lives, so [`Driver::stop`] can
|
||||
/// find the server it has to end.
|
||||
session_dir: PathBuf,
|
||||
}
|
||||
|
||||
impl LlamaDriver {
|
||||
pub fn spawn(
|
||||
/// Takes charge of this session's `llama-server`: the one already
|
||||
/// loaded if there is one, otherwise a new one.
|
||||
///
|
||||
/// One entry point, for the reason `ClaudeDriver::launch` gives -- the
|
||||
/// choice is not the caller's and a second process is the expensive
|
||||
/// mistake. Here it is expensive in a different currency: two servers
|
||||
/// holding the same model is twice the memory, and the second would
|
||||
/// bind a different port while the phone kept talking to the first.
|
||||
pub fn launch(
|
||||
meta: &SessionConfig,
|
||||
provider: &ProviderConfig,
|
||||
transport: &Transport,
|
||||
models_dir: &Path,
|
||||
transcript: &Path,
|
||||
session_dir: &Path,
|
||||
sink: EventSink,
|
||||
) -> Result<Self> {
|
||||
if !matches!(transport, Transport::Here) {
|
||||
@@ -85,6 +96,30 @@ impl LlamaDriver {
|
||||
)?;
|
||||
let path = model_path(models_dir, model)?;
|
||||
|
||||
// Already loaded and still running: keep talking to it. The
|
||||
// health poll below is what confirms it is really answering, so
|
||||
// adopting a pid whose server has wedged still reports as a
|
||||
// failure rather than as a session that silently never replies.
|
||||
if let Some(process::Record {
|
||||
detail: process::Detail::Http { port },
|
||||
pid,
|
||||
..
|
||||
}) = process::live(session_dir)
|
||||
{
|
||||
tracing::info!(
|
||||
"session {} reattaching to the llama-server it left loaded (pid {pid}, port {port})",
|
||||
meta.id
|
||||
);
|
||||
return Ok(Self::attached(
|
||||
format!("http://127.0.0.1:{port}"),
|
||||
meta,
|
||||
model,
|
||||
transcript,
|
||||
session_dir,
|
||||
sink,
|
||||
));
|
||||
}
|
||||
|
||||
let port = free_port().context("finding a port for llama-server")?;
|
||||
let mut args: Vec<String> = vec![
|
||||
"-m".into(),
|
||||
@@ -110,38 +145,62 @@ impl LlamaDriver {
|
||||
|
||||
let program = provider.command.as_deref().unwrap_or("llama-server");
|
||||
let launch = Launch::new(program, args, meta.cwd.as_deref());
|
||||
let mut child = transport.spawn(&launch)?;
|
||||
// Its output goes to files, not pipes. Not only so the process can
|
||||
// outlive this server: nothing ever read those pipes, so a chatty
|
||||
// llama-server filled the 64 KB buffer and blocked mid-load with
|
||||
// no sign of why.
|
||||
let child = transport.spawn(
|
||||
&launch,
|
||||
Streams::Detached {
|
||||
stdin: std::process::Stdio::null(),
|
||||
stdout: log_file(&session_dir.join(SERVER_LOG))?.into(),
|
||||
stderr: log_file(&session_dir.join(SERVER_LOG))?.into(),
|
||||
},
|
||||
)?;
|
||||
let pid = child
|
||||
.id()
|
||||
.context("llama-server exited before it could be recorded")?;
|
||||
tracing::info!(
|
||||
"session {} running {program} for {model} on 127.0.0.1:{port}",
|
||||
"session {} running {program} for {model} on 127.0.0.1:{port} as pid {pid}",
|
||||
meta.id
|
||||
);
|
||||
// Reaped so it does not become a zombie while this server is still
|
||||
// its parent; the health poll and the record are what actually say
|
||||
// whether the session is alive, because after a restart there is no
|
||||
// `Child` here to ask.
|
||||
tokio::spawn(async move {
|
||||
let mut child = child;
|
||||
let _ = child.wait().await;
|
||||
});
|
||||
|
||||
let endpoint = format!("http://127.0.0.1:{port}");
|
||||
let (kill_tx, kill_rx) = tokio::sync::oneshot::channel::<()>();
|
||||
{
|
||||
let sink = sink.clone();
|
||||
let label = format!("{} ({model})", provider.name);
|
||||
tokio::spawn(async move {
|
||||
let status = tokio::select! {
|
||||
status = child.wait() => status.ok(),
|
||||
_ = kill_rx => {
|
||||
let _ = child.kill().await;
|
||||
None
|
||||
}
|
||||
};
|
||||
if let Some(status) = status
|
||||
&& !status.success()
|
||||
{
|
||||
let _ = sink.send(Event::Error {
|
||||
message: format!("{label} exited: {status}"),
|
||||
});
|
||||
}
|
||||
let _ = sink.send(Event::Status {
|
||||
state: SessionStatus::Exited,
|
||||
});
|
||||
});
|
||||
}
|
||||
let record = process::Record::of(pid, process::Detail::Http { port })
|
||||
.context("llama-server was gone before its start time could be read")?;
|
||||
process::write(session_dir, &record);
|
||||
|
||||
Ok(Self::attached(
|
||||
format!("http://127.0.0.1:{port}"),
|
||||
meta,
|
||||
model,
|
||||
transcript,
|
||||
session_dir,
|
||||
sink,
|
||||
))
|
||||
}
|
||||
|
||||
/// The driver for a `llama-server` at `endpoint`, however it got there.
|
||||
///
|
||||
/// Shared by starting one and adopting one, because everything after
|
||||
/// "there is a server at this address" is identical -- including
|
||||
/// waiting for it to answer, which an adopted one still owes: a
|
||||
/// recorded pid says a process exists, not that its model is loaded.
|
||||
fn attached(
|
||||
endpoint: String,
|
||||
meta: &SessionConfig,
|
||||
model: &str,
|
||||
transcript: &Path,
|
||||
session_dir: &Path,
|
||||
sink: EventSink,
|
||||
) -> Self {
|
||||
// Loading is slow enough to be worth saying so: the session shows
|
||||
// as running until the model is in memory, then goes idle, rather
|
||||
// than looking ready and refusing the first message.
|
||||
@@ -152,12 +211,14 @@ impl LlamaDriver {
|
||||
let sink = sink.clone();
|
||||
let endpoint = endpoint.clone();
|
||||
let model = model.to_string();
|
||||
let session_dir = session_dir.to_path_buf();
|
||||
std::thread::spawn(move || match wait_until_ready(&endpoint) {
|
||||
Ok(()) => {
|
||||
tracing::info!("{model} loaded and answering at {endpoint}");
|
||||
let _ = sink.send(Event::Status {
|
||||
state: SessionStatus::Idle,
|
||||
});
|
||||
watch(session_dir, sink);
|
||||
}
|
||||
Err(err) => {
|
||||
let _ = sink.send(Event::Error {
|
||||
@@ -166,6 +227,7 @@ impl LlamaDriver {
|
||||
let _ = sink.send(Event::Status {
|
||||
state: SessionStatus::Exited,
|
||||
});
|
||||
process::clear(&session_dir);
|
||||
}
|
||||
});
|
||||
}
|
||||
@@ -184,17 +246,84 @@ impl LlamaDriver {
|
||||
}
|
||||
}
|
||||
|
||||
Ok(Self {
|
||||
Self {
|
||||
sink,
|
||||
endpoint,
|
||||
transcript: transcript.to_path_buf(),
|
||||
sampling,
|
||||
cancel: Arc::new(AtomicBool::new(false)),
|
||||
kill: Mutex::new(Some(kill_tx)),
|
||||
})
|
||||
session_dir: session_dir.to_path_buf(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Where llama-server's own output goes. One file for both streams: it is
|
||||
/// diagnostics nobody parses, and interleaving them is how it reads in a
|
||||
/// terminal anyway.
|
||||
const SERVER_LOG: &str = "llama-server.log";
|
||||
|
||||
/// How often a loaded server is checked for still being there.
|
||||
///
|
||||
/// Slower than the Claude driver's stdout poll because nothing is waiting
|
||||
/// on it: this only has to notice a server that has gone, and a few
|
||||
/// seconds late costs nothing.
|
||||
const WATCH_INTERVAL: std::time::Duration = std::time::Duration::from_secs(2);
|
||||
|
||||
/// Grace period between asking llama-server to stop and killing it.
|
||||
const STOP_GRACE: std::time::Duration = std::time::Duration::from_secs(5);
|
||||
|
||||
/// An owner-only log opened for appending, so the two streams pointed at
|
||||
/// it do not overwrite each other and a reattach keeps what came before.
|
||||
fn log_file(path: &Path) -> Result<std::fs::File> {
|
||||
use std::os::unix::fs::OpenOptionsExt;
|
||||
std::fs::OpenOptions::new()
|
||||
.create(true)
|
||||
.append(true)
|
||||
.mode(0o600)
|
||||
.open(path)
|
||||
.with_context(|| format!("opening {}", path.display()))
|
||||
}
|
||||
|
||||
/// Reports the server going away, for as long as the session is there to
|
||||
/// report it to.
|
||||
///
|
||||
/// Polled rather than waited on, for the reason the Claude driver gives:
|
||||
/// after a restart this server is not the process's parent and has nothing
|
||||
/// to wait on, so liveness has to be a question asked of the record -- and
|
||||
/// asking it two different ways is how the two answers come to disagree.
|
||||
fn watch(session_dir: PathBuf, sink: EventSink) {
|
||||
std::thread::spawn(move || {
|
||||
loop {
|
||||
std::thread::sleep(WATCH_INTERVAL);
|
||||
match process::recorded(&session_dir) {
|
||||
Some((_, process::Liveness::Alive)) => {}
|
||||
// Nothing recorded means the session was stopped or
|
||||
// deleted deliberately, and whoever did that has already
|
||||
// said so.
|
||||
None => return,
|
||||
Some((_, process::Liveness::Dead)) => {
|
||||
let _ = sink.send(Event::Error {
|
||||
message: "llama-server exited".to_string(),
|
||||
});
|
||||
let _ = sink.send(Event::Status {
|
||||
state: SessionStatus::Exited,
|
||||
});
|
||||
process::clear(&session_dir);
|
||||
return;
|
||||
}
|
||||
Some((_, process::Liveness::Unknown)) => {
|
||||
let _ = sink.send(Event::Status {
|
||||
state: SessionStatus::Unknown,
|
||||
});
|
||||
}
|
||||
}
|
||||
if sink.is_closed() {
|
||||
return;
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
impl Driver for LlamaDriver {
|
||||
fn send_user_message(&self, text: String, images: Vec<ImageRef>) {
|
||||
if !images.is_empty() {
|
||||
@@ -212,13 +341,17 @@ impl Driver for LlamaDriver {
|
||||
// Its own thread: the request blocks for as long as the model
|
||||
// takes to generate, which is the whole point of streaming it.
|
||||
std::thread::spawn(move || {
|
||||
// Nothing is ever held back here -- there is no queue to wait
|
||||
// in -- so the message is taken the moment it arrives. Said
|
||||
// anyway, because this is what records it: see `MessageTaken`.
|
||||
let _ = sink.send(Event::MessageTaken { text: text.clone() });
|
||||
let _ = sink.send(Event::Status {
|
||||
state: SessionStatus::Running,
|
||||
});
|
||||
// Everything before this message, plus this message. Read
|
||||
// rather than remembered, and `text` is appended here rather
|
||||
// than waited for, because the manager's UserMessage event is
|
||||
// still on its way to the transcript when this runs.
|
||||
// than waited for, because the message's own transcript entry
|
||||
// is still on its way when this runs.
|
||||
let mut messages = conversation(&transcript);
|
||||
messages.push(Message {
|
||||
role: "user".into(),
|
||||
@@ -270,11 +403,26 @@ impl Driver for LlamaDriver {
|
||||
});
|
||||
}
|
||||
|
||||
fn shutdown(&self) {
|
||||
/// Stops generating and leaves the server loaded.
|
||||
///
|
||||
/// Worth being deliberate about, because the cost is asymmetric and
|
||||
/// points the other way from the Claude driver's: a `llama-server`
|
||||
/// holds its whole model in memory, so a leaked one is gigabytes
|
||||
/// nobody is using. It is left anyway, because the alternative is
|
||||
/// unloading and reloading that model on every backend restart --
|
||||
/// minutes of disk, for a session somebody is in the middle of. The
|
||||
/// record is what keeps it from being *nobody's*: the next run of this
|
||||
/// server adopts it rather than starting a second one.
|
||||
fn detach(&self) {
|
||||
self.cancel.store(true, Ordering::Relaxed);
|
||||
if let Some(kill) = self.kill.lock().unwrap().take() {
|
||||
let _ = kill.send(());
|
||||
}
|
||||
|
||||
fn stop(&self) {
|
||||
self.cancel.store(true, Ordering::Relaxed);
|
||||
if let Some(record) = process::live(&self.session_dir) {
|
||||
process::stop(&record, STOP_GRACE);
|
||||
}
|
||||
process::clear(&self.session_dir);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user