feat(daemon): say what to do next on startup, and prove multiple clients agree
Starting the daemon printed 'ready' and nothing else, so the token and the next step were both somewhere else. It now prints what other commands can do from here, and when --bind is given, the token itself with the two config commands a remote client needs. All on stderr, so stdout stays empty and daemon still composes in a pipeline. Adds the guarantee daemon mode is for as a test: eight clients connected at once are each answered correctly, none has its replies crossed with another's, and the daemon still counts all eight. Plus 250 connect-and-drop cycles, well past the connection cap, to catch a per-connection resource that is not released — the shape a daemon that works for a while and then refuses everybody would take. A connection the daemon drops before it can be served now logs why at warning level. A client only ever sees 'the connection closed', so that line is the only account of the reason.
This commit is contained in:
Michel de Graaf
+14
-13
@@ -30,15 +30,15 @@ soulseek-rs daemon status --json >/dev/null 2>&1 || {
|
||||
```
|
||||
|
||||
Wait for it, do not just launch it. A daemon takes a second or two to log in,
|
||||
and `soulseek-rs daemon status … || soulseek-rs daemon &` does **not** do this
|
||||
— `&` backgrounds the whole list, so it returns instantly and the next command
|
||||
and `soulseek-rs daemon status … || soulseek-rs daemon &` does **not** do this.
|
||||
`&` backgrounds the whole list, so it returns instantly and the next command
|
||||
races the login it is supposed to be using. A command that arrives too early
|
||||
finds no daemon and quietly opens a session of its own, which is the outcome
|
||||
this whole section exists to avoid.
|
||||
|
||||
Then use every other command exactly as documented below. They find the daemon
|
||||
by themselves — it listens on a Unix socket only your user can open, so there
|
||||
is nothing to configure, no address to pass, and no token to handle.
|
||||
by themselves. It listens on a Unix socket only your user can open, so there is
|
||||
nothing to configure, no address to pass, and no token to handle.
|
||||
|
||||
**Why this matters, and why it is not optional for you.** Soulseek is a real
|
||||
network of other people's computers, and the server allows **one session per
|
||||
@@ -46,7 +46,7 @@ account**. Without a daemon, every invocation is its own login, and running two
|
||||
at once leaves you two bad choices: share a username and they silently cut each
|
||||
other off (exit 7), or give each run a throwaway name and *register a new
|
||||
account on a public server for every search you make*. The second is what a
|
||||
naive script does, and it is the antisocial one — it is account churn on
|
||||
naive script does, and it is the antisocial one: account churn on
|
||||
infrastructure other people maintain and share.
|
||||
|
||||
A daemon is one login for as long as you work, however many commands you run
|
||||
@@ -156,8 +156,9 @@ participant.
|
||||
**`daemon`** — stay logged in as a service other commands share. Prints
|
||||
progress on stderr and nothing on stdout; it does not return until stopped.
|
||||
While it runs, every other command on this machine uses its session
|
||||
automatically — no flags, no credentials, no second login. `--bind ADDR` also
|
||||
accepts connections from other machines, which need the token.
|
||||
automatically, with no flags, no credentials, and no second login.
|
||||
`--bind ADDR` also accepts connections from other machines, which need the
|
||||
token.
|
||||
**`daemon status`** — one object: `user`, `server`, `version`, `protocol`,
|
||||
`listening`, `listen_port`, `shared_folders`, `shared_files`, `download_dir`,
|
||||
`session_loss`, `clients`, `uptime_secs`. Exit 3 when nothing is running.
|
||||
@@ -165,8 +166,8 @@ accepts connections from other machines, which need the token.
|
||||
**`daemon token`** — the secret a client on another machine needs. Local
|
||||
clients never need it.
|
||||
|
||||
The daemon can also be on a *different* machine — a home server or NAS doing
|
||||
the downloading. Point at it once with `config set daemon host:5030` and
|
||||
The daemon can also be on a *different* machine, such as a home server or NAS
|
||||
doing the downloading. Point at it once with `config set daemon host:5030` and
|
||||
`config set daemon_token <token>`, and every command afterwards drives that
|
||||
machine; files land there, not here. The interface is an open JSON-RPC schema
|
||||
(`rpc.discover` serves it), so a purpose-built client is a reasonable thing to
|
||||
@@ -218,7 +219,7 @@ is the normal way to be selective.
|
||||
|
||||
## Running several at once
|
||||
|
||||
With a daemon running this needs no thought — they all share its one session:
|
||||
With a daemon running this needs no thought. They all share its one session:
|
||||
|
||||
```bash
|
||||
for q in 'gary beck fold' 'marcal steady'; do
|
||||
@@ -230,7 +231,7 @@ wait
|
||||
Without one, the same loop is a bug: the runs share a username, so each login
|
||||
displaces the last and they kill each other's session (exit 7). Giving each run
|
||||
its own `--username` avoids the collision by registering a fresh account on the
|
||||
server for every search, which is worse — do not do it. Start the daemon.
|
||||
server for every search, which is worse. Do not do it. Start the daemon.
|
||||
|
||||
The listener port needs no care either way: a run that finds the configured
|
||||
port taken binds a free one and tells the server about that one instead.
|
||||
@@ -250,8 +251,8 @@ port taken binds a free one and tells the server about that one instead.
|
||||
- **Exit 4 is not a failure.** A search that finds nothing means the query was
|
||||
too narrow or misspelled. Widen it; use `whoami` to rule out the connection.
|
||||
- **Exit 7 is not an answer.** The session ended mid-command, so nothing it saw
|
||||
counts. It means something else logged in as you — almost always a second
|
||||
run of your own. Start a daemon and route through it rather than hunting for
|
||||
counts. Something else logged in as you, almost always a second run of your
|
||||
own. Start a daemon and route through it rather than hunting for
|
||||
an unused name.
|
||||
- **Never put a password in argv** — `ps` shows it to every user on the box.
|
||||
Use `--password-stdin`, the OS keychain, or `SOULSEEK_PASSWORD_CMD`.
|
||||
|
||||
@@ -199,7 +199,7 @@ pub fn run(
|
||||
downloads: Arc::clone(&downloads),
|
||||
download_dir: ctx.download_dir.clone(),
|
||||
server: ctx.settings.server_address.to_string(),
|
||||
token,
|
||||
token: token.clone(),
|
||||
started: Instant::now(),
|
||||
stop: Arc::clone(&stop),
|
||||
open: Arc::new(std::sync::atomic::AtomicUsize::new(0)),
|
||||
@@ -225,7 +225,7 @@ pub fn run(
|
||||
"logged in as {}, sharing {files} files in {folders} folders",
|
||||
session.client.username()
|
||||
));
|
||||
ctx.out.status("ready");
|
||||
ready(ctx, bind, &token);
|
||||
|
||||
// The listeners run on their own threads; this one only has to notice a
|
||||
// `daemon.stop` and write state back on the way past.
|
||||
@@ -247,6 +247,57 @@ pub fn run(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Say what is now possible, once, on stderr.
|
||||
///
|
||||
/// The token goes here rather than only into a file because the moment you
|
||||
/// need it is the moment you bind a port, and hunting for `daemon token`
|
||||
/// afterwards is a poor first experience. stdout stays empty either way, so
|
||||
/// `daemon` still composes in a pipeline.
|
||||
fn ready(ctx: &Ctx, bind: Option<&str>, token: &str) {
|
||||
ctx.out.status("");
|
||||
ctx.out.status("ready — leave this running.");
|
||||
ctx.out.status("");
|
||||
ctx.out
|
||||
.status(" On this machine, nothing else to do. Other commands find");
|
||||
ctx.out
|
||||
.status(" it on their own and share this one login:");
|
||||
ctx.out.status("");
|
||||
ctx.out.status(" soulseek-rs search 'aphex twin'");
|
||||
ctx.out
|
||||
.status(" soulseek-rs (the TUI attaches)");
|
||||
ctx.out.status(" soulseek-rs daemon status");
|
||||
ctx.out.status(" soulseek-rs daemon stop");
|
||||
ctx.out.status("");
|
||||
|
||||
let Some(address) = bind else {
|
||||
ctx.out.status(
|
||||
" To drive it from another machine, restart with --bind ADDR;",
|
||||
);
|
||||
ctx.out
|
||||
.status(" `soulseek-rs daemon token` prints the secret it needs.");
|
||||
ctx.out.status("");
|
||||
return;
|
||||
};
|
||||
|
||||
ctx.out.status(&format!(
|
||||
" From another machine, point a client at {address} with"
|
||||
));
|
||||
ctx.out.status(" this token:");
|
||||
ctx.out.status("");
|
||||
ctx.out.status(&format!(" {token}"));
|
||||
ctx.out.status("");
|
||||
ctx.out
|
||||
.status(" soulseek-rs config set daemon <this-host>:PORT");
|
||||
ctx.out
|
||||
.status(" soulseek-rs config set daemon_token <token above>");
|
||||
ctx.out.status("");
|
||||
ctx.out.status(
|
||||
" That port has no encryption. Keep it off the open internet and \
|
||||
reach it over SSH instead.",
|
||||
);
|
||||
ctx.out.status("");
|
||||
}
|
||||
|
||||
/// Remove the socket file so the next start does not have to reason about
|
||||
/// whether it is stale.
|
||||
fn cleanup() {
|
||||
|
||||
@@ -155,14 +155,20 @@ fn spawn_connection(stream: Stream, daemon: Arc<Daemon>) {
|
||||
if open > MAX_CONNECTIONS {
|
||||
daemon.open.fetch_sub(1, Ordering::Relaxed);
|
||||
soulseek_rs::warn!(
|
||||
"refusing a control connection: {MAX_CONNECTIONS} already open"
|
||||
"refusing a control connection: {MAX_CONNECTIONS} already open. \
|
||||
This is a leak unless you really have that many clients — please \
|
||||
report it with `soulseek-rs daemon status`."
|
||||
);
|
||||
stream.shutdown();
|
||||
return;
|
||||
}
|
||||
std::thread::spawn(move || {
|
||||
if let Err(e) = handle(stream, daemon.as_ref()) {
|
||||
soulseek_rs::debug!("control connection ended: {e}");
|
||||
// Loud, not debug: a client sees only "the connection closed", so
|
||||
// this line is the only account of why it did.
|
||||
soulseek_rs::warn!(
|
||||
"control connection dropped before it could be served: {e}"
|
||||
);
|
||||
}
|
||||
daemon.open.fetch_sub(1, Ordering::Relaxed);
|
||||
});
|
||||
|
||||
@@ -565,3 +565,105 @@ fn a_config_driven_remote_client_reports_the_daemons_world() {
|
||||
let _ = std::fs::remove_dir_all(&config);
|
||||
let _ = std::fs::remove_dir_all(&state);
|
||||
}
|
||||
|
||||
/// Several clients at once, all seeing the same session.
|
||||
///
|
||||
/// This is the promise daemon mode makes and the reason the fan-out exists:
|
||||
/// the server allows one login, so the alternative to sharing is displacing.
|
||||
/// Each client gets its own connection, its own event queue, and the same
|
||||
/// answers.
|
||||
#[test]
|
||||
fn many_clients_share_one_session_and_agree_about_it() {
|
||||
let daemon = daemon_or_skip!();
|
||||
|
||||
// Held open together, not one after another: the point is concurrency.
|
||||
let mut clients: Vec<Connection> = (0..8)
|
||||
.map(|_| {
|
||||
let mut connection = daemon.connect_tcp().expect("connect");
|
||||
let reply = connection
|
||||
.authenticate(Some(&daemon.token()))
|
||||
.expect("an answer");
|
||||
assert!(reply.contains("\"result\""), "client rejected: {reply}");
|
||||
connection
|
||||
})
|
||||
.collect();
|
||||
|
||||
// Every one of them is told the same thing about the same session.
|
||||
let mut seen = Vec::new();
|
||||
for (i, client) in clients.iter_mut().enumerate() {
|
||||
let reply = client
|
||||
.call(&format!(
|
||||
r#"{{"jsonrpc":"2.0","id":{},"method":"daemon.status"}}"#,
|
||||
100 + i
|
||||
))
|
||||
.unwrap_or_else(|| panic!("client {i} lost its connection"));
|
||||
let status: serde_json::Value =
|
||||
serde_json::from_str(&reply).expect("valid JSON");
|
||||
assert_eq!(
|
||||
status["id"],
|
||||
serde_json::json!(100 + i),
|
||||
"replies must not be crossed between clients: {reply}"
|
||||
);
|
||||
seen.push((
|
||||
status["result"]["username"].clone(),
|
||||
status["result"]["server"].clone(),
|
||||
));
|
||||
}
|
||||
assert!(
|
||||
seen.windows(2).all(|pair| pair[0] == pair[1]),
|
||||
"every client should describe the same session: {seen:?}"
|
||||
);
|
||||
|
||||
// And the daemon counts them all, rather than having quietly dropped some.
|
||||
let reply = clients[0]
|
||||
.call(r#"{"jsonrpc":"2.0","id":200,"method":"daemon.status"}"#)
|
||||
.expect("still connected");
|
||||
let status: serde_json::Value =
|
||||
serde_json::from_str(&reply).expect("valid JSON");
|
||||
assert_eq!(
|
||||
status["result"]["clients"],
|
||||
serde_json::json!(8),
|
||||
"all eight should still be attached: {reply}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Connecting and dropping, over and over, must not degrade the daemon.
|
||||
///
|
||||
/// Every command makes at least one throwaway connection while it works out
|
||||
/// whether a daemon is there, so a per-connection resource that is not
|
||||
/// released shows up as a daemon that works for a while and then refuses
|
||||
/// everyone.
|
||||
#[test]
|
||||
fn rapid_connect_and_drop_does_not_wear_the_daemon_out() {
|
||||
let daemon = daemon_or_skip!();
|
||||
|
||||
// Comfortably more than the connection cap, so a leak of one slot per
|
||||
// connection would have exhausted it well before the end.
|
||||
for _ in 0..200 {
|
||||
drop(daemon.connect_tcp().expect("connect"));
|
||||
}
|
||||
// Half-open ones too: authenticated, then abandoned mid-conversation.
|
||||
for _ in 0..50 {
|
||||
let mut connection = daemon.connect_tcp().expect("connect");
|
||||
connection.authenticate(Some(&daemon.token()));
|
||||
drop(connection);
|
||||
}
|
||||
|
||||
// Give the daemon a moment to notice the hangups, then prove it is well.
|
||||
std::thread::sleep(Duration::from_secs(2));
|
||||
let mut connection = daemon.connect_tcp().expect("connect");
|
||||
let reply = connection
|
||||
.authenticate(Some(&daemon.token()))
|
||||
.expect("the daemon should still be answering");
|
||||
assert!(reply.contains("\"result\""), "{reply}");
|
||||
let status = connection
|
||||
.call(r#"{"jsonrpc":"2.0","id":1,"method":"daemon.status"}"#)
|
||||
.expect("an answer");
|
||||
let parsed: serde_json::Value =
|
||||
serde_json::from_str(&status).expect("valid JSON");
|
||||
assert_eq!(
|
||||
parsed["result"]["clients"],
|
||||
serde_json::json!(1),
|
||||
"abandoned connections must not still be counted: {status}"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user