mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-04 11:01:16 +01:00
fix(runner): set Runner OOM protection at launch (#18730)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The native Runner supervises the harness and tool processes for each run > - A launcher can start Runner with a negative `oom_score_adj`, and Linux children inherit that value > - Then a large tool process has the same protection as Runner > - If that protection is -1000, the kernel cannot kill the process, and a VM with no other candidate panics > - The correct protection for Runner is different in each environment, so one static value is not correct > - This pull request adds an optional `--oom-score-adj` launch flag. With the flag, Runner also sets supervised processes to 0 when Runner has a negative score > - The benefit is that the launcher controls Runner protection, and the kernel can kill the large process and let Runner report the failure ## Linked Issues or Issue Description Related: #14509 and #16844. These pull requests change scores for the server and for sandbox tool commands. This pull request changes the native Runner. **What happened?** A native Runner on a VM inherited `oom_score_adj` -1000 from the VM init process. A typecheck under Runner used most of the VM memory. The kernel could not select any process and panicked with "System is deadlocked on memory". The VM restarted, and the run failed with a transport error. **Expected behavior** The kernel can kill the large tool process. Runner continues and reports the result. **Steps to reproduce** Start Runner with a negative score. Start a supervised process. Read `/proc/<pid>/oom_score_adj` for the supervised process. The value is the same as the Runner value. After this change, the value is 0 when the launcher gives the flag. **Deployment mode** Native Runner on Linux, when the launcher gives Runner a negative score. ## What Changed - Add the optional `--oom-score-adj <-1000..=1000>` flag to `paperclip-runnerd`. Runner has no default value. - Stop Runner startup when the value is not valid. When the kernel refuses a valid value, or the platform is not Linux, write a diagnostic and continue, because Runner is then only less protected than the launcher asked for. - When the launcher gives the flag and the Runner score is negative, set each supervised process and verified stdio MCP server to 0 after spawn. - Without the flag, Runner does not change any score. Children inherit the Runner score, as they do now. - Add unit and integration tests. ## Verification Tests ran in `rust:1-bookworm` (rustc 1.99.1) on Linux arm64 with `CAP_SYS_RESOURCE`: - `cargo fmt --check -p paperclip-runner-core` - `cargo test -p paperclip-runner-core --no-fail-fast`, before the flag gate commit: all targets pass except `codex_provider` and `stdio_mcp`. - `provider_receives_the_isolated_codex_auth_home` and its subprocess helper fail in this container. They fail in the same way on clean `master`. - `receipt_limit_polls_an_authoritative_terminal_with_unacknowledged_events` and `cancellation_waiting_behind_output_is_handled_before_new_dispatch` failed once in the full run. They pass when they run alone, on this branch and on clean `master`. I think that they are sensitive to load, but I did not prove this. CI is the check for these tests. - `--test oom_score` and `--test oom_score_without_flag` pass with and without `CAP_SYS_RESOURCE`. It covers the real Runner binary, flags that are not valid, a Runner that cannot apply a negative score, and a supervised child of a Runner with a negative score. The tests for children of a Runner with a negative score skip themselves without `CAP_SYS_RESOURCE`. The unit test covers the same reset logic without that capability. I also ran the debug Runner binary through the agent computer `enter` command in a container, as UID 1000, with a floor of -500: - Runner stayed alive at -500. - The `oom_score` integration test passed in the same setup. Its supervised child had the score 0. After the flag gate commit, I ran the library `oom_score` tests, `--test oom_score`, `--test oom_score_without_flag`, and `--test process_supervisor` again. CI runs the complete suite. I did not run this on a real VM. ## Risks - A fork in the short time after exec and before the write keeps the Runner score. The kernel can still kill that process unless the score is -1000. - The write to the child can fail. Runner ignores this failure, because the child has no more protection than Runner. - A launcher without the necessary privilege gets a Runner at the inherited score. Runner writes a diagnostic to standard error, and the run continues. A launcher that discards standard error does not see the diagnostic. - A launcher that does not give the flag has no change, also when Runner starts with a negative score. On Linux, each spawn has one more atomic read and no system call. A `oom_score_without_flag` test binary checks this. - A new Runner under a launcher that does not give the flag and gives Runner -1000 keeps its children at -1000. Update that launcher to give the flag. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
e5713d95c6
commit
32f93cf9dd
6 files changed
+319
-2
No files matched your search
@@ -14,6 +14,7 @@ use paperclip_runner_core::durable::{
|
||||
};
|
||||
use paperclip_runner_core::local_runner::{run_local_runner, LocalRunnerError, RunnerConfig};
|
||||
use paperclip_runner_core::native_provider_backend::NativeProviderCommandExecutor;
|
||||
use paperclip_runner_core::oom_score::{parse_oom_score_adj, set_own_oom_score_adj};
|
||||
use serde_json::json;
|
||||
|
||||
const RUNNERD_BUILD_METADATA_SCHEMA: &str = "paperclip-runner/runnerd-build-metadata/v1";
|
||||
@@ -452,6 +453,20 @@ fn run_durable(args: &[String]) -> Result<(), LocalRunnerError> {
|
||||
.map_err(|error| LocalRunnerError::invalid(error.to_string()))
|
||||
}
|
||||
|
||||
fn apply_oom_score_adj(args: &[String]) -> Result<(), LocalRunnerError> {
|
||||
let Some(value) = optional_value(args, "--oom-score-adj")? else {
|
||||
return Ok(());
|
||||
};
|
||||
let score = parse_oom_score_adj(&value).ok_or_else(|| {
|
||||
LocalRunnerError::invalid("invalid --oom-score-adj: expected -1000..=1000")
|
||||
})?;
|
||||
// Without the privilege, Runner is only less protected than requested.
|
||||
if let Err(error) = set_own_oom_score_adj(score) {
|
||||
eprintln!("paperclip-runnerd: failed to set --oom-score-adj: {error}");
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn run(args: &[String]) -> Result<(), LocalRunnerError> {
|
||||
#[cfg(unix)]
|
||||
if matches!(
|
||||
@@ -471,6 +486,7 @@ fn run(args: &[String]) -> Result<(), LocalRunnerError> {
|
||||
println!("{}", metadata_with_provider_packs(args)?);
|
||||
return Ok(());
|
||||
}
|
||||
apply_oom_score_adj(args)?;
|
||||
if args.iter().any(|argument| argument == "--connect-url")
|
||||
|| args.iter().any(|argument| argument == "--listen-address")
|
||||
|| args.iter().any(|argument| argument == "--listen-port")
|
||||
|
||||
@@ -26,6 +26,7 @@ pub mod local_runner;
|
||||
pub mod managed_provider;
|
||||
pub mod managed_provider_backend;
|
||||
pub mod native_provider_backend;
|
||||
pub mod oom_score;
|
||||
pub mod process_supervisor;
|
||||
pub mod provider_backend;
|
||||
pub mod provider_bridge;
|
||||
|
||||
@@ -0,0 +1,105 @@
|
||||
//! Linux out-of-memory priority for Runner and the processes it supervises.
|
||||
|
||||
use std::io;
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
|
||||
const SCORE_RANGE: std::ops::RangeInclusive<i16> = -1000..=1000;
|
||||
|
||||
// Set only by the launch flag, so a launcher that does not opt in keeps today's
|
||||
// inheritance even when Runner starts with a negative score.
|
||||
static RELEASE_SUPERVISED_PROCESSES: AtomicBool = AtomicBool::new(false);
|
||||
|
||||
pub fn parse_oom_score_adj(value: &str) -> Option<i16> {
|
||||
value
|
||||
.parse()
|
||||
.ok()
|
||||
.filter(|score| SCORE_RANGE.contains(score))
|
||||
}
|
||||
|
||||
/// Each environment decides how much Runner is worth protecting, so Runner has
|
||||
/// no default. A negative value needs privilege or a floor set by the launcher;
|
||||
/// callers treat a failure as non-fatal because it only leaves Runner killable.
|
||||
pub fn set_own_oom_score_adj(score: i16) -> io::Result<()> {
|
||||
// The launcher opted in even if this write fails and Runner keeps its score.
|
||||
RELEASE_SUPERVISED_PROCESSES.store(true, Ordering::Relaxed);
|
||||
#[cfg(target_os = "linux")]
|
||||
{
|
||||
std::fs::write("/proc/self/oom_score_adj", score.to_string())
|
||||
}
|
||||
#[cfg(not(target_os = "linux"))]
|
||||
{
|
||||
let _ = score;
|
||||
Err(io::Error::new(
|
||||
io::ErrorKind::Unsupported,
|
||||
"oom_score_adj is supported only on Linux",
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
/// Children inherit Runner's score. With the launch flag set, protection meant for
|
||||
/// Runner must not make harness and tool processes as hard to kill as Runner.
|
||||
///
|
||||
/// A fork in the window after exec keeps Runner's score. So does the child if this
|
||||
/// write fails, which leaves it no more protected than Runner, so it is not fatal.
|
||||
pub(crate) fn release_supervised_process(pid: u32) {
|
||||
if !RELEASE_SUPERVISED_PROCESSES.load(Ordering::Relaxed) {
|
||||
return;
|
||||
}
|
||||
#[cfg(target_os = "linux")]
|
||||
if let Some(score) = std::fs::read_to_string("/proc/self/oom_score_adj")
|
||||
.ok()
|
||||
.and_then(|score| score.trim().parse().ok())
|
||||
{
|
||||
release_from_score(score, pid);
|
||||
}
|
||||
#[cfg(not(target_os = "linux"))]
|
||||
let _ = pid;
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
fn release_from_score(own_score: i16, pid: u32) {
|
||||
if own_score < 0 {
|
||||
let _ = std::fs::write(format!("/proc/{pid}/oom_score_adj"), "0");
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn parses_only_the_kernel_range() {
|
||||
assert_eq!(parse_oom_score_adj("-1000"), Some(-1000));
|
||||
assert_eq!(parse_oom_score_adj("1000"), Some(1000));
|
||||
assert_eq!(parse_oom_score_adj("-1001"), None);
|
||||
assert_eq!(parse_oom_score_adj("1001"), None);
|
||||
assert_eq!(parse_oom_score_adj("low"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[cfg(target_os = "linux")]
|
||||
fn releases_children_only_from_a_protected_score() {
|
||||
struct Sleeper(std::process::Child);
|
||||
impl Drop for Sleeper {
|
||||
fn drop(&mut self) {
|
||||
let _ = self.0.kill();
|
||||
let _ = self.0.wait();
|
||||
}
|
||||
}
|
||||
let child = Sleeper(
|
||||
std::process::Command::new("sleep")
|
||||
.arg("30")
|
||||
.spawn()
|
||||
.unwrap(),
|
||||
);
|
||||
let pid = child.0.id();
|
||||
let path = format!("/proc/{pid}/oom_score_adj");
|
||||
let score = || std::fs::read_to_string(&path).unwrap().trim().to_owned();
|
||||
std::fs::write(&path, "7").unwrap();
|
||||
|
||||
release_from_score(0, pid);
|
||||
assert_eq!(score(), "7");
|
||||
release_from_score(-1, pid);
|
||||
assert_eq!(score(), "0");
|
||||
}
|
||||
}
|
||||
@@ -460,7 +460,7 @@ impl VerifiedProcessLaunch {
|
||||
environment: &std::collections::BTreeMap<String, String>,
|
||||
) -> Result<Child, LocalRunnerError> {
|
||||
let inherited = self.inherited_command()?;
|
||||
Command::new(&inherited.program)
|
||||
let child = Command::new(&inherited.program)
|
||||
.args(&inherited.args)
|
||||
.env_clear()
|
||||
.envs(environment)
|
||||
@@ -468,7 +468,9 @@ impl VerifiedProcessLaunch {
|
||||
.stdout(Stdio::piped())
|
||||
.stderr(Stdio::inherit())
|
||||
.spawn()
|
||||
.map_err(|_| LocalRunnerError::invalid("failed to spawn verified stdio MCP server"))
|
||||
.map_err(|_| LocalRunnerError::invalid("failed to spawn verified stdio MCP server"))?;
|
||||
crate::oom_score::release_supervised_process(child.id());
|
||||
Ok(child)
|
||||
}
|
||||
|
||||
pub fn with_inherited_runtime_executable(mut self) -> Self {
|
||||
@@ -1077,6 +1079,7 @@ impl SupervisedProcess {
|
||||
))
|
||||
})?;
|
||||
let process_group_id = child.id();
|
||||
crate::oom_score::release_supervised_process(process_group_id);
|
||||
let stdin = child
|
||||
.stdin
|
||||
.take()
|
||||
|
||||
@@ -0,0 +1,148 @@
|
||||
#![cfg(target_os = "linux")]
|
||||
|
||||
use std::fs;
|
||||
use std::net::TcpListener;
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
use std::path::Path;
|
||||
use std::process::{Command, Stdio};
|
||||
use std::thread;
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
use paperclip_runner_core::oom_score::set_own_oom_score_adj;
|
||||
use paperclip_runner_core::process_supervisor::SupervisedProcess;
|
||||
|
||||
fn listener_runner(directory: &Path, port: u16, flag: &[&str]) -> Command {
|
||||
let mut command = Command::new(env!("CARGO_BIN_EXE_paperclip-runnerd"));
|
||||
command
|
||||
.env(
|
||||
"PAPERCLIP_RUNNER_BOOTSTRAP_TICKET",
|
||||
"fixture-bootstrap-ticket",
|
||||
)
|
||||
.args(flag)
|
||||
.args(["--listen-address", "0.0.0.0", "--listen-port"])
|
||||
.arg(port.to_string())
|
||||
.args(["--listen-path", "/api/runner/v1/connect/run", "--state-dir"])
|
||||
.arg(directory)
|
||||
.args([
|
||||
"--runner-id",
|
||||
"runner",
|
||||
"--environment-lease-id",
|
||||
"lease",
|
||||
"--run-id",
|
||||
"run",
|
||||
"--session-id",
|
||||
"session",
|
||||
"--turn-id",
|
||||
"turn",
|
||||
"--item-id",
|
||||
"item",
|
||||
"--runner-version",
|
||||
"0.0.0",
|
||||
"--runner-digest",
|
||||
&format!("sha256:{}", "a".repeat(64)),
|
||||
"--max-runtime-ms",
|
||||
"10000",
|
||||
])
|
||||
.stdout(Stdio::null())
|
||||
.stderr(Stdio::piped());
|
||||
command
|
||||
}
|
||||
|
||||
fn private_directory() -> std::path::PathBuf {
|
||||
let directory = std::env::temp_dir().join(format!("runner-oom-{}", uuid::Uuid::new_v4()));
|
||||
fs::create_dir(&directory).unwrap();
|
||||
fs::set_permissions(&directory, fs::Permissions::from_mode(0o700)).unwrap();
|
||||
directory
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn launch_flag_sets_the_runner_score() {
|
||||
let directory = private_directory();
|
||||
let reservation = TcpListener::bind("0.0.0.0:0").unwrap();
|
||||
let port = reservation.local_addr().unwrap().port();
|
||||
drop(reservation);
|
||||
let mut runner = listener_runner(&directory, port, &["--oom-score-adj", "9"])
|
||||
.spawn()
|
||||
.unwrap();
|
||||
let path = format!("/proc/{}/oom_score_adj", runner.id());
|
||||
let deadline = Instant::now() + Duration::from_secs(10);
|
||||
let mut score = String::new();
|
||||
while Instant::now() < deadline {
|
||||
score = fs::read_to_string(&path).unwrap();
|
||||
if score.trim() == "9" {
|
||||
break;
|
||||
}
|
||||
thread::sleep(Duration::from_millis(10));
|
||||
}
|
||||
let _ = runner.kill();
|
||||
let _ = runner.wait();
|
||||
let _ = fs::remove_dir_all(&directory);
|
||||
assert_eq!(score.trim(), "9");
|
||||
}
|
||||
|
||||
// Lowering is refused without privilege, and that must not stop the run.
|
||||
#[test]
|
||||
fn launch_flag_starts_without_the_privilege_to_apply_it() {
|
||||
let directory = private_directory();
|
||||
let reservation = TcpListener::bind("0.0.0.0:0").unwrap();
|
||||
let port = reservation.local_addr().unwrap().port();
|
||||
drop(reservation);
|
||||
let mut runner = listener_runner(&directory, port, &["--oom-score-adj", "-1"])
|
||||
.stderr(Stdio::null())
|
||||
.spawn()
|
||||
.unwrap();
|
||||
thread::sleep(Duration::from_millis(500));
|
||||
let status = runner.try_wait().unwrap();
|
||||
let _ = runner.kill();
|
||||
let _ = runner.wait();
|
||||
let _ = fs::remove_dir_all(&directory);
|
||||
assert_eq!(status, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn launch_flag_rejects_an_invalid_score() {
|
||||
let directory = private_directory();
|
||||
for flag in [
|
||||
&["--oom-score-adj", "low"][..],
|
||||
&["--oom-score-adj", "1001"][..],
|
||||
&["--oom-score-adj"][..],
|
||||
] {
|
||||
let output = listener_runner(&directory, 43127, flag).output().unwrap();
|
||||
assert!(!output.status.success());
|
||||
}
|
||||
let _ = fs::remove_dir_all(&directory);
|
||||
}
|
||||
|
||||
// Runs under CAP_SYS_RESOURCE only: nothing else in this binary reads its score.
|
||||
#[test]
|
||||
fn supervised_processes_do_not_inherit_protection() {
|
||||
if set_own_oom_score_adj(-1).is_err() {
|
||||
eprintln!("skipped: lowering oom_score_adj requires CAP_SYS_RESOURCE");
|
||||
return;
|
||||
}
|
||||
let directory = private_directory();
|
||||
let result = directory.join("score");
|
||||
let mut process = SupervisedProcess::spawn(
|
||||
Path::new("/bin/sh"),
|
||||
&[
|
||||
"-c".to_owned(),
|
||||
format!(
|
||||
"read -r start; cat /proc/self/oom_score_adj > {}.tmp; mv {0}.tmp {0}; read -r finish",
|
||||
result.display()
|
||||
),
|
||||
],
|
||||
Duration::from_secs(2),
|
||||
1024,
|
||||
)
|
||||
.unwrap();
|
||||
// Read the score only after spawn returns, which is when Runner has reset it.
|
||||
process.send(&"start").unwrap();
|
||||
let deadline = Instant::now() + Duration::from_secs(5);
|
||||
while !result.exists() && Instant::now() < deadline {
|
||||
thread::sleep(Duration::from_millis(10));
|
||||
}
|
||||
let score = fs::read_to_string(&result).unwrap();
|
||||
let _ = set_own_oom_score_adj(0);
|
||||
let _ = fs::remove_dir_all(&directory);
|
||||
assert_eq!(score.trim(), "0");
|
||||
}
|
||||
@@ -0,0 +1,44 @@
|
||||
#![cfg(target_os = "linux")]
|
||||
|
||||
use std::fs;
|
||||
use std::path::Path;
|
||||
use std::thread;
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
use paperclip_runner_core::process_supervisor::SupervisedProcess;
|
||||
|
||||
// A separate test binary: the flag's process-wide opt-in must stay unset here.
|
||||
// Runs under CAP_SYS_RESOURCE only, because lowering a score needs it.
|
||||
#[test]
|
||||
fn supervised_processes_keep_inheritance_without_the_flag() {
|
||||
if fs::write("/proc/self/oom_score_adj", "-1").is_err() {
|
||||
eprintln!("skipped: lowering oom_score_adj requires CAP_SYS_RESOURCE");
|
||||
return;
|
||||
}
|
||||
let directory = std::env::temp_dir().join(format!("runner-oom-{}", uuid::Uuid::new_v4()));
|
||||
fs::create_dir(&directory).unwrap();
|
||||
let result = directory.join("score");
|
||||
let mut process = SupervisedProcess::spawn(
|
||||
Path::new("/bin/sh"),
|
||||
&[
|
||||
"-c".to_owned(),
|
||||
format!(
|
||||
"read -r start; cat /proc/self/oom_score_adj > {}.tmp; mv {0}.tmp {0}; read -r finish",
|
||||
result.display()
|
||||
),
|
||||
],
|
||||
Duration::from_secs(2),
|
||||
1024,
|
||||
)
|
||||
.unwrap();
|
||||
// Read the score only after spawn returns, when a reset would have happened.
|
||||
process.send(&"start").unwrap();
|
||||
let deadline = Instant::now() + Duration::from_secs(5);
|
||||
while !result.exists() && Instant::now() < deadline {
|
||||
thread::sleep(Duration::from_millis(10));
|
||||
}
|
||||
let score = fs::read_to_string(&result).unwrap();
|
||||
let _ = fs::write("/proc/self/oom_score_adj", "0");
|
||||
let _ = fs::remove_dir_all(&directory);
|
||||
assert_eq!(score.trim(), "-1");
|
||||
}
|
||||
Reference in new issue
Block a user