An accepted upload only ever dialled the downloader at the address the
server reported. Most downloaders sit behind a NAT, so the dial timed out
after 20s ("serve …: connection timed out") or, with port 0, the upload
sat in pending_serves forever. There was no fallback: an inbound
PierceFirewall was always treated as a P control connection.
Do what the reference clients do. When an upload starts, ask the server to
have the downloader connect back (ConnectToPeer type F) and dial them at
the same time; the first socket to arrive claims the transfer. Over the
pierced socket only the bare 4-byte transfer token precedes the offset
exchange. CantConnectToPeer and the brokered-connect timeout fail the
upload and free its slot; port 0 skips the dial instead of parking the job.
Covered by an e2e test where the leecher runs without a listener.
Closes#102
A caller that gives up on a login had no way to reach the client: the
thread that owns it sits in login() for up to 45 seconds, and disconnect()
needs the client to itself. Until that thread returned, the abandoned
client kept connecting and still wrote its login frame the moment the
server finished the TCP handshake. When that happened after a replacement
had logged in under the same name, the server kicked the replacement.
Client::cancel_handle() hands out a CancelHandle that can be moved to any
thread. After cancel():
- the server actor writes nothing more. The check and the write share a
lock with cancel(), so once cancel() returns no frame leaves, whether it
was waiting in the queue, in the dispatcher channel, or mid-flush;
- login() returns NotConnected within 100 ms, and hands no login to the
actor at all when the client was already cancelled;
- connect() refuses before binding the listener when the client was
already cancelled. A cancel that lands during that check can still let
the bind through.
Either way the owner drops the client as before, which disconnects it and
gives its port back.
A frame already written before cancel() returned cannot be recalled. The
API is additive, and login() on a client nobody cancels behaves as before.
Soulseek.NET tears the whole connection down when ConnectAsync is
cancelled, and Nicotine+ drops outgoing messages for a socket it closed;
this gives soulseek-rs the same end state from another thread.
disconnect() told the listener thread to stop and dialled it awake, then
returned while that thread was still unwinding, so the TcpListener was
often still open when the caller moved on. A client built straight after
a drop then found its configured port taken and fell back to an ephemeral
one. The TUI login screen does exactly that when it replaces an attempt.
The listener thread now raises a flag once it has dropped its socket, and
after the wake-up dial has landed, disconnect() waits for that flag,
bounded at two seconds, so the port can be bound again by the time
disconnect() or Drop returns. A listener that cannot be woken, or is held
past the bound by a full handshake queue, finishes on its own as before.
The flag is an Arc<AtomicBool>, like the stop flag beside it: keeping the
thread's JoinHandle instead would leave Client neither UnwindSafe nor
RefUnwindSafe, which is a breaking change for the library.
Nicotine+ and Soulseek.NET both close the listen socket as part of
disconnecting; this makes that hold by the time the call returns.
`Client::set_download_speed_limit(bytes_per_second)` puts one ceiling on
every download at once. Zero lifts it and is the default. It can be
changed at any time, and a change reaches transfers already running
within one 100 ms tick.
The limiter works the way Soulseek.NET's does, which slskd uses for its
own download limit: one token bucket shared by all downloads, refilled
to a tenth of a second's worth every 100 ms, handing out partial grants
and taking back what a short read or a timeout did not use. A download
is paced by reading less from the socket. The unread bytes stay in the
kernel's receive buffer and TCP flow control slows the uploader; no
protocol message is involved, as in Nicotine+ too.
With no cap set, each read costs one atomic load. A same-hour stress
run scored develop 88.98/88.98 and this branch 89.00/88.99/88.97, at
about 133 MiB/s both.
An upload ceiling is left out, as before.
Five files ran between 1.2k and 2.4k lines, and one function past the
length the quality gate allows. None of it was doing anything wrong; they
had simply accumulated until you had to scroll to hold one in your head.
The move throughout is into *child* modules rather than siblings, because
a child still sees its parent's private items. So `ClientContext` keeps
its private fields and `ServerActor` keeps its private state, while the
methods that work on them move out — no encapsulation is widened for the
sake of the split. Three `ServerActor` methods become `pub(super)`, which
is only acknowledging the calls the parent already made.
The client's operation loop and the actor's standing-message match are
split by subject: the arms move out whole, so the bodies read exactly as
they did inside. Where an arm said `continue` it now says `return` — the
match was the last thing the loop did, and none of the arms that moved
holds a loop of its own, so skipping the rest of an arm and ending its
handler are the same move.
The test modules split by topic the same way, which is most of the size:
main_tui's alone was 2.1k of the 2.4k lines.
No behaviour changes. Verified against a live server: the full e2e suite
passes, including the 98 CLI and 19 daemon-protocol cases that drive the
dispatch loop this touches.
The window could show any peer's share tree but never this account's, so
"am I actually sharing that?" had no answer short of asking a friend to
browse you. The tree is already there; what was missing was feeding it
from our own index.
B opens it — the same collapsible tree, filter, and navigation the peer
browse offers, built from `Shares::directories()`, which is the listing
GetShareFileList answers with. So what it shows is what the network sees,
not what the config meant to share. r re-scans the disk rather than
re-asking a peer, o steps across to the folders that feed it, and nothing
here queues a download from ourselves.
Attached to a daemon the index lives on the daemon's host, so it comes
back over a new `shares.files` method rather than being scanned locally;
`shares files [--filter]` prints the same listing headless, through the
`emit_listing` that `browse` now shares with it — the two are only
comparable while they are built the same way.
Two allocation fixes came out of the review, both on paths this reaches:
the daemon hands its listing to the DTO instead of deep-copying one it
already owns, and `Shares::directories()` allocates its map key once per
folder rather than once per file, which the peer-facing serve path has
been paying for all along.
Correctness:
- A child that stopped reading could freeze the whole client: the relay
writes under the context lock and the socket was blocking, so one stalled
peer took every download, upload and API call with it. Child writes now
have a deadline, and a child that hits it loses its slot.
- Relaying searches to children was gated by the leaf's answer budget, so a
parent forwarded a fraction of the stream while advertising itself as
healthy. The budget bounds our own share scan, which is what it was for;
the tree's stream goes down whole.
- A dropped client kept its listener port for the life of the process, so a
retried login advertised an ephemeral port nobody was told about. The
listener is now stopped and the operations thread let go.
- Excluded phrases were compared against a lowercased path without being
lowercased themselves, so a capitalised phrase never matched. They are
normalised where they arrive.
- Losing our own operatorship wiped the room's whole operator roster,
though we are still a member who can see it.
- A brokered ConnectToPeer of type D was logged and dropped; the peer is
now told, since we never dial out to adopt a child.
- A peer request made with no server connection returned Ok and parked
forever; it says NotConnected, as its docs always claimed.
- An empty interest was refused locally but still sent, leaving the
server's list and ours permanently apart.
Simplification:
- Deleted the excluded-phrase helper nothing called, and the field doc that
promised behaviour it never had.
- The four roster handlers and four standing handlers were eight
constructors carrying constants their own message code already implies:
one newtype each, 100 lines less.
- One place registers a search; the phrase list is borrowed, not cloned per
relayed search.
The speed-derived child limit read a missing ratio as a refusal, so on any
server that never announces ParentSpeedRatio — soulfind among them — the
client carried no children at all, however it was configured. A ratio we
were never told is not a decision: the server's figures set the limit only
when it has given them, and otherwise the configured cap stands, which is
how slskd runs throughout. A speed below the announced minimum still means
none, which is the safety property the rule exists for.
Also stops repeating AcceptChildren: it is a standing state on the server,
so it goes out when the answer changes rather than on every message that
happens to prompt a recount.
The client answered a UserInfoRequest but could never send one, so a
user's profile — description, upload slots, queue length — was something
this client served and never saw. The reply (peer code 16) is now parsed
too, with the tolerances other clients carry: a picture is skipped rather
than kept, a length that overruns the message is clamped, and a message
that stops early (older clients, and Museek+ writing a boolean as an
integer) still yields every field before it.
The child limit is derived from the upload speed the server records for
us, and the server pushes a user's stats to whoever watches that user —
so a client that does not watch itself never learns its own figure and
would carry the default cap forever. Nicotine+ reads the same number off
the stats the server sends about the logged-in user.
We now watch ourselves as each session starts, and the stats that come
back — the initial reply and every later push — set the figure the limit
is derived from. Client::own_average_speed exposes it.
The server holds interests only for the session that set them, so a client
that does not re-send its list comes back from a reconnect with none. That
is why Nicotine+ sends its whole list at every login, and why it lowercases
each item before sending — the server matches them case-insensitively, so
two spellings would otherwise be held as two interests.
Ours now keeps the list, in one spelling, and puts it back on the wire when
a session starts; an interest set before there is a connection is kept
rather than dropped. A room's ticker board is also let go when we leave the
room, since the server sends the whole board again on the next join.
Reading Nicotine+ and slskd against what was just written turned up three
places where this client had the message right and the behaviour wrong.
Excluded search phrases (code 160) are not a filter on the searches we
send — the server already drops those. Nicotine+ applies them to the files
it offers, leaving out any whose path carries an excluded phrase, and a
reply with nothing left is not sent. Ours now does the same, and the
local refusal of outgoing searches is gone.
Serving children now follows the same rules as Nicotine+: the limit comes
from the server's ParentMinSpeed and ParentSpeedRatio against the upload
speed the server records for us (capped at ten, none at all below the
minimum), children are refused until something feeds us the search stream
and let go when it stops, a second link from a child we already carry is
refused, and AcceptChildren is re-sent whenever that answer changes. Where
the server has never reported our speed there is nothing to apply the
formula to, so the configured cap stands, which is how slskd runs.
Queued downloads now ask where they sit every five minutes, as Nicotine+
does, instead of only when a caller asks; a brokered file connection we
cannot complete reports CantConnectToPeer like a control connection does;
and a peer's shares can be asked for one folder at a time (peer code 36
and its reply, 37), which is what "download folder" is built on.
The client read the distributed network but never carried it: a peer
dialling with a D connection was logged and dropped, and AcceptChildren
was hardcoded false, so this client took from the tree without ever
relaying a search for anyone else.
With accept_children set, peers hang from us: each is told where our
branch sits when it connects and again when the branch moves, every
search that reaches us is passed down, and a child that goes away loses
its slot. Ten at a time, matching Nicotine+, since each costs a socket
and a copy of the network's whole search stream — which is also why it is
opt-in, through the new accept_children config key (ignored without a
listener, since a child would have nothing to dial).
Two bugs an e2e test found the moment one existed for this path.
A private message the server held while we were offline arrives with
new_message false, and the handler only acknowledged the ones flagged
new — so exactly the messages that need an ack never got one and the
server re-delivered them at every login, forever. Every delivery is now
acknowledged.
A dropped Client also left its server connection open, so the session
lived on: mail addressed to us went to a socket nobody was reading and a
later login collided with our own ghost. Client::disconnect closes the
server and peer connections, and Drop calls it.
Also covers two features that were never driven end to end: an offline
message delivered once and not again, and a wishlist search answered like
any other.
Private rooms were unreachable: the client could ask to join one but had
no way to invite, promote, resign or be told what happened, and a room
name already owned by someone else came back as silence rather than a
refusal (code 1003).
Adds the rosters the server pushes after login (133/148), the changes it
narrates to them (134/135, 143/144), our own standing (139/140, 145/146),
and the messages that drive them, with the rosters kept current in client
state. The daemon protocol and the TUI room transcript carry the new
events rather than dropping them.
Three features the client half-had. SetStatus (28) was only ever sent as
"online" at login, so there was no way to go away; excluded search phrases
(160) were parsed and logged into nowhere, so every client kept spending
searches on queries the server was never going to answer; and a queued
download could answer a peer asking about its place but could never ask
about its own.
Each is now driven end to end: the away status seen by another user, a
refused query on a stub server that announces phrases (soulfind keeps its
filters in a database the suite cannot seed), and a peer's answer landing
on the download as its queue position.
Adds the server messages the client had no way to speak: user and room
search (42/120), room tickers (113-116), the global room feed (150-152),
interests and the recommendation queries built on them (51/52/54/56/57/
110/111/112/117/118), a keepalive ping (32), gifted privileges (123), a
password change (142) and multi-user private messages (149).
CantConnectToPeer (1001) closes a real gap in both directions: a dial the
server asked us to make now answers the waiting peer instead of falling
back to brokering the same unreachable link the other way, and an incoming
one ends the wait on downloads queued for that peer rather than letting
them sit until a timeout.
A candidate that accepted the dial and then said nothing kept its slot
for the session, and ten of those blocked every later batch; a link now
closes after a minute of silence. A parent that flaps its level made us
tell the server four messages per flap; moves are now told at most once
a second, the latest one winning. A parent that reports level zero is
its own root from then on, whatever it said before. The wait for a
searcher's control connection runs from the last message queued for it,
not the first. A successful login is pinned to reset the leaf.
Review of the leaf found the seams a stranger on the other end of a D
connection could pull at, and a few that a slow one would.
- A code-93 envelope is unwrapped once, as Nicotine+ does; a frame made
of nothing but envelopes used to recurse once per byte.
- A branch level is validated before it is used: negative levels leave
the candidate unadoptable and the largest one no longer overflows.
- The link reads through the crate's MessageReader, so a frame split
across a read timeout is no longer torn or mistaken for a closed link.
- Every event a link raises carries its link number, so a stale thread
cannot adopt, close or move a newer link to the same user.
- Later branch facts from the parent move our own place and are told to
the server; a fresh login resets the leaf, whose stance is now the one
place the four announcement messages come from.
- Candidates from a later batch join the ones in flight instead of
cancelling them, capped at ten; searches from the parent are budgeted
at fifty a second; a reply queued for a searcher the server cannot
place expires after a minute.
- Truncated frames and half-read PossibleParents entries are dropped
instead of yielding an empty root, a level of zero or a phantom
candidate.
The e2e now has a searcher of its own, so the answer must go to whoever
asked and not to the parent that relayed it.
The server hands every client a list of possible parents and expects it to
hang from one so searches also travel peer to peer. Until now we told the
server we were a lone root and left it at that, which the network tolerates
but does not favour.
A leaf now dials each PossibleParents candidate over a D connection, adopts
the first that passes a search down after stating its branch, announces the
new branch root and level to the server, and answers those searches the same
way as ones from the server. Losing the parent or a ResetDistributed puts it
back to a parentless stance. The leaf takes no children.
The Leaf state machine is pure so its rules are unit-tested without sockets;
the e2e test drives a real client from a mock parent, since soulfind never
hands out parents.
An upload offer counted against the slot cap until the peer accepted
it, and a peer that never answered, having gone away or hung, held that
slot for the life of the process; with two slots by default, two such
peers shut uploads down until a restart. An offer nobody answers within
thirty seconds is now dropped, recorded as a failed upload so the
operator sees why, and the slot goes to the next in line.
Opening a user's profile in SoulseekQt or Nicotine+ asks that peer
directly (peer code 15), and a peer that never answers shows an endless
spinner. The reply (code 16) carries no description or picture, the
configured upload slot count, how many uploads are waiting, and whether a
slot is free right now, which is what the reference clients send.
"Download folder" in SoulseekQt and Nicotine+ asks the sharer for that
one folder (peer code 36) instead of the whole share, and a sharer that
never answers leaves the folder download hanging. The requested folder
now comes back as a FolderContentsResponse (code 37), which carries a
directory listing in the same form a browse does, so the two share one
codec. The browse and folder replies also share one reply helper.
Every reply to a peer's search claimed a free upload slot, no speed and
an empty queue, whatever the sharer was doing, and searchers rank
sources on exactly those fields. A reply now says whether a slot is
free, the bytes per second of the last completed upload, and how many
uploads are waiting.
A search token was the first five hex digits of the query's MD5, so two
queries could share one, and a peer's answer to either landed in whichever
search the map handed back first. Tokens now come from a counter, which
no two live searches can share.
The e2e test uses two queries that collide under the old scheme.
A broad query against a big share was answered with every match, so one
search could hand the whole index to a stranger, and every such search
did it again. A reply now carries at most MAX_SEARCH_REPLY_FILES (150,
Nicotine+'s default), which is more sources than any searcher scrolls
through.
Adds the server-side watch subscription a buddy list needs: WatchUser (5)
asks the server to keep pushing a user's status, UnwatchUser (6) cancels it.
The reply to a watch carries the user's current status and share statistics,
which merge into the existing per-user snapshot read via Client::user_info.
Issue #47 asked for AddUserList (141) / WatchedUserAdded (142) /
WatchedUserRemoved (143). Those codes are EnableRoomInvitations,
ChangePassword and AddRoomOperator in the Soulseek protocol -- there are no
such messages on the wire. Watching is codes 5 and 6, so that is what this
implements.
- message/server/watch_user.rs: WatchUserHandler (code 5), tolerant of an
absent user and of a truncated frame
- MessageFactory::build_watch_user / build_unwatch_user
- Client::watch_user / unwatch_user / watched_users
- The watch reply has no privileged flag, so it preserves whatever an
earlier GetUserStatus recorded rather than asserting false
The JoinRoom (code 14) reply carries, after the membership list, four
parallel vectors describing each member: status, share statistics, free
upload slots and country. The handler stopped after the usernames, so all of
it was discarded. It is now parsed and exposed as Client::room_member_stats.
Issue #48 asked for RoomStats (57) and RoomListStats (55). Those codes are
UserInterests and MyRecommendations in the Soulseek protocol; there is no
RoomStats message on the wire. Per-room user counts already arrive with
RoomList (64), which the client handles, and the per-member statistics are
part of the JoinRoom reply -- so that is where this adds them.
- join_room.rs: parse the stat vectors, each bounded by the payload so a
hostile count cannot hang or overallocate
- types::RoomUserStats; every field looked up positionally so a server
sending short vectors (or omitting countries) does not shift the roster
- ServerMessage/ClientOperation::RoomMemberStats and the context snapshot
behind Client::room_member_stats, replaced on each rejoin
Document the minor-version registry on ClientVersion: within the
soulseek-rs major (176), minor 1 stays the library default for
unidentified consumers and minor 2 is now the reference CLI/TUI client.
Downstream clients reserve their own minor via issue #12.
Review of the capacity work surfaced three real defects and two gaps:
- A peer dribbling bytes re-armed the 10s handshake read timeout on
every read and held its accept permit forever; 128 such sockets
wedged the accept loop for good. The handshake now has one deadline
across all reads, plus a 4 KiB cap on buffered handshake bytes.
- A capacity refusal was logged and swallowed, leaving the refused
peer's queued downloads stuck until the caller's own timeout. Both
registration paths now fail those downloads so the caller gets a
verdict, and the listener only announces PeerConnected when the
registration actually happened.
- The transfer-claim was undone by the worker thread writing Queued
back over it; that write is gone, so a replayed TransferResponse
can no longer start a second writer on the same .part file.
- Offered uploads and queue waiters joined the eviction-protected
set, and the browse protection window matches the minutes a real
share listing takes rather than one.
Regression tests: the broker-timeout sweep failing a queued download
end to end, the replay guard observed over a real socket, and the
handshake deadline against a dribbling and an oversized peer.
One popular search produced a ConnectToPeer for every responder, and
each one cost a 2 MiB wrapper thread, an unbounded 256 KiB actor, a
listener thread per inbound socket, and on failure a 2 MiB thread that
slept for 20 seconds. A container with a pids limit died on one album
search (issues #32, #33).
Four bounds, no behavior change for a quiet session:
- P-type registrations run inline on the ops loop; only F transfers
keep a worker thread. The dial already ran on the actor thread.
- The broker timeout is a deadline swept by the ops loop instead of a
sleeping thread per failed dial.
- The registry caps live actors (default 512, Client::set_max_peers).
At capacity the oldest peer outside the caller's protected set
(live downloads, uploads, pending serves, queued messages, browses
in flight) is evicted; if none qualify the registration is refused
before any thread is spawned.
- Concurrent listener handshakes are bounded by a 128-permit
semaphore released once the peer-init arrives, so slow handshakes
cannot multiply threads while transfers stay unthrottled.
A replayed TransferResponse for an InProgress or Completed token is
dropped instead of starting a second transfer thread over the same
.part file.
Fixes#32Fixes#33
The peer listener died with "Too many open files (os error 24)" on any
long-lived non-daemon session: only the daemon raised RLIMIT_NOFILE, and
peer sockets were unbounded — no idle reaping, no deadlines on accepted
transfer streams, and one peer could hold two connections under split
registry identities.
- raise the open-file limit at the top of run(), for every entry point,
and warn from Client::connect when the soft limit is below 1024
- reap peer control connections idle for five minutes; a clean
disconnect no longer discards the peer's queued uploads
- give listener-supplied download streams and upload serving the same
read/write deadlines dialled sockets already had
- register inbound peers under their plain username so the registry
holds one connection per peer, and announce them like pierced ones
- hold the registry lock across actor spawn + insert and surface
thread-spawn failure, so an instantly-dead peer cannot leave a
zombie entry
Seven region audits, every finding adversarially verified against the
whole workspace before being applied; 80 landed and the two that failed
in application were refuted honestly (ParseError has a from_str_radix
producer the audit missed; NatPmpError's Display is the only reader of
its diagnostic payloads).
The bigger cuts:
- The NewPeer/PierceFireWall cluster: an operation variant nothing ever
sent, its 77-line arm, a protocol-wrong Client::pierce_firewall that
wrote a peer-init frame down the server socket, and the struct only
they used. The live paths in peer/listen.rs are untouched.
- The ops loop reads `for operation in reader` instead of hand-rolling
recv/break, de-indenting the whole dispatch body.
- Dead lib surface: Message::print_hex and four unused accessors,
UploadFailed, registry and actor getters, build_watch_user,
strip_ansi_codes, delegating shims on ClientContext. Removing public
items makes this a semver major, which the release pipeline enforces.
- Never-used flexibility: the state-migration framework that never held
a migration, border-type helpers that all answered Rounded, the TUI's
max_concurrent_downloads field nothing read.
- Convergence: six poll loops into poll_until, three wrap-around
navigation blocks into cycle, hit tests into Rect::contains, char
walkers into floor/ceil_char_boundary, five Download literals into
one helper, the suites' shared scaffolding into tests/common.
Deliberately not taken: swapping the hand-rolled zlib inflate and MD5
for crates (they replace wire-facing machinery whose tolerance of
real-world peer quirks no local test exercises) and pulling chrono into
the lib for a log timestamp. Recorded for their own decision.
Net -1002 lines. Both e2e modes stay green: 94 CLI tests each way, 16
protocol, 33 lib, unit suites, clippy pedantic clean, aislop 100/100
with the four scanner false positives suppressed inline where they
fire.
`serve` samples `uploads()` every 500ms and emits what changed, so an upload
state shorter than that window never reached the log. Nothing surfaced this
while every upload sat on its slot for a 500ms linger, which was exactly one
poll; with the linger gone a peer can queue and be served between two samples,
and "it had to wait" — the one thing the queue exists to record — silently
disappeared.
The pump now keeps whatever it left waiting, and `serve` drains that alongside
the snapshot, so the fact survives regardless of when the next sample lands.
The list is deduplicated per peer and file and capped, so a caller that never
drains cannot grow it without bound.
This is what made cli_e2e::serve_reports_a_queued_upload_once_the_slots_are_full
fail on CI while passing locally: the assertion was real, and it was timing that
decided whether the client could answer it.
The slot cap merged from master defaults to 2 uploads, and the CLI capped
downloads at 5. Those are the numbers you pick when your own bandwidth is the
scarce resource, but a Soulseek transfer is paced by the peer on the other end
— usually a few hundred KiB/s — so what fills a modern link is many transfers
at once, not faster ones. Upload slots go to 10, downloads to 20. The queue
itself stays: privileged users being able to jump it is a condition of being a
tolerated third-party client.
serve_file also slept half a second after the last byte so the downloader could
drain before the socket closed. With a cap that is time no other peer can use,
and 64 uploads over 10 slots paid it six times over. A half-close says "that is
the whole file" and TCP still delivers everything written before the FIN.
Measured with 64 waiting peers: 2 slots 24.2s, 8 slots 6.2s, 32 slots 3.1s; at
10 slots the linger alone was 5.4s to 3.05s.
The client had never been driven at scale. A stress harness that mixes
searches, downloads, uploads and browses against a local soulfind found four
ceilings, three of which lose work silently rather than merely being slow.
Peers were capped at one per core. ActorSystem ran each actor's whole lifetime
as one job on a fixed-size ThreadPool, and actor loops block until stopped, so
the pool acted as a semaphore: past the cap an actor never started, never
connected and never errored, and uploads stopped working entirely. The pool had
exactly one consumer and every job it received was non-terminating, so it is
gone; actors get their own thread.
TransferResponse raced the bookkeeping it depends on. It was sent straight back
to the peer while UpdateDownloadTokens was still queued for the client loop —
but the response is what invites the peer's file connection, and that connection
is matched by token. Under load the client loop lost, the file connection
matched nothing, and the download sat on Queued forever. The response now goes
out after the token is recorded.
Download tokens were md5(filename) truncated to 20 bits, and the store deletes
every entry matching a token, so two downloads sharing one destroyed each other
— which fetching the same filename from two peers does every time. They now
come from a counter.
Port 0 from GetPeerAddress means the server does not know where a user listens.
It was cached and dialled, and the upload waiting on it was dropped. It is no
longer cached or served against; the connect attempt stays, because its failure
is what makes an unreachable peer fall back to server brokering.
Mixed profile at 1024 peers: 512/512 downloads, 256/256 uploads, 400-850 MiB/s
in ~7s, against 71/96 downloads at 7.9 MiB/s and a 60s timeout before.
client/mod.rs had grown to 1124 lines, a third of it tests: six loose #[test]
functions wedged between the two `impl ClientContext` blocks, plus the usual
`mod tests` at the bottom. Both now live in client/tests.rs, leaving mod.rs at
790 lines of actual client.
Same 11 tests, same 568 passing across the workspace.
Running several one-shot commands at once returned "no results" for queries that
had plenty. Four defects, each of which made a dead session look like an empty
network:
- The peer listener bound its port inside a detached thread and panicked on
EADDRINUSE, unwinding that thread alone. The process carried on with no
listener, so no peer could deliver a search response. `Listen::bind` now takes
the socket before login and falls back to an ephemeral port when the
configured one is held.
- SetWaitPort advertised the configured port whether or not it was bound, so
peers dialled a sibling process that discarded responses to tickets it never
issued. The port the listener really holds is what gets advertised, mapped
through UPnP, and reported by `whoami` and `serve`.
- A zero-byte read is EOF on a TCP socket, but MessageReader reported it as
success, so a server that closed cleanly left the actor polling a dead socket.
It is now an error, and Relogged (code 41) has a handler. The peer actor
classifies that error: a control connection that was established and then
closed is a clean disconnect, because peers drop idle sockets while the bytes
move over their own connection, and reporting an error there fails every
download queued with that peer including the one streaming. A connection that
never established keeps the error path, which is what the server-brokered
fallback reads to know a peer is unreachable.
- A session the server cut off - usually because another process logged in under
the same name - exited 4, documented as "it worked and found nothing". Callers
concluded the file was unavailable. Sessions now record why they ended and
those verdicts become exit 7.
The server allows one session per account, so concurrency still needs a distinct
--username per run; SKILL.md documents that. The listener port needs no such
care any more.
Six e2e tests against soulfind cover it, each of which fails without the fix: a
taken listen port, a displaced session, three clients searching at once under
one configured port, the two CLI equivalents, and a peer that hangs up its
control connection while a transfer is still to come.
BREAKING CHANGE: `Listen::start` is replaced by `Listen::bind` and
`Listen::serve`, so the caller owns the socket and can read the port it actually
bound. `Client::listen_port` and `Client::session_loss` are new.
Closes the last two gaps in Soulseek's list of features an alternative client
must implement to be tolerated, and adds the result filters Nicotine+ has had
all along.
Wishlist: server code 103 is sent and code 104's announced interval is kept
rather than logged and discarded, so the re-search timer runs off the schedule
the server set. `wish add|remove|list|run` plus `serve --wishlist`; wishes
persist in config.toml as a TOML array so a comma inside a query survives.
`wish run` prints search-shaped records, so it pipes into `download --stdin`.
Privilege recognition: the code-69 user list is parsed instead of counted and
dropped, and code 92 answers `whoami` as `privilege_seconds`. Uploads now go
through a queue with a slot cap (`--upload-slots`, default 2) that serves
privileged users first — the cap is what makes the ordering mean anything, so
they are one change. `PlaceInQueueRequest` (peer code 51) is answered, where it
previously had no handler at all, and a departed peer's slot is released so a
stalled peer cannot wedge uploads shut.
Search filters: `--exclude`, `--extension`, `--min-size`, `--max-size` beside
the existing `--min-bitrate` and `--free-slots`, in one flattened arg group
shared by `search`, `get` and `wish run`.
Also serialises the library e2e suite behind the same gate `cli_e2e` already
uses: the new queue tests hold peer sockets open on purpose, which starved
concurrently-spawned soulfinds and failed unrelated download tests with
`login: Timeout`.
BREAKING CHANGE: `UploadStatus` gains a `Queued(u32)` variant, so an exhaustive
match on it no longer compiles, and `cli::Commands` gains `Wish`.
The Login message hardcoded SoulseekQt's major version 157, so the
server could not tell soulseek-rs apart from the official client. The
protocol document now reserves a unique major version per project, so
claim 176 and send it via a new ClientVersion on ClientSettings.
Clients and bots built on the library keep major 176 and pick their own
minor version (default 1); the reference CLI/TUI logs in with minor 100.
Refs #12
An agent driving the binary could search, fetch and chat, but it could not be
a peer: nothing kept a session online to serve files, nothing reported an
upload, shares could only be set at startup, and there was no way to ask
whether a peer was worth queueing to or to check credentials without doing
something else.
New commands:
serve [--follow] stay online sharing; one record per upload
whoami credentials, connection and what we offer
user <NAME> a peer's presence and share counts
room users <ROOM> the member roster
shares list|add|remove|status|reindex
config path|list|get|set
`config` and `shares list|add|remove` need no account, so they run before the
credential check — asking a script to log in for a `config get` would be a
poor trade. `serve` is the only place uploads are visible: they live in the
process serving them, so a one-shot `uploads list` could print nothing but an
empty list.
Library additions, all request/poll pairs matching the existing conventions:
- Client::room_members, kept current from the join and leave events
- Client::request_user_info / user_info, backed by new handlers for
GetUserStatus (7) and GetUserStats (36) — codes the dispatcher previously
logged as unhandled. The wire format is taken from soulfind's own source
and pinned by an e2e test that asserts a peer's real share count.
- UserInfo carries presence and stats as Options, so a reply that never
arrived cannot be read as "offline" or "shares nothing"
- request_user_info clears the previous answer, so the documented poll loop
observes this request rather than the last one
- ServerMessage and ClientOperation are now #[non_exhaustive]: every protocol
message the client learns adds a variant, and that should not break callers
- the crate root and prelude re-export every type a Client method returns
Also pins the output contract with tests: under --json, stdout carries only
newline-delimited JSON across fifteen command shapes while progress still
goes to stderr, and a failing command writes nothing to stdout at all.
Peer layer:
- Stream peer messages with a partial-write queue instead of write_all, so a
large SharedFileListResponse over the non-blocking socket is no longer
truncated (third-party clients like SoulseekQt can now browse our shares).
- Bound the peer-init handshake with a read timeout and handle each accepted
connection on its own thread, so one stalled peer can't wedge the listener.
- e2e: third-party direct/brokered/large-share browse + stalled-peer coverage.
TUI:
- Uploads share the downloads table columns (real Speed) and the info pane now
renders details for a selected upload.
- Download progress bar counts only in-progress transfers and hides when idle.
- Always-visible unread badge (private messages + rooms) in the shortcuts bar,
self-drawn blink so it works under tmux.
- Persist the unread private-message count across restarts.
Press 'o' for a settings popup: edit the download folder, add/remove
share paths (applied live via a rescan and persisted to config.toml),
and force a re-index with 'r' for files changed on disk. The state
machine is unit-tested; feedback shows the shared file/folder counts.
The downloads pane is now Downloads/Uploads: uploads being served are
listed after downloads with an up-arrow marker, live progress, and
status; 'x' cancels an in-progress upload. Client::shared_counts exposes
the index size for the status line.
serve_file streams in chunks, updating a shared byte counter and
aborting with Interrupted when a cancel flag is set. Each served upload
is registered (peer, virtual path, size, live progress, status) and
exposed via Client::uploads(); Client::cancel_upload flags a matching
in-progress transfer. e2e: the download test now also asserts the
uploader recorded a completed upload with the right peer and byte
count.
Client::set_shared_directories rescans the given roots into a fresh
in-memory index, swaps it for peers, and re-announces the folder/file
counts to the server. The current directory list moves into the client
context so the running set is queryable. e2e-verified: a browser sees a
directory that was added after login.
Shares::scan_many merges several roots into one index, suffixing
duplicate root names so virtual paths cannot collide, and skipping
unreadable roots instead of failing. ClientSettings now takes
shared_directories: Vec<String>; config gains a shared_dirs list
(combined with the single shared_dir and --shared-dir, deduped), with an
explicitly empty value still disabling sharing and the
share-what-you-download default unchanged.
- remove 34 trivial restating comments (auto-fix)
- replace all 20 production .unwrap() calls with graceful handling:
?-propagation via LockPoisoned, unwrap_or_default on infallible
conversions, let _ = on actor-channel sends, and a downloads_sender()
helper in the TUI that also dedupes three channel-init blocks
- collapse nested if into match guard (clippy::collapsible_match)
- split client.rs (2275 lines) into a client/ module: connection,
search, downloads, uploads, rooms, operations
- split ui/main_tui.rs (2027 lines) into a main_tui/ module: render,
input, browse, rooms, downloads, search
- extract oversized functions: peer_actor/server_actor handle_message
into per-message helpers, main() into init_logging/run_default_tui,
download_selector and main_tui render into per-section helpers
Mechanical only, zero behavior change. Verified: clippy clean
(pedantic+nursery), fmt clean, 143 unit tests + 16 e2e tests against
soulfind pass, lint clean.
Michel de Graaf