Skip to content

Make daemon startup reliable: validated config, atomic lockfile, waiting up - #92

Merged
sequoiap merged 1 commit into
masterfrom
fix/daemon-startup
Oct 4, 2026
Merged

sequoiap merged 1 commit into
masterfrom
fix/daemon-startup

Conversation

@sequoiap

@sequoiap sequoiap commented Oct 4, 2026

Copy link
Copy Markdown
Collaborator

Makes starting the background daemon reliable. Closes #54 and #56. A follow-up PR on the process manager (#50, #51, #52, #53, #59) builds on this one and should be merged after it.

Configuration cross-references (#54)

  • What's checked: PyroLabConfiguration now rejects a config where any reference points to something undefined. That covers each service's daemon and nameservers, each daemon's nameservers, and every autolaunch entry. A service with no daemon: setting refers to one called default, and the error says so.
  • Every problem at once: all dangling references are reported together, one readable line each, through the new describe_config_error().
  • pyrolab config update rejects an invalid file, shows the problems, says the installed config wasn't changed, and exits 1. Before, it either installed a config that later killed the daemon, or printed a pydantic traceback.
  • pyrolab info reports an invalid installed config the same way.
  • pyrolab rename now checks the result before saving. It refuses to rename something other entries still refer to, because that would write an invalid config. Updating those references automatically is pyrolab rename silently orphans every reference to the renamed entity #67, which is still open.

Startup (#54, #56)

  • New pyrolabd.main():
    1. Refuses to start if another daemon is running.
    2. Loads and validates the config before starting the process manager, so a bad config leaves nothing running.
    3. Registers the daemon and publishes the lockfile.
    4. Only then runs autolaunch, in a background thread. The daemon answers requests right away, and ps shows entities appearing as they start.
  • Autolaunch failures are isolated: each entry is launched independently, so one failure is logged and the rest still start.
  • Startup errors are reported: if the daemon can't start, it writes the reason to STARTUP_ERROR_FILE for up to show. The cleanup in finally removes the lockfile only if this process wrote it.

Lockfile (#56)

  • Atomic writes: contents go to a temporary file in the same directory, then os.replace moves it into place. Readers see either no lockfile or a complete one.
  • Tolerant reads: read_lockfile() returns None for a missing or unreadable lockfile instead of raising a ValidationError.
  • Stale detection uses the PID: a lockfile is stale when its PID isn't running, checked with the new utils.pid_is_running(). This replaces "the connection was refused, so delete it", which wrongly treated a busy daemon as dead. On Windows the check uses ctypes, because os.kill there kills the process instead of checking it. No new dependency.
  • Known small race: two pyrolab up commands run at the same instant could both pass the "already running" check. Before, the lockfile was created up front with an exclusive create, but then left empty while the daemon started. os.replace gives up that exclusivity in exchange for atomic writes, which is the trade we chose.

pyrolab up waits

  • Waits for a response: up waits until the new daemon responds, 30 seconds by default (--timeout). If the daemon exits first, up prints why: for example, "invalid configuration in …: autolaunch refers to daemon 'typo', which is not defined". If it times out, up says so and points to ps and the logs.
  • Refuses a second daemon: up won't start another daemon while one is running, including one that's running but not responding. Before, an unresponsive daemon was treated as dead: its lockfile was deleted and a second daemon started next to it.

Thread safety

ProcessManager is now used concurrently by Pyro's request threads, the checkup timer, and the background autolaunch. The methods that read or change its process tables hold a shared reentrant lock. Without it, ps running during autolaunch could fail with "dictionary changed size during iteration".

Found while testing: importing pyrolab.api could lock users out

Importing pyrolab.api reads the user config to set default nameserver settings. Once validation was strict, an invalid installed config made that import raise. Every CLI command would then crash, including config update, the command needed to fix the file, and so would any client script. That import step is now best-effort: problems are logged as a warning. This also fixes two older crashes in the same code: a config with no nameservers (next() on an empty dict), and a public host that can't be resolved on an isolated network (#80).

Checks run

  • Suite: 307 passed, 9 skipped. pre-commit passes on every file.
  • New tests cover:
    • each kind of dangling reference;
    • the lockfile helpers (round trip, no leftover temp file, missing, empty and partial files);
    • pid_is_running for this process, an exited process, and invalid PIDs. CI runs that on Windows too.
    • every up path: success, stale lockfile, already running, running but unresponsive, startup error, unexplained exit, timeout;
    • main() run in a thread over a real Pyro connection: it publishes the lockfile, autolaunches in the background, shuts down, and cleans up;
    • main() with an invalid config, and when another daemon is running;
    • importing pyrolab.api with invalid, unparseable and nameserver-less configs.
  • The tests catch the original bugs: against master, the behavior tests fail: stale and partial lockfiles, config update with dangling references, renames that orphan references, info with an invalid config, and the api import.
  • End-to-end run with real processes, from a scratch copy with an isolated data folder:
    • config update rejected the bad file;
    • up returned in 0.3 s and the services appeared in ps a few seconds later;
    • a second up was refused;
    • a stale lockfile was replaced;
    • with an invalid config installed, up and info both explained the problem, and config update followed by up recovered.
    • No processes were left behind.

🤖 Generated with Claude Code

…ing up

- Config (#54): PyroLabConfiguration rejects dangling cross-references
  (service daemon/nameservers, daemon nameservers, autolaunch entries),
  reporting every problem at once. `config update` and `info` print
  them readably; `rename` refuses to write a config it would invalidate.
- Startup (#54, #56): pyrolabd.main() validates the config before
  starting anything, publishes the lockfile as soon as the daemon can
  answer, then autolaunches in a background thread with each entry
  isolated. A startup failure is written to STARTUP_ERROR_FILE.
- Lockfile (#56): written atomically (temp file + os.replace), read
  tolerantly, and judged stale by whether its PID is running (new
  utils.pid_is_running, using ctypes on Windows) rather than by a
  failed connection.
- `pyrolab up` waits until the daemon responds (--timeout, default 30s)
  and otherwise reports why it exited; it refuses to start a second
  daemon while one is running, even an unresponsive one.
- ProcessManager methods share a reentrant lock, now that autolaunch,
  the checkup timer, and Pyro request threads run concurrently.
- Importing pyrolab.api no longer fails on an invalid user config, one
  with no nameservers, or an unresolvable "public" host; this would
  otherwise have locked users out of every CLI command.

Closes #54
Closes #56

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sequoiap
sequoiap merged commit ffa963f into master Oct 4, 2026
9 checks passed
@sequoiap
sequoiap deleted the fix/daemon-startup branch October 4, 2026 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

An invalid autolaunch entry silently kills the background daemon at startup

1 participant