-
Notifications
You must be signed in to change notification settings - Fork 193
fix(server): add --detach and --pidfile for background/supervised mode (duplicated PR) #525
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,56 @@ | ||||||||||||||||||
| // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||||||||||||||||||
| // SPDX-License-Identifier: Apache-2.0 | ||||||||||||||||||
|
|
||||||||||||||||||
| //! Detached execution for `switchyard-server`. | ||||||||||||||||||
| //! | ||||||||||||||||||
| //! The server is otherwise a foreground process that drains on `SIGTERM`/ | ||||||||||||||||||
| //! `SIGINT`. To run it as a managed background service that outlives the | ||||||||||||||||||
| //! launching terminal, call [`detach_into_background`] *before* the Tokio | ||||||||||||||||||
| //! runtime does significant work: it re-executes the current binary under the | ||||||||||||||||||
| //! system `setsid` in a new session with stdio disconnected and writes a | ||||||||||||||||||
| //! pidfile, so the original process exits and the child keeps serving. Spawning | ||||||||||||||||||
| //! before the async runtime boots avoids the hazard of `fork`/`setsid` after an | ||||||||||||||||||
| //! OS-thread/signal-handler runtime has initialised. | ||||||||||||||||||
|
|
||||||||||||||||||
| use std::io::Write; | ||||||||||||||||||
| use std::path::Path; | ||||||||||||||||||
| use std::process::{Command, Stdio}; | ||||||||||||||||||
|
|
||||||||||||||||||
| /// Re-exec the current binary under `setsid` in a detached background session. | ||||||||||||||||||
| /// | ||||||||||||||||||
| /// Returns `Ok(())` in the detached child (the caller should then boot the | ||||||||||||||||||
| /// server); the parent process exits successfully after spawning the child. | ||||||||||||||||||
| #[cfg(unix)] | ||||||||||||||||||
| pub(crate) fn detach_into_background(pidfile: &Path) -> std::io::Result<()> { | ||||||||||||||||||
| let current = std::env::current_exe()?; | ||||||||||||||||||
| // Use the system `setsid` to spawn a detached session (stable, no unstable | ||||||||||||||||||
| // std features). stdio is disconnected so the child is independent of the | ||||||||||||||||||
| // launching terminal. Drop `--detach` from the re-exec args so the child | ||||||||||||||||||
| // serves normally instead of recursing into another detach. | ||||||||||||||||||
| let mut child = Command::new("setsid"); | ||||||||||||||||||
| child.arg(¤t); | ||||||||||||||||||
| for arg in std::env::args().skip(1) { | ||||||||||||||||||
| if arg != "--detach" { | ||||||||||||||||||
| child.arg(arg); | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
| child.stdin(Stdio::null()).stdout(Stdio::null()).stderr(Stdio::null()); | ||||||||||||||||||
|
|
||||||||||||||||||
| let handle = child.spawn()?; | ||||||||||||||||||
| write_pidfile(pidfile, handle.id())?; | ||||||||||||||||||
|
Comment on lines
+39
to
+40
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win Stop the child if PID-file creation fails. If Minimal cleanup- let handle = child.spawn()?;
- write_pidfile(pidfile, handle.id())?;
+ let mut handle = child.spawn()?;
+ if let Err(error) = write_pidfile(pidfile, handle.id()) {
+ let _ = handle.kill();
+ let _ = handle.wait();
+ return Err(error);
+ }📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
| // Parent exits; the detached child continues and serves in its own session. | ||||||||||||||||||
| std::process::exit(0); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| /// Write `pid` to `path`, creating parent directories as needed. | ||||||||||||||||||
| pub(crate) fn write_pidfile(path: &Path, pid: u32) -> std::io::Result<()> { | ||||||||||||||||||
| if let Some(parent) = path.parent() { | ||||||||||||||||||
| if !parent.as_os_str().is_empty() { | ||||||||||||||||||
| std::fs::create_dir_all(parent)?; | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
| let mut file = std::fs::File::create(path)?; | ||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C4 'write_pidfile|File::create|OpenOptions|pidfile|remove_file' crates/switchyard-server/srcRepository: NVIDIA-NeMo/Switchyard Length of output: 6568 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- daemon outline ---'
ast-grep outline crates/switchyard-server/src/daemon.rs
printf '%s\n' '--- daemon source ---'
cat -n crates/switchyard-server/src/daemon.rs
printf '%s\n' '--- CLI pidfile definitions ---'
cat -n crates/switchyard-server/src/cli.rs | sed -n '1,100p'
printf '%s\n' '--- target and feature configuration ---'
rg -n -C3 '^\[workspace\]|switchyard-server|target|unix|windows|pidfile|detach' Cargo.toml crates/*/Cargo.toml .github 2>/dev/null || true
printf '%s\n' '--- relevant call sites and tests ---'
rg -n -C5 'detach_into_background|write_pidfile|default_pidfile|--pidfile|--detach' .Repository: NVIDIA-NeMo/Switchyard Length of output: 24595 🌐 Web query:
💡 Result: In Rust, the behavior of file creation concerning symbolic links depends on which method is used, with OpenOptions::create_new providing explicit protection against following links [1][2][3]. 1. OpenOptions::create_new: This method is designed to be atomic and secure [1][4][5]. When.create_new(true) is set, Rust ensures that no file exists at the target location, which explicitly includes checking for symbolic links (including dangling ones) [1][2][3]. If a symbolic link exists at the target path, the operation will fail [1][4][3]. This behavior is implemented to prevent security vulnerabilities, such as TOCTOU (Time-of-Check to Time-of-Use) attacks where an unprivileged process might trick a privileged one into overwriting a target file by manipulating a symlink [4][5]. On Unix platforms, this is achieved by using flags that prevent symlink following at the final path element [4]. 2. std::fs::File::create: The standard File::create(path) function is a convenience wrapper that opens a file in write-only mode, creating it if it does not exist or truncating it if it does [6][7]. Unlike create_new, this method does not explicitly prohibit following symbolic links [6][7]. If the target path is a symbolic link, File::create will follow the link and open (and truncate) the file pointed to by the link [6][7]. In summary, if you need to guarantee that you are creating a new file without following existing symbolic links, you must use OpenOptions::new.write(true).create_new(true).open(path) [1][2]. Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
daemon = Path("crates/switchyard-server/src/daemon.rs").read_text()
cli = Path("crates/switchyard-server/src/cli.rs").read_text()
assert "std::fs::File::create(path)" in daemon
assert daemon.index("let handle = child.spawn()?;") < daemon.index("write_pidfile(pidfile, handle.id())?")
assert daemon.index("write_pidfile(pidfile, handle.id())?;") < daemon.index("std::process::exit(0);")
assert "std::env::temp_dir()" in cli
assert re.search(r'dir\.push\("switchyard-server\.pid"\)', cli)
assert "create_new(true)" not in daemon
print("write_pidfile uses truncating File::create")
print("the parent spawns the detached child before writing the PID file")
print("the default PID-file name is fixed under std::env::temp_dir()")
print("write_pidfile has no exclusive-create operation")
PYRepository: NVIDIA-NeMo/Switchyard Length of output: 378 Create the PID file without following symlinks.
🤖 Prompt for AI Agents |
||||||||||||||||||
| writeln!(file, "{pid}")?; | ||||||||||||||||||
| file.flush()?; | ||||||||||||||||||
| Ok(()) | ||||||||||||||||||
| } | ||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Handle re-exec portability and non-Unicode arguments.
The Unix-only
setsidpath assumes that asetsidexecutable is installed, so--detachfails to spawn on systems without it. The re-exec argument handling also usesstd::env::args(), which can panic for a non-UTF-8--pidfilepath. Gate the feature to supported targets or use a native session API, and preserve arguments withargs_os()while removing--detachviaOsStr.📍 Affects 1 file
crates/switchyard-server/src/daemon.rs#L32-L36(this comment)crates/switchyard-server/src/daemon.rs#L30-L30🤖 Prompt for AI Agents
Source: Coding guidelines