From 8e033a048096cf7edd11854245be7924ff5ce7cd Mon Sep 17 00:00:00 2001 From: Akirabane Date: Fri, 6 Nov 2026 12:18:06 +0100 Subject: [PATCH] dedicated: no move onto a server that is full or stopping player-expect is refused as "the server is full" once the places the player limit counts and those of the other expected players reach max-players: each expectation holds a place, so two proxies sending players at once cannot fill a server past its limit. As at login, an op who bypasses the limit is never refused, and a renewal holds no second place. Once a stop has begun (/stop from the console, a player or the control endpoint, or a stop signal), player-expect, player-import and player-confirm are refused as "the server is stopping": a player must not arrive on a server that is about to close. A retry after the commit is still answered, and export, release and drop stay open, since they are how players leave. transfer-status says whether the server is stopping. Refs #7484 Co-Authored-By: Claude Opus 5.5 --- .../src/game/dedicated/status.rs | 2 + .../src/game/dedicated/transfer.rs | 12 +- crates/horizon-client/src/lan/arrivals.rs | 147 ++++++++++++++++-- crates/horizon-client/src/lan/import.rs | 6 +- docs/server.md | 27 +++- tools/server_transfer_e2e.py | 18 ++- 6 files changed, 191 insertions(+), 21 deletions(-) diff --git a/crates/horizon-client/src/game/dedicated/status.rs b/crates/horizon-client/src/game/dedicated/status.rs index abdc588f43..f09139d0e1 100644 --- a/crates/horizon-client/src/game/dedicated/status.rs +++ b/crates/horizon-client/src/game/dedicated/status.rs @@ -44,6 +44,8 @@ impl Game<'_> { /// Answers the requests waiting on the control endpoint. pub(super) fn poll_server_control(&mut self, state: &mut ServerState) { while let Some(request) = self.control.as_ref().and_then(|c| c.try_recv()) { + // A `console stop` just before stops this server for the next request too. + self.note_stopping(state.lists.lock().unwrap().stop); let reply = match request.line.trim().split_once(' ') { _ if request.line.trim() == "server-status" => self.server_status(state), // A proxy reading whether it may send players here (transfer.rs). diff --git a/crates/horizon-client/src/game/dedicated/transfer.rs b/crates/horizon-client/src/game/dedicated/transfer.rs index fa9b20bdf7..7a53aa7f62 100644 --- a/crates/horizon-client/src/game/dedicated/transfer.rs +++ b/crates/horizon-client/src/game/dedicated/transfer.rs @@ -8,7 +8,7 @@ //! it sends players here). They run //! between two ticks, on the server thread, so the player is never caught half-way through a //! tick. The rules are `lan/arrivals.rs`'s. All of them are refused while `server.toml`'s -//! `[transfer]` is off. +//! `[transfer]` is off, and those that bring a player in once the server has begun to stop. use crate::game::{Dim, Game, lan::LanHost}; use horizon_client::lan::arrivals; use horizon_protocol::packets::play::{self, Clientbound as C}; @@ -40,6 +40,16 @@ impl Game<'_> { reply } + /// Once a stop has begun (`/stop` from the console, a player or the control endpoint, or a stop + /// signal), the transfers know it before the next request is answered: what would bring a player + /// in is refused from then on, and the ways out stay open (`arrivals::Transfers::stopping`). + pub(super) fn note_stopping(&mut self, stop: bool) { + let stopping = stop || horizon_threads::stop_requests() > 0; + if let Some(transfers) = self.lan.as_mut().and_then(|h| h.hub.transfers.as_mut()) { + transfers.stopping |= stopping; + } + } + /// `transfer-status`: the counts a proxy reads before it sends players here /// (`arrivals::status_request`). A proxy asks it every few seconds, so it writes nothing to /// the log. diff --git a/crates/horizon-client/src/lan/arrivals.rs b/crates/horizon-client/src/lan/arrivals.rs index 046674960b..6ee58fb804 100644 --- a/crates/horizon-client/src/lan/arrivals.rs +++ b/crates/horizon-client/src/lan/arrivals.rs @@ -102,6 +102,10 @@ impl Settings { pub struct Transfers { pub settings: Settings, pub expected: Arc>, + /// The server has begun to stop (`/stop` or a stop signal; the game sets it, and a stop is not + /// taken back): no player is expected, imported or placed any more, but the players here can + /// still be exported and released, which is their way out. + pub stopping: bool, } impl Transfers { @@ -110,7 +114,11 @@ impl Transfers { settings.max_expected, settings.max_expect, ))); - Self { settings, expected } + Self { + settings, + expected, + stopping: false, + } } /// The table, locked. A thread that panicked while holding it left it @@ -219,9 +227,14 @@ impl Expectations { /// How many expectations are still alive at `now`, without sweeping the others. pub fn alive(&self, now: Instant) -> usize { + self.holding(None, now) + } + + /// The places the live expectations of players other than `except` hold at `now`. + pub fn holding(&self, except: Option, now: Instant) -> usize { self.until - .values() - .filter(|deadline| now < **deadline) + .iter() + .filter(|(uuid, deadline)| Some(**uuid) != except && now < **deadline) .count() } @@ -515,15 +528,28 @@ fn error(message: &str) -> serde_json::Value { /// The reply to every `player-*` request on a server with transfers off. pub const OFF: &str = "transfers are off on this server"; +/// The reply to `player-expect` when the players here and those expected take every place. +pub const FULL: &str = "the server is full"; + +/// The reply to `player-expect`, `player-import` and `player-confirm` once the server has begun +/// to stop: a player must not arrive on a server that is about to close. +pub const STOPPING: &str = "the server is stopping"; + /// `player-expect [seconds]`: the player will log in within that time /// (`[transfer]`'s `expect_seconds` by default, `max_expect_seconds` at most). /// The reply says whether an earlier expectation was renewed. The login uses /// the expectation up when it starts; one that no login used is reported when -/// it runs out. +/// it runs out. Each expectation holds a place: once the places the player limit +/// counts and those of the other expected players reach `max-players`, the +/// request is refused as full, unless the player is an op who bypasses the limit +/// (`PlayerList.canPlayerLogin`). It is refused too once the server has begun to stop. pub fn expect_request(hub: &mut Hub, args: &str, now: Instant) -> serde_json::Value { let Some(transfers) = &hub.transfers else { return error(OFF); }; + if transfers.stopping { + return error(STOPPING); + } let mut parts = args.split_whitespace(); let Some(uuid) = parts.next().and_then(parse_uuid) else { return error("player-expect needs the player's unique id"); @@ -541,7 +567,18 @@ pub fn expect_request(hub: &mut Hub, args: &str, now: Instant) -> serde_json::Va if hub.guests.iter().any(|g| g.uuid == uuid) { return error("that player is already connected to this server"); } + let shared = &hub.lan.shared; + let (online, max_players) = (shared.online(), shared.max_players()); + let bypass = shared + .config + .bypasses_limit + .as_ref() + .is_some_and(|b| b(uuid)); let mut table = transfers.table(); + // A renewal holds no second place. + if !bypass && online + table.holding(Some(uuid), now) >= max_players { + return error(FULL); + } match table.expect(uuid, ttl, now) { Ok(renewed) => serde_json::json!({ "ok": true, @@ -572,6 +609,7 @@ pub fn status_request(hub: &Hub, now: Instant) -> serde_json::Value { "online": online, "max_players": max_players, "full": online >= max_players, + "stopping": transfers.stopping, }) } @@ -617,7 +655,8 @@ pub fn drop_request(hub: &mut Hub, args: &str) -> serde_json::Value { /// `player-confirm `: the held player whose record of that hand-over was /// imported, ready to be placed (`Hub::imported_arrival`). The game writes its file, then places /// it (`Hub::confirm_arrival`). Until then `player-drop` removes it without a save or a word to -/// anyone. +/// anyone. Once the server has begun to stop it is refused, though a retry after the commit is +/// still answered. pub fn confirm_args(hub: &Hub, args: &str) -> Result<(u128, u128), String> { let mut parts = args.split_whitespace(); let Some(uuid) = parts.next().and_then(parse_uuid) else { @@ -630,6 +669,9 @@ pub fn confirm_args(hub: &Hub, args: &str) -> Result<(u128, u128), String> { return Err("player-confirm takes a unique id and a hand-over id".into()); } if hub.placed_arrival(uuid, id).is_none() { + if hub.transfers.as_ref().is_some_and(|t| t.stopping) { + return Err(STOPPING.into()); + } hub.imported_arrival(uuid, id)?; } Ok((uuid, id)) @@ -873,10 +915,13 @@ mod hub_tests { use std::time::{Duration, Instant}; fn hub() -> Hub { - let config = NetConfig { + hub_with(NetConfig { announce: false, ..NetConfig::loopback(0, "ArrivalsTest".into(), 4) - }; + }) + } + + fn hub_with(config: NetConfig) -> Hub { let mut sync = data::SyncData::empty(); // The entity types the tracker names: the player, as id 0. sync.builtin.registries.insert( @@ -1332,7 +1377,7 @@ mod hub_tests { assert_eq!( super::status_request(&hub, now), serde_json::json!({"ok": true, "players": 0, "held": 0, "expected": 0, "frozen": 0, - "online": 0, "max_players": 4, "full": false}) + "online": 0, "max_players": 4, "full": false, "stopping": false}) ); let (mut placed, _a) = guest(1, 1, "Placed"); placed.placed = true; @@ -1359,7 +1404,7 @@ mod hub_tests { assert_eq!( super::status_request(&hub, now), serde_json::json!({"ok": true, "players": 2, "held": 1, "expected": 1, "frozen": 1, - "online": 3, "max_players": 4, "full": false}) + "online": 3, "max_players": 4, "full": false, "stopping": false}) ); assert!(hub.lan.shared.try_admit(address, 6, "p").is_none()); let status = super::status_request(&hub, now); @@ -1448,6 +1493,8 @@ mod hub_tests { #[test] fn player_expect_refuses_when_the_table_is_full() { let mut hub = hub(); + // Places for every expectation: it is the table that is full, not the server. + hub.lan.shared.set_max_players(1000); let now = Instant::now(); for n in 0..super::Settings::default().max_expected as u128 { let reply = super::expect_request(&mut hub, &format!("{:032x}", 1000 + n), now); @@ -1460,6 +1507,88 @@ mod hub_tests { ); } + /// Each expectation holds a place: once the logins and the other expected players take every + /// place, a new expectation is refused as a login would be, though not for an op who bypasses + /// the limit. A renewal holds no second place, and one past its time frees its place. + #[test] + fn player_expect_refuses_a_full_server_and_the_expected_hold_their_places() { + const OP: u128 = 0xF; + let mut hub = hub_with(NetConfig { + announce: false, + bypasses_limit: Some(std::sync::Arc::new(|uuid| uuid == OP)), + ..NetConfig::loopback(0, "ArrivalsTest".into(), 2) + }); + let now = Instant::now(); + let expect = |hub: &mut Hub, uuid: u128, at: Instant| { + super::expect_request(hub, &format!("{uuid:032x}"), at) + }; + let address = "127.0.0.1:25565".parse().unwrap(); + assert!(hub.lan.shared.try_admit(address, 1, "p").is_none()); + assert_eq!(expect(&mut hub, 2, now)["ok"], true); + assert_eq!(expect(&mut hub, 2, now)["renewed"], true); + let full = serde_json::json!({ "error": super::FULL }); + assert_eq!(expect(&mut hub, 3, now), full); + assert_eq!(expect(&mut hub, OP, now)["ok"], true); + assert_eq!(table(&hub).len(), 2); + // Player 2's expectation ran out: its place is free again. + let later = now + Duration::from_secs(11); + assert_eq!(expect(&mut hub, 3, later)["ok"], true); + assert_eq!(expect(&mut hub, 4, later), full); + // A login gives a place back when it ends (`Shared::release`). + hub.lan.shared.release(1); + assert_eq!(expect(&mut hub, 4, later)["ok"], true); + } + + /// Once the server has begun to stop, no player is expected, imported or placed any more, and + /// `transfer-status` says so; a player here can still be exported and released, which is its + /// way out, and a held one dropped. + #[test] + fn a_server_that_is_stopping_takes_no_one_in_but_lets_its_players_go() { + let stopping = serde_json::json!({ "error": super::STOPPING }); + let stop = |hub: &mut Hub| hub.transfers.as_mut().unwrap().stopping = true; + let mut hub = hub(); + let now = Instant::now(); + stop(&mut hub); + assert_eq!(super::expect_request(&mut hub, UUID, now), stopping); + assert!(table(&hub).is_empty()); + assert_eq!(super::status_request(&hub, now)["stopping"], true); + // A player here is exported and released as before. + let (mut mover, _rx) = guest(1, UUID_N, "Mover"); + mover.placed = true; + hub.guests.push(mover); + assert!(super::export_record(&mut hub, 0, true, Default::default()).is_ok()); + let (mut hub, _seen, _mine, h) = frozen_with_a_watcher(); + stop(&mut hub); + let reply = super::release_request(&mut hub, &format!("{UUID} {h} moved")); + assert_eq!(reply["released"], "moved"); + // The record of a held player cannot be imported... + let (record, mut hub, _seen, _mine) = held_in_a_second_hub(); + stop(&mut hub); + let error = + import_player(&mut hub, UUID_N, &serde_json::json!({ "record": record })).unwrap_err(); + assert_eq!(error, super::STOPPING); + assert!(hub.guests[1].held && !hub.guests[1].imported); + // ... nor one already imported confirmed, but it can be dropped. + let (mut hub, seen, _mine) = import_without_placing(serde_json::json!({})); + stop(&mut hub); + assert_eq!(confirm(&mut hub, UUID), stopping); + assert!(hub.guests[1].held && !hub.guests[1].placed); + assert_eq!(super::drop_request(&mut hub, UUID)["held"], true); + assert_eq!(ids(&seen), Vec::::new()); + } + + /// After the commit, a retry finds the player placed from that hand-over whatever came since: + /// a stop that began meanwhile does not turn the retry into a refusal. + #[test] + fn a_retry_after_the_commit_is_answered_on_a_server_that_is_stopping() { + let (mut hub, _seen, _mine) = import_without_placing(serde_json::json!({})); + let id = hub.guests[1].arrival_handover.unwrap(); + let args = format!("{UUID} {}", super::handover_text(id)); + assert_eq!(confirm(&mut hub, &args)["placed"], true); + hub.transfers.as_mut().unwrap().stopping = true; + assert_eq!(super::confirm_args(&hub, &args), Ok((UUID_N, id))); + } + /// Dropping a held player disconnects it and says nothing to the world. #[test] fn player_drop_removes_a_held_player_without_a_word_to_anyone() { diff --git a/crates/horizon-client/src/lan/import.rs b/crates/horizon-client/src/lan/import.rs index 0f4040bf94..06a9cfd499 100644 --- a/crates/horizon-client/src/lan/import.rs +++ b/crates/horizon-client/src/lan/import.rs @@ -485,8 +485,12 @@ impl Kept { /// statistics (`Imported::advancements`, `Imported::stats`) and its attachments. /// /// Nothing changes when it refuses: the record is read and checked first, and the -/// guest is touched only once everything is known to be well. +/// guest is touched only once everything is known to be well. It refuses everything once +/// the server has begun to stop. pub fn import_player(hub: &mut Hub, uuid: u128, request: &Value) -> Result { + if hub.transfers.as_ref().is_some_and(|t| t.stopping) { + return Err(super::arrivals::STOPPING.into()); + } let request = read_request(request)?; let (tag, handover) = read_compound(request.record, uuid)?; let Some(i) = hub diff --git a/docs/server.md b/docs/server.md index be592440f7..176f75211f 100644 --- a/docs/server.md +++ b/docs/server.md @@ -181,23 +181,32 @@ dashes in an answer (`123e4567-e89b-12d3-a456-426614174000`). A hand-over id is `transfer-status` is what a proxy reads before it sends players to a server. It names no player and changes nothing: ```json -{"ok": true, "players": 12, "held": 1, "expected": 2, "frozen": 0, "online": 13, "max_players": 20, "full": false} +{"ok": true, "players": 12, "held": 1, "expected": 2, "frozen": 0, "online": 13, "max_players": 20, "full": false, "stopping": false} ``` `players` are the players placed in the world (a player a `moved` release took out is no longer counted), `held` the logins held for a hand-over, `expected` the expectations still alive, and `frozen` the players this server is handing over. `online` is what the player limit counts: the placed and the held players and the logins still joining. `full` is `online` at `max_players` or above: an ordinary login, held or not, is then refused as -`multiplayer.disconnect.server_full`, unless the player is an operator with `bypassesPlayerLimit`. A proxy asks it -often, so it writes nothing to the log. +`multiplayer.disconnect.server_full`, unless the player is an operator with `bypassesPlayerLimit`. `player-expect` +counts the expected players too: it is refused once `online` and `expected` reach `max_players`. `stopping` is true +once the server has begun to stop, and no player is taken in any more. A proxy asks it often, so it writes nothing to +the log. `player-expect [seconds]` tells the target server that the player is about to log in: within `expect_seconds` by default, `max_expect_seconds` at most (a longer time is cut to it), and at most `max_expected` players are expected at once. The reply is `{"ok": true, "renewed": false, "ttl_seconds": 10}`; asking again for the same player renews the time and says `"renewed": true`. It is refused for a missing or -malformed id, a time that is not a whole number of seconds above zero, a player already connected, and a full -table. The player's next login uses the expectation up as it starts, once the bans, the whitelist and the player -limit let it in, so a login still in configuration when the time runs out (a resource pack prompt) keeps it. A +malformed id, a time that is not a whole number of seconds above zero, a player already connected, a full table, +a full server (`{"error": "the server is full"}`) and a server that is stopping (`{"error": "the server is +stopping"}`). Each expectation holds a place until it is used up or runs out: once the places the player limit +counts and those of the other expected players reach `max-players`, a new one is refused as a login would be, so two +proxies sending players at once do not fill the server past its limit. As at login, an operator with +`bypassesPlayerLimit` is never refused as full. The places are held against other expectations only: an ordinary +login (one nobody expected) sees the vanilla limit alone, so it can take a place an expected player was counting on. +That player's login is then refused as `server_full`, and the proxy aborts the hand-over on the source, where the +player goes on as it was. `full` in `transfer-status` is likewise the vanilla condition alone. The player's next +login uses the expectation up as it starts, once the bans, the whitelist and the player limit let it in, so a login still in configuration when the time runs out (a resource pack prompt) keeps it. A login that then fails before it reaches play (its connection drops in configuration, or the client refuses a required pack) gives the expectation back with its own deadline, so the proxy's next attempt within that time is still expected. An expectation no login used is reported in the log when its time runs out. @@ -423,6 +432,12 @@ Every step that is not taken in time is undone by itself: an expectation after ` `max_hold_seconds`, a freeze after `max_freeze_seconds`, each with a warning in the log. A `moved` that comes after the freeze ran out is refused, so the proxy must confirm and release within `max_freeze_seconds` of the export. +**A server that stops.** From the `/stop` command (the console's, a player's or the control endpoint's) or a stop +signal on, `player-expect`, `player-import` and `player-confirm` are refused `{"error": "the server is stopping"}`: a +player must not arrive on a server that is about to close. The proxy drops the held player there and aborts on the +source. A retry after the commit is still answered, and `player-export`, `player-release` and `player-drop` stay +open: they are the way out. + **In the log.** Each `player-*` request writes one line with what became of it: `INFO` and what it did (`player-import Alex (): ok, parts inventory,experience, placed false, announced true, recentered false, hand-over `), or `WARN` and the reason it was refused (`player-export Alex (): refused: that player is diff --git a/tools/server_transfer_e2e.py b/tools/server_transfer_e2e.py index 16228192c0..22f07d5639 100644 --- a/tools/server_transfer_e2e.py +++ b/tools/server_transfer_e2e.py @@ -24,7 +24,8 @@ Steps, in docs/server.md's order ("A hand-over, in order"), each checked: writes no file for it, before or after it stops. `transfer-status` is read on the way: after 2 (B expects one player), after 4 (B holds it, A has it placed and -frozen) and after 7 (the player is on B, A has nobody). +frozen) and after 7 (the player is on B, A has nobody). B runs with `max-players=1`: a second player is refused +as full after 2 (the expectation holds the place) and after 7 (the player takes it). Then the cancels: a hand-over from B back to A is aborted (`player-drop` on A, `player-release ... abort` on B) and the player plays on on B; a late `moved` for it is refused. @@ -254,12 +255,12 @@ class Relay: # ---- an engine and its control socket ------------------------------------------------------------ class Engine: - def __init__(self, bins, resources, folder, out, label): + def __init__(self, bins, resources, folder, out, label, **extra): self.label = label self.folder = folder folder.mkdir(parents=True) self.port = lab.free_port() - props = dict(PROPERTIES, **{'server-port': str(self.port), 'motd': f'transfer e2e {label}'}) + props = dict(PROPERTIES, **extra, **{'server-port': str(self.port), 'motd': f'transfer e2e {label}'}) (folder / 'server.properties').write_text(''.join(f'{k}={v}\n' for k, v in props.items())) (folder / 'eula.txt').write_text('eula=true\n') (folder / 'forwarding.secret').write_bytes(SECRET + b'\n') @@ -426,7 +427,9 @@ def run(bins, resources, scratch, out): relay_log.flush() a = Engine(bins, resources, scratch / 'a', out, 'a') - b = Engine(bins, resources, scratch / 'b', out, 'b') + # One place on B: the player moved there takes it. + b = Engine(bins, resources, scratch / 'b', out, 'b', **{'max-players': '1'}) + stranger = str(offline_uuid('Stranger')) relays, bots = [], [] try: a.ready() @@ -474,6 +477,9 @@ def run(bins, resources, scratch, out): steps.check('2. transfer-status on B counts the player as expected, and B is not full', status.get('expected') == 1 and status.get('players') == 0 and status.get('full') is False, status=status) + reply = b.ask(f'player-expect {stranger}') + steps.check("2. B's one place is the expected player's: a second player-expect is refused as full", + reply == {'error': 'the server is full'}, reply=reply) # 3. A exports it, frozen; a repeat answers the same bytes. reply = a.ask(f'player-export {player_uuid} freeze') @@ -570,6 +576,10 @@ def run(bins, resources, scratch, out): status_b.get('players') == 1 and status_b.get('held') == 0 and status_a.get('players') == 0 and status_a.get('frozen') == 0, a=status_a, b=status_b) + reply = b.ask(f'player-expect {stranger}') + steps.check('7. B is full with the player on it: player-expect is refused, and transfer-status says full', + reply == {'error': 'the server is full'} and status_b.get('full') is True + and status_b.get('stopping') is False, reply=reply) # The cancel: from B back to A, aborted. reply = a.ask(f'player-expect {player_uuid}')