bxcore::instance::lock (File::try_lock, file 0600, missing directory 0700), LockError::line for the two lines the daemons print, and the already-running runbook fragment; loopd locks <home>/run/loopd.lock before its socket and self-test, inferproxy <socket>.lock. Checks: given tests fail at ref 11 as the task says (bxcore E0432; loopd 3 and inferproxy 3 fail); the task's quoted edits applied to ref 11 reproduce ref 12 byte for byte; replay gate ok with 1113 tests (1099 + 14), /tmp unchanged; 25 mutations, 24 caught (the uncaught one maps TryLockError::Error to Held, not reachable from a test); given files rustfmt-clean, under 500 lines; no chat-template tags. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
328 lines
16 KiB
Markdown
328 lines
16 KiB
Markdown
# M5b task 12: a lock file keeps `loopd` and `inferproxy` to one instance each
|
|
|
|
**Branch:** `m5b` (run `git switch m5b`; `git status --short` must be empty, otherwise stop)
|
|
**Commit subject:** `bxcore, loopd, inferproxy: a lock file keeps each to one instance`
|
|
|
|
## Goal
|
|
|
|
Every daemon removes a socket it finds at its path before it binds, so a socket left by a daemon
|
|
that was killed never blocks the next start. But the removal is unconditional: a second copy of a
|
|
daemon, started by mistake, takes the path from the one that runs, which keeps running on a socket
|
|
nobody can reach, and nothing says so. From this task on, a daemon refuses to start instead (M5b
|
|
spec, section 9a):
|
|
|
|
- **Before it touches its sockets**, each daemon takes an exclusive lock on a lock file with the
|
|
standard library's `File::try_lock`, and holds it for the life of the process. The system
|
|
releases it when the process ends, however it ends, so a killed daemon never leaves a lock
|
|
behind. The file itself stays where it is.
|
|
- **The lock file** is `<home>/run/<role>.lock` (here `loopd`'s: `<home>/run/loopd.lock`), and
|
|
for `inferproxy`, which has no home, `<socket>.lock` beside its `--listen` socket.
|
|
- **Another instance holds it**: the daemon exits 1 with one line, and touches nothing else:
|
|
|
|
```text
|
|
loopd: another loopd is running (it holds /var/lib/boxmaker/run/loopd.lock); see docs/runbook.md#already-running
|
|
```
|
|
|
|
- **Any other error** on the lock file (a `run/` it cannot write, a read-only socket directory):
|
|
it exits 1, naming the file and the error:
|
|
|
|
```text
|
|
inferproxy: cannot take the lock /run/bx/infer.sock.lock: Permission denied (os error 13); see docs/runbook.md#already-running
|
|
```
|
|
|
|
This task writes the lock itself, `bxcore::instance`, and makes `loopd` and `inferproxy` take it.
|
|
Task 13 does the same for `brokerd`, `memoryd` and `gatewayd`. The runbook entry the pointer
|
|
names, `already-running`, is given as a fragment (it already covers all five daemons).
|
|
|
|
| Name | What it is |
|
|
|---|---|
|
|
| `bxcore::instance` | new module (given as a skeleton; you fill two functions) |
|
|
| `instance::lock` | `pub fn lock(path: &Path) -> Result<InstanceLock, LockError>` |
|
|
| `instance::InstanceLock` | the held lock: the open file. Given. |
|
|
| `instance::LockError` | `Held`, or `Io(std::io::Error)`, with `Display` and `Error`. Given. |
|
|
| `LockError::line` | `pub fn line(&self, role: &str, path: &Path) -> String`: the line the daemon prints |
|
|
| `instance::RUNBOOK` | `"see docs/runbook.md#already-running"`. Given. |
|
|
|
|
## Files
|
|
|
|
- Copy, from `docs/plans/M5b/files/` (the same path under it):
|
|
- the skeleton `crates/bxcore/src/instance.rs` (you fill 2 functions in it)
|
|
- tests: `crates/bxcore/tests/instance.rs`, `crates/loopd/tests/instance.rs`,
|
|
`crates/inferproxy/tests/instance.rs` (all three new)
|
|
- Append, from `docs/plans/M5b/files/`, to the end of `docs/runbook.md`, with one blank line
|
|
before it: `runbook-already-running.md` (step 3 gives the command).
|
|
- Modify: `crates/bxcore/src/lib.rs` (L1, L2), `crates/loopd/src/main.rs` (M1, M2),
|
|
`crates/inferproxy/src/main.rs` (P1, P2), `docs/implementer-log.md`.
|
|
- Do not touch: any other source file, any `Cargo.toml`, any other test, and in the skeleton
|
|
anything but the 2 `todo!()` bodies.
|
|
|
|
## Facts you need
|
|
|
|
- **`File::try_lock`** (standard library, stable since Rust 1.89; the workspace needs 1.95, so it
|
|
is there): `pub fn try_lock(&self) -> Result<(), TryLockError>`. It tries once and never waits.
|
|
On Unix it is `flock` with `LOCK_EX | LOCK_NB`. From its documentation: it returns
|
|
`Err(TryLockError::WouldBlock)` "if a different lock is already held on this file (via another
|
|
handle/descriptor)", and "the lock will be released when this file (along with any other file
|
|
descriptors/handles duplicated or inherited from it) is closed". So:
|
|
- Two separate opens of the same file are two holders, in one process as in two. The bxcore
|
|
tests rely on that: they call `lock` twice in one test.
|
|
- The lock lives as long as the `File` does. `InstanceLock` holds the `File`, and each daemon
|
|
keeps its `InstanceLock` in a local variable of the function that serves, so it lives until
|
|
the process ends. `let _ = lock(..)` or a `match` arm `Ok(_) => {}` would drop it at once,
|
|
and the lock with it; the edits bind it to a name, `_instance`.
|
|
- Rust opens files with close-on-exec, so a program a daemon starts does not inherit the lock.
|
|
- **`std::fs::TryLockError`** (stable since 1.89): `pub enum TryLockError { Error(io::Error),
|
|
WouldBlock }`. `Error` never holds an error of kind `WouldBlock`.
|
|
- **The modes.** `OpenOptionsExt::mode(0o600)` and `DirBuilderExt::mode(0o700)` apply only when
|
|
the file or directory is created, and the process's umask can only remove bits. An existing
|
|
lock file is opened as it is, and never truncated (`truncate(false)`): it holds nothing, and a
|
|
killed daemon's file must be taken again unchanged.
|
|
- **The directory.** `lock` creates the lock file's directory when it is missing, with
|
|
`DirBuilder::new().recursive(true).mode(0o700)`, as `brokerd`, `memoryd` and `gatewayd` already
|
|
make their socket directories (and so `<home>/run` on a first start). An existing directory is
|
|
fine, and an empty parent (a lock file named without a directory) is the current directory,
|
|
also fine. A directory that cannot be written is not made writable: the error is
|
|
`LockError::Io`, and the daemon stops with the second line of the Goal.
|
|
- **Where the lock is taken.**
|
|
- `loopd serve` (`run_serve` in `crates/loopd/src/main.rs`): right after the config loads, before
|
|
the stale socket is removed, and so before the self-test. A second `loopd` then never sends
|
|
the server a prompt either, which would take slot 0's cache from the one that runs. The home
|
|
is `cfg.paths.home` (`[paths] home`, or `$BOXMAKER_HOME`). `loopd selftest` takes no lock: it
|
|
binds no socket.
|
|
- `inferproxy` (`main` in `crates/inferproxy/src/main.rs`): right after the arguments are
|
|
parsed, before the stale socket is removed. The lock file is the `--listen` path with `.lock`
|
|
added. Since `lock` makes a missing directory, `inferproxy` now also starts when its socket's
|
|
directory does not exist yet; before, `bind` failed there.
|
|
- **The given tests start the daemons as processes.** Every wait has a deadline and every process
|
|
is killed by a guard. Before this task, the tests that expect a second instance to be refused
|
|
wait out that deadline (the second copy starts and serves), so the `loopd` file takes about 20 s
|
|
to fail and the `inferproxy` file about 10 s. A test that "hangs" for that long is this, not a
|
|
fault. The fake inference server in `crates/loopd/tests/support/mod.rs` does not change.
|
|
- The calls the bodies and edits make:
|
|
|
|
| Call | Signature, and what it gives |
|
|
|---|---|
|
|
| `DirBuilder` | `DirBuilder::new().recursive(true).mode(0o700).create(dir) -> io::Result<()>` |
|
|
| `OpenOptions` | `OpenOptions::new().write(true).create(true).truncate(false).mode(0o600).open(path) -> io::Result<File>` |
|
|
| `File::try_lock` | `file.try_lock() -> Result<(), TryLockError>` |
|
|
| `Result::map_err` | `.map_err(LockError::Io)?`: an `io::Error` becomes `LockError::Io` |
|
|
| `Path::parent` | `path.parent() -> Option<&Path>` |
|
|
| `format!`, `Path::display` | `format!("… {} …", path.display())`; `{RUNBOOK}` and `{e}` inline |
|
|
| `instance::lock` | `instance::lock(&lock_path) -> Result<InstanceLock, LockError>` |
|
|
| `LockError::line` | `e.line("loopd", &lock_path) -> String` |
|
|
| `OsString::push` | `let mut lock_path = path.clone().into_os_string(); lock_path.push(".lock");` |
|
|
|
|
## The edits, exactly
|
|
|
|
Nothing else in these files changes. Each quoted "before" block occurs exactly once in its file.
|
|
|
|
### `crates/bxcore/src/lib.rs`
|
|
|
|
**L1.** The lines
|
|
|
|
```rust
|
|
//! Shared data types and the frame codec for Boxmaker. No policy and no I/O beyond frames, except
|
|
//! the secret store (`secrets`), behind the feature of that name.
|
|
```
|
|
|
|
become
|
|
|
|
```rust
|
|
//! Shared data types and the frame codec for Boxmaker. No policy and no I/O beyond frames, except
|
|
//! the secret store (`secrets`), behind the feature of that name, and the lock that keeps a daemon
|
|
//! to one instance (`instance`).
|
|
```
|
|
|
|
**L2.** The line
|
|
|
|
```rust
|
|
pub mod ids;
|
|
```
|
|
|
|
becomes
|
|
|
|
```rust
|
|
pub mod ids;
|
|
pub mod instance;
|
|
```
|
|
|
|
`instance` is not behind a feature: `loopd` uses it.
|
|
|
|
### `crates/loopd/src/main.rs`
|
|
|
|
**M1.** The lines
|
|
|
|
```rust
|
|
use std::time::Duration;
|
|
|
|
use loopd::broker_port::{BrokerPort, NoBroker, not_configured_line};
|
|
```
|
|
|
|
become
|
|
|
|
```rust
|
|
use std::time::Duration;
|
|
|
|
use bxcore::instance;
|
|
use loopd::broker_port::{BrokerPort, NoBroker, not_configured_line};
|
|
```
|
|
|
|
**M2.** In `run_serve`, the lines
|
|
|
|
```rust
|
|
let socket = cfg.channel_socket();
|
|
if socket.exists()
|
|
```
|
|
|
|
become
|
|
|
|
```rust
|
|
// One loopd per home (M5b spec, section 9a). The lock comes before the socket is touched and
|
|
// before the self-test, so a second copy neither takes the socket of the one that runs nor
|
|
// sends its slot a prompt. Held until the process ends.
|
|
let lock_path = cfg.paths.home.join("run/loopd.lock");
|
|
let _instance = match instance::lock(&lock_path) {
|
|
Ok(held) => held,
|
|
Err(e) => {
|
|
eprintln!("{}", e.line("loopd", &lock_path));
|
|
return ExitCode::from(1);
|
|
}
|
|
};
|
|
|
|
let socket = cfg.channel_socket();
|
|
if socket.exists()
|
|
```
|
|
|
|
### `crates/inferproxy/src/main.rs`
|
|
|
|
**P1.** The line
|
|
|
|
```rust
|
|
use inferproxy::{Limits, serve};
|
|
```
|
|
|
|
becomes
|
|
|
|
```rust
|
|
use bxcore::instance;
|
|
use inferproxy::{Limits, serve};
|
|
```
|
|
|
|
`inferproxy` depends on `bxcore` already; its `Cargo.toml` does not change.
|
|
|
|
**P2.** In `main`, the lines
|
|
|
|
```rust
|
|
let path: PathBuf = listen.into();
|
|
match fs::remove_file(&path) {
|
|
```
|
|
|
|
become
|
|
|
|
```rust
|
|
let path: PathBuf = listen.into();
|
|
// One inferproxy per socket (M5b spec, section 9a): the lock beside the socket comes before
|
|
// the stale socket is removed, so a second copy never takes the socket of the one that runs.
|
|
// Held until the process ends; `process::exit` leaves it to the system to release.
|
|
let mut lock_path = path.clone().into_os_string();
|
|
lock_path.push(".lock");
|
|
let lock_path = PathBuf::from(lock_path);
|
|
let _instance = match instance::lock(&lock_path) {
|
|
Ok(held) => held,
|
|
Err(e) => {
|
|
eprintln!("{}", e.line("inferproxy", &lock_path));
|
|
process::exit(1);
|
|
}
|
|
};
|
|
match fs::remove_file(&path) {
|
|
```
|
|
|
|
## Steps
|
|
|
|
- [ ] **1. Copy the given tests.** From the repository root, three commands:
|
|
|
|
```sh
|
|
cp docs/plans/M5b/files/crates/bxcore/tests/instance.rs crates/bxcore/tests/
|
|
cp docs/plans/M5b/files/crates/loopd/tests/instance.rs crates/loopd/tests/
|
|
cp docs/plans/M5b/files/crates/inferproxy/tests/instance.rs crates/inferproxy/tests/
|
|
```
|
|
|
|
- [ ] **2. See them fail.** Three commands, one per file, each with a shell timeout of 600000 ms:
|
|
- `cargo test -p bxcore --test instance` does not compile: one `E0432`, unresolved import
|
|
`bxcore::instance`.
|
|
- `cargo test -p loopd --test instance` compiles, and its 3 tests fail (the run takes about
|
|
20 s):
|
|
|
|
| Test | Before this task |
|
|
|---|---|
|
|
| `a_second_loopd_on_the_same_home_is_refused_and_the_first_keeps_serving` | `the second loopd: …`, `left: None`, `right: Some(1)`: the second copy started and served |
|
|
| `after_the_first_is_killed_a_new_loopd_removes_the_stale_socket_and_serves` | `the lock file, mode 0600`, `left: None`, `right: Some(384)` |
|
|
| `a_run_directory_that_cannot_be_written_stops_loopd_naming_the_error` | `wanted the line` …: `loopd` ran its self-test and failed later, on the socket directory |
|
|
|
|
- `cargo test -p inferproxy --test instance` compiles, and its 3 tests fail (about 10 s): the
|
|
same three shapes, `the second inferproxy: …` with `left: None`, `the lock file, beside the
|
|
socket` with `left: None`, and `wanted the line` for the read-only directory.
|
|
- [ ] **3. The runbook.** From the repository root:
|
|
|
|
```sh
|
|
{ echo; cat docs/plans/M5b/files/runbook-already-running.md; } >> docs/runbook.md
|
|
```
|
|
|
|
Then `grep -c '^## already-running$' docs/runbook.md` prints `1`.
|
|
- [ ] **4. The module.** Copy the skeleton, then make edits L1 and L2:
|
|
|
|
```sh
|
|
cp docs/plans/M5b/files/crates/bxcore/src/instance.rs crates/bxcore/src/
|
|
```
|
|
|
|
`cargo check -p bxcore`: no error, and 5 warnings, all from the unfilled bodies: two lines of
|
|
unused imports (`DirBuilder`, `OpenOptions`, `TryLockError`; `DirBuilderExt`,
|
|
`OpenOptionsExt`) and three unused variables (`path` twice, `role`). They go as you fill the
|
|
bodies. `cargo test -p bxcore --test instance` now compiles: 1 test passes
|
|
(`the_error_displays_and_has_a_source`: `Display` and `Error` are given), and 7 fail with
|
|
`not yet implemented`.
|
|
- [ ] **5. Fill the two functions of `instance.rs`, one at a time, in this order**, each from its
|
|
comment, and run `cargo check -p bxcore` after each: `LockError::line`, then `lock`. Change
|
|
nothing else in the file. Then `cargo test -p bxcore --test instance`: 8 passed.
|
|
- [ ] **6. `loopd`.** Make edits M1 and M2. `cargo check -p loopd`: no error.
|
|
`cargo test -p loopd --test instance`: 3 passed.
|
|
- [ ] **7. `inferproxy`.** Make edits P1 and P2. `cargo check -p inferproxy`: no error.
|
|
`cargo test -p inferproxy --test instance`: 3 passed.
|
|
- [ ] **8. Check.** `grep -rn 'runbook.md#already-running' crates/*/src` prints exactly one line,
|
|
the `RUNBOOK` constant in `crates/bxcore/src/instance.rs`: both daemons print the pointer
|
|
through `line`, never a copy of it. `git diff --stat -- '*Cargo.toml'` prints nothing.
|
|
- [ ] **9. Run the gate.** `cargo fmt --all`, then `make gate`, with a shell timeout of 600000 ms.
|
|
Expected last line: `gate: ok`, with 1113 tests: 1099 before this task, plus 8 in
|
|
`crates/bxcore/tests/instance.rs`, 3 in `crates/loopd/tests/instance.rs` and 3 in
|
|
`crates/inferproxy/tests/instance.rs`.
|
|
- [ ] **10. Log and commit.** `git status --short` lists `crates/bxcore/src/lib.rs`,
|
|
`crates/loopd/src/main.rs`, `crates/inferproxy/src/main.rs`, `docs/runbook.md` and your log as
|
|
modified, and `crates/bxcore/src/instance.rs` and the three test files as new. Stage with
|
|
`git add crates/bxcore crates/loopd crates/inferproxy docs/runbook.md docs/implementer-log.md
|
|
Cargo.lock`, then commit as `AGENTS.md` says. `Cargo.lock` does not change in this task; the
|
|
`git add` is harmless.
|
|
|
|
## Done when
|
|
|
|
- `make gate` prints `gate: ok` with 1113 tests.
|
|
- `git diff HEAD~1 --stat` lists only `crates/bxcore/src/lib.rs`, `crates/bxcore/src/instance.rs`,
|
|
`crates/loopd/src/main.rs`, `crates/inferproxy/src/main.rs`, the three new test files,
|
|
`docs/runbook.md` and `docs/implementer-log.md`.
|
|
|
|
## Stop and report if
|
|
|
|
- A "before" block of L1, L2, M1, M2, P1 or P2 is not in its file exactly as shown, or is there
|
|
more than once.
|
|
- Step 2, 3 or 4 shows a different result: other errors, other failing tests, or other counts.
|
|
- `cargo check` reports an error after a body you filled or an edit you made, and the comment or
|
|
the edit does not tell you how to fix it. Quote the error, and stop.
|
|
- A comment in the skeleton does not fit the code under it. Quote it and stop.
|
|
- After step 5, 6 or 7 a test fails although each function follows its comment and each edit is
|
|
as shown. Do not change a test or the skeleton's given code: stop, and give the failing tests'
|
|
names and their messages.
|
|
- The fix seems to need a change anywhere else: another file of `loopd` or `inferproxy`, the
|
|
test support files, a `Cargo.toml`. It does not; stop and say why you think so.
|
|
- A test outside these three files fails in the gate: check once whether it fails without your
|
|
change, then stop and report both results.
|
|
- The gate passes with a count other than 1113.
|