Enhance school management and protocol handling by introducing a new worker failure mechanism, ensuring proper cleanup of schools when a worker thread encounters an error. Update the map snapshot protocol to allow larger school data sizes, improving the handling of extensive map layouts. Revise documentation to reflect changes in save intervals and introduce a minimum save interval for better performance. Update tests to validate the new functionalities and ensure robustness in handling large data scenarios.
ci / server (push) Failing after 3m29s
ci / client (push) Successful in 16s

This commit is contained in:
Leonid Pershin
2026-08-18 17:02:28 +03:00
parent 6ca9ff9d03
commit f000b128f6
14 changed files with 325 additions and 74 deletions
+9
View File
@@ -142,6 +142,15 @@ say so explicitly in the change description.
runs six times over once six schools exist. `OpenSchool` only subscribes to clock frames.
- The menu polls `GET /api/schools` once a second and patches its cards in place. Rebuilding the
list on every refresh would drop focus and swallow clicks.
- **An abandoned `WaitToReadAsync` stays queued on its channel.** `GameClient.RunSendLoopAsync`
keeps its two wait tasks across iterations for that reason; recreating both on every idle pass
leaked the loser of `Task.WhenAny` once per clock frame.
- **A map snapshot is not bounded by `MaxMessageSize`.** That constant limits what the server
reads. Outbound snapshots are sized with `ProtocolCodec.MapSnapshotSize`, because a school the
player enlarged in the create editor passes 8 KiB at roughly sixty furnished rooms.
- **A worker that dies must tell the supervisor** (`GameCommand.WorkerFailed`). It owns the table,
so a school that removed itself would break invariant 3 — and a school that removes nothing
leaves a card in the menu whose clock never moves again.
- The client outbox drops the oldest frame under pressure. That is correct for clock frames and
wrong for anything that must arrive exactly once — such a message would need its own path.
- `erasableSyntaxOnly` is off in `tsconfig.app.json` on purpose: constructor parameter properties
+1
View File
@@ -118,6 +118,7 @@ Simulation tunables live under the `Simulation` section of
| `SavesDirectory` | `saves` | per-school JSON files |
| `ModsDirectory` | `mods` | pack folders; `core` is required |
| `SaveIntervalSeconds` | 30 | rare clock snapshot; not every tick |
| `MinSaveIntervalMilliseconds` | 1000 | shortest gap between saves caused by pause or speed |
## What is deliberately missing
+5 -2
View File
@@ -97,8 +97,11 @@ cannot land between a mouse-down and a click.
## Saves
Each school is a JSON file under `Simulation:SavesDirectory` (`saves/{id}.json` plus `index.json`
for the next id). The worker writes on create, pause, speed change, shutdown, and on a rare clock
snapshot (`SaveIntervalSeconds`, 30 by default) — never on every tick. The file also stores the
for the next id). The worker writes on create, on shutdown, and on a rare clock snapshot
(`SaveIntervalSeconds`, 30 by default) — never on every tick. Pause and speed changes are written
too, but coalesced to at most one write per `MinSaveIntervalMilliseconds`: a client can send those
as fast as the socket allows, and each one is a file write on the school's own thread. Shutdown
always flushes, so a pause is never lost. The file also stores the
mod pack ids and the map layout; the catalog is loaded again from `mods/` on start. A missing
mod folder or a map that no longer validates leaves the file in place and that school unstarted.
+3 -1
View File
@@ -248,7 +248,9 @@ Each node:
## Guarantees and limits
- Frames larger than 8 KiB are refused with close status `1009 MessageTooBig`.
- **Inbound** frames larger than 8 KiB are refused with close status `1009 MessageTooBig`. That
limit is about what the server reads; it does not bound what the server sends. A `MapSnapshot`
of a large school legitimately exceeds it, and the server sizes that frame from the message.
- A malformed frame closes the connection with `1007 InvalidPayloadData`.
- Unknown message ids are ignored rather than fatal, so new ids can be added without breaking
older clients within the same protocol version.
+2 -2
View File
@@ -94,13 +94,13 @@ export class GameConnection {
this.connect();
}
/** Starts watching a school; its calendar starts running server-side. */
/** Starts watching a school. Its calendar was already running — opening only subscribes. */
openSchool(schoolId: number): void {
this.openSchoolId = schoolId;
this.send(encodeOpenSchool(schoolId));
}
/** Back to the menu; the school stops ticking. */
/** Back to the menu. Clock frames stop; the school keeps running until the player pauses it. */
closeSchool(): void {
if (this.openSchoolId === null) {
return;
+37
View File
@@ -1,3 +1,5 @@
using System.Text;
namespace HSchool.Protocol;
/// <summary>
@@ -99,6 +101,38 @@ public static class ProtocolCodec
return writer.Position;
}
/// <summary>
/// Bytes <see cref="WriteMapSnapshot"/> needs for this message.
///
/// A school's map has no fixed size — the create editor lets a player add rooms — so a
/// snapshot can outgrow <see cref="ProtocolConstants.MaxMessageSize"/>. That limit guards
/// what the server *reads*; callers size an outbound snapshot from the message itself.
/// </summary>
public static int MapSnapshotSize(ServerMapSnapshotMessage message)
{
var size = sizeof(byte) + sizeof(int) + sizeof(ushort);
foreach (var node in message.Nodes)
{
size += sizeof(byte);
size += StringSize(node.Id) + StringSize(node.ParentId) + StringSize(node.Name);
size += sizeof(byte);
foreach (var item in node.Items)
{
size += StringSize(item);
}
size += sizeof(byte);
foreach (var position in node.Positions)
{
size += StringSize(position);
}
}
return size;
}
public static int WriteMapSnapshot(Span<byte> destination, ServerMapSnapshotMessage message)
{
if (message.Nodes.Count > ushort.MaxValue)
@@ -249,6 +283,9 @@ public static class ProtocolCodec
return new ServerMapSnapshotMessage(schoolId, nodes);
}
/// <summary>Matches <see cref="PacketWriter.WriteString"/>: a <c>u16</c> length plus UTF-8.</summary>
private static int StringSize(string value) => sizeof(ushort) + Encoding.UTF8.GetByteCount(value);
private static void Expect(ref PacketReader reader, MessageType expected)
{
var actual = reader.ReadMessageType();
+6
View File
@@ -35,4 +35,10 @@ internal abstract record GameCommand
/// <summary>Stops every worker, re-reads the save directory, starts workers from those files.</summary>
internal sealed record ReloadSaves(TaskCompletionSource Result) : GameCommand;
/// <summary>
/// A worker thread ended on an exception. The supervisor owns the table, so the worker asks
/// it to drop the school instead of removing itself.
/// </summary>
internal sealed record WorkerFailed(int SchoolId) : GameCommand;
}
+36 -5
View File
@@ -144,9 +144,39 @@ internal sealed class GameLoopService(
case GameCommand.ReloadSaves reload:
await HandleReloadAsync(reload).ConfigureAwait(false);
break;
case GameCommand.WorkerFailed failed:
HandleWorkerFailed(failed.SchoolId);
break;
}
}
/// <summary>
/// A school's thread died. Drop it from the table so the menu stops drawing a card whose clock
/// never moves again, and tell anybody watching it to go back to the menu. The save file stays
/// where it is — a restart or <c>reload-schools</c> is the way back.
/// </summary>
private void HandleWorkerFailed(int schoolId)
{
if (!_workers.ContainsKey(schoolId))
{
return;
}
Untrack(schoolId);
foreach (var client in clients.All)
{
if (client.OpenSchoolId == schoolId)
{
client.OpenSchoolId = null;
SendSchoolGone(client, schoolId);
}
}
logger.LogError("School {SchoolId} stopped after a worker failure; its save file is unchanged.", schoolId);
}
private async Task HandleCreateAsync(GameCommand.CreateSchool command)
{
try
@@ -373,6 +403,11 @@ internal sealed class GameLoopService(
await worker.StopAsync(persist: false).ConfigureAwait(false);
}
}
if (_workers.Count > 0)
{
logger.LogInformation("Restored {Count} school(s) from disk.", _workers.Count);
}
}
private async Task StopAllWorkersAsync(bool persist)
@@ -386,11 +421,6 @@ internal sealed class GameLoopService(
{
await Task.WhenAll(stopping).ConfigureAwait(false);
}
if (_workers.Count > 0)
{
logger.LogInformation("Restored {Count} school(s) from disk.", _workers.Count);
}
}
private SchoolWorker SpawnWorker(
@@ -416,6 +446,7 @@ internal sealed class GameLoopService(
metrics,
store,
mods,
onFailed: schoolId => commands.Enqueue(new GameCommand.WorkerFailed(schoolId)),
loggerFactory.CreateLogger($"HSchool.Server.Game.SchoolWorker.{id}"));
private void Track(SchoolWorker worker)
+25
View File
@@ -92,6 +92,11 @@ internal sealed class SchoolStore
{
var saves = new List<SchoolSave>();
// The id inside the file decides which school this is and which file it is saved back to,
// so two files claiming the same id would give the menu two cards over one worker and then
// overwrite each other on the next save.
var claimed = new Dictionary<int, string>();
foreach (var path in Directory.EnumerateFiles(DirectoryPath, "*.json"))
{
if (string.Equals(Path.GetFileName(path), IndexFileName, StringComparison.OrdinalIgnoreCase))
@@ -109,6 +114,15 @@ internal sealed class SchoolStore
continue;
}
if (save.Id <= 0)
{
_logger.LogWarning(
"Save {Path} has id {Id}; ids start at 1. Leaving the file in place.",
path,
save.Id);
continue;
}
if (!GameClock.IsValidStartDate(save.GameTime))
{
_logger.LogWarning(
@@ -117,6 +131,17 @@ internal sealed class SchoolStore
continue;
}
// Claimed last, so a file rejected above does not reserve an id a good file needs.
if (!claimed.TryAdd(save.Id, path))
{
_logger.LogWarning(
"Save {Path} claims id {Id}, already taken by {Owner}; leaving the file in place.",
path,
save.Id,
claimed[save.Id]);
continue;
}
saves.Add(new SchoolSave
{
Format = save.Format,
+108 -34
View File
@@ -29,6 +29,7 @@ internal sealed class SchoolWorker
private readonly bool _isNew;
private readonly IReadOnlyList<string>? _modIds;
private readonly MapLayout? _savedMap;
private readonly Action<int> _onFailed;
private readonly int _id;
private readonly string _name;
@@ -40,6 +41,8 @@ internal sealed class SchoolWorker
private School? _school;
private Task? _run;
private bool _persistOnStop = true;
private bool _settingsDirty;
private long _lastSettingsSave;
public SchoolWorker(
int id,
@@ -55,6 +58,7 @@ internal sealed class SchoolWorker
GameMetrics metrics,
SchoolStore store,
ModContent mods,
Action<int> onFailed,
ILogger logger)
{
_id = id;
@@ -70,6 +74,7 @@ internal sealed class SchoolWorker
_metrics = metrics;
_store = store;
_mods = mods;
_onFailed = onFailed;
_logger = logger;
_snapshot = new SchoolState(id, name, time, running, (byte)speedIndex);
}
@@ -127,11 +132,35 @@ internal sealed class SchoolWorker
{
_logger.LogWarning(ex, "School {SchoolId} was not started; the save file is unchanged.", _id);
_started.TrySetException(ex);
ReportFailure();
}
catch (Exception ex)
{
_logger.LogError(ex, "School {SchoolId} worker died.", _id);
_started.TrySetException(ex);
ReportFailure();
}
}
/// <summary>
/// Tells the supervisor this school is gone. Without it a dead worker stayed in the table and
/// the menu kept drawing its card with a frozen clock, as if the school were alive.
/// </summary>
private void ReportFailure()
{
if (_stopping.IsCancellationRequested)
{
// Already being torn down on purpose; the supervisor knows.
return;
}
try
{
_onFailed(_id);
}
catch (Exception ex)
{
_logger.LogError(ex, "Could not report the failure of school {SchoolId}.", _id);
}
}
@@ -219,6 +248,8 @@ internal sealed class SchoolWorker
BroadcastClock();
}
FlushSettings();
if (Stopwatch.GetElapsedTime(lastSave) >= _options.SaveInterval)
{
Persist();
@@ -272,32 +303,45 @@ internal sealed class SchoolWorker
while (_mailbox.Reader.TryRead(out var command))
{
switch (command)
// Every command here was triggered by a browser. One of them failing — an oversized
// snapshot, a client that vanished mid-send — must cost that command, not the school.
try
{
case WorkerCommand.Open open:
open.Client.OpenSchoolId = _id;
SendMapSnapshot(open.Client, school);
BroadcastClockTo(open.Client, school);
break;
switch (command)
{
case WorkerCommand.Open open:
open.Client.OpenSchoolId = _id;
SendMapSnapshot(open.Client, school);
BroadcastClockTo(open.Client, school);
break;
case WorkerCommand.Close close:
var leaving = _clients.Find(close.PlayerId);
if (leaving?.OpenSchoolId == _id)
{
leaving.OpenSchoolId = null;
}
case WorkerCommand.Close close:
var leaving = _clients.Find(close.PlayerId);
if (leaving?.OpenSchoolId == _id)
{
leaving.OpenSchoolId = null;
}
break;
break;
case WorkerCommand.SetRunning setRunning:
school.Clock.IsRunning = setRunning.Running;
dirty = true;
break;
case WorkerCommand.SetRunning setRunning:
school.Clock.IsRunning = setRunning.Running;
dirty = true;
break;
case WorkerCommand.SetSpeed setSpeed:
school.Clock.SpeedIndex = setSpeed.SpeedIndex;
dirty = true;
break;
case WorkerCommand.SetSpeed setSpeed:
school.Clock.SpeedIndex = setSpeed.SpeedIndex;
dirty = true;
break;
}
}
catch (Exception ex)
{
_logger.LogError(
ex,
"Command {Command} failed for school {SchoolId}; the school keeps running.",
command.GetType().Name,
_id);
}
}
@@ -305,10 +349,29 @@ internal sealed class SchoolWorker
{
PublishSnapshot();
BroadcastClock();
Persist();
// Not written here: a client can send SetSpeed as fast as the socket allows, and each
// one used to be a synchronous file write on this thread. FlushSettings coalesces them.
_settingsDirty = true;
}
}
/// <summary>
/// Writes pause/speed changes, at most once per <see cref="SimulationOptions.MinSaveInterval"/>.
/// A single click still lands within that window; a burst collapses into one write.
/// </summary>
private void FlushSettings()
{
if (!_settingsDirty || Stopwatch.GetElapsedTime(_lastSettingsSave) < _options.MinSaveInterval)
{
return;
}
Persist();
_settingsDirty = false;
_lastSettingsSave = Stopwatch.GetTimestamp();
}
private void PublishSnapshot()
{
var school = _school;
@@ -335,17 +398,25 @@ internal sealed class SchoolWorker
return;
}
_store.Save(new SchoolSave
// A full disk or a locked file must not end the school; the next save will try again.
try
{
Format = SchoolStore.CurrentFormat,
Id = school.Id,
Name = school.Name,
GameTime = school.Clock.Time,
Running = school.Clock.IsRunning,
SpeedIndex = school.Clock.SpeedIndex,
ModIds = school.Catalog?.PackIds,
Map = school.Map,
});
_store.Save(new SchoolSave
{
Format = SchoolStore.CurrentFormat,
Id = school.Id,
Name = school.Name,
GameTime = school.Clock.Time,
Running = school.Clock.IsRunning,
SpeedIndex = school.Clock.SpeedIndex,
ModIds = school.Catalog?.PackIds,
Map = school.Map,
});
}
catch (Exception ex)
{
_logger.LogError(ex, "Could not save school {SchoolId}; it keeps running unsaved.", _id);
}
}
private void BroadcastClock()
@@ -387,8 +458,11 @@ internal sealed class SchoolWorker
node.Positions);
}
var frame = new byte[ProtocolConstants.MaxMessageSize];
var length = ProtocolCodec.WriteMapSnapshot(frame, new ServerMapSnapshotMessage(school.Id, nodes));
// Sized from the message, not from the inbound frame limit: a map the player enlarged in
// the create editor outgrows 8 KiB somewhere past sixty furnished rooms.
var message = new ServerMapSnapshotMessage(school.Id, nodes);
var frame = new byte[ProtocolCodec.MapSnapshotSize(message)];
var length = ProtocolCodec.WriteMapSnapshot(frame, message);
client.TrySendReliable(frame.AsMemory(0, length));
}
+33 -26
View File
@@ -80,6 +80,13 @@ internal sealed class GameClient(uint playerId, WebSocket socket)
var reliable = _reliable.Reader;
var outbox = _outbox.Reader;
// The two waits live across iterations on purpose. Creating a fresh pair every idle pass
// left the loser of Task.WhenAny queued on its channel forever — and because clock frames
// arrive twenty times a second, the reliable channel collected one dead waiter per tick,
// each holding a cancellation registration, for as long as the player stayed in a school.
Task<bool>? waitReliable = null;
Task<bool>? waitOutbox = null;
while (Socket.State == WebSocketState.Open && !cancellationToken.IsCancellationRequested)
{
if (reliable.TryRead(out var reliableFrame))
@@ -102,44 +109,44 @@ internal sealed class GameClient(uint playerId, WebSocket socket)
continue;
}
var waitReliable = reliable.WaitToReadAsync(cancellationToken).AsTask();
var waitOutbox = outbox.WaitToReadAsync(cancellationToken).AsTask();
// A completed channel is empty for good; waiting on it again would spin.
if (!reliable.Completion.IsCompleted)
{
waitReliable ??= reliable.WaitToReadAsync(cancellationToken).AsTask();
}
if (!outbox.Completion.IsCompleted)
{
waitOutbox ??= outbox.WaitToReadAsync(cancellationToken).AsTask();
}
if (waitReliable is null && waitOutbox is null)
{
return;
}
Task<bool> finished;
try
{
finished = await Task.WhenAny(waitReliable, waitOutbox).ConfigureAwait(false);
finished = waitReliable is null ? waitOutbox!
: waitOutbox is null ? waitReliable
: await Task.WhenAny(waitReliable, waitOutbox).ConfigureAwait(false);
await finished.ConfigureAwait(false);
}
catch (OperationCanceledException)
{
return;
}
bool hasData;
try
// Only the wait that finished is dropped; the other one stays queued on its channel.
if (ReferenceEquals(finished, waitReliable))
{
hasData = await finished.ConfigureAwait(false);
waitReliable = null;
}
catch (OperationCanceledException)
else
{
return;
}
if (hasData)
{
continue;
}
var other = ReferenceEquals(finished, waitReliable) ? waitOutbox : waitReliable;
try
{
if (!await other.ConfigureAwait(false))
{
return;
}
}
catch (OperationCanceledException)
{
return;
waitOutbox = null;
}
}
}
+4 -2
View File
@@ -95,6 +95,10 @@ internal sealed class GameSocketHandler(
commands.Enqueue(new GameCommand.CloseSchool(client.PlayerId, watchedSchoolId));
}
// Cancel first, then wait. The other order hung here whenever the send loop sat inside
// SendAsync on a half-open connection: nothing was left to cancel it.
await connectionCts.CancelAsync().ConfigureAwait(false);
if (sendLoop is not null)
{
try
@@ -106,8 +110,6 @@ internal sealed class GameSocketHandler(
// Expected while tearing the connection down.
}
}
await connectionCts.CancelAsync().ConfigureAwait(false);
}
}
+10 -2
View File
@@ -38,15 +38,23 @@ public sealed class SimulationOptions
public string ModsDirectory { get; set; } = "mods";
/// <summary>
/// How often a running school writes its clock to disk. Create, pause, speed and shutdown
/// write immediately; the tick itself never does.
/// How often a running school writes its clock to disk. Create and shutdown write
/// immediately; the tick itself never does.
/// </summary>
public int SaveIntervalSeconds { get; set; } = 30;
/// <summary>
/// Shortest gap between two saves caused by pause or speed. A client can send those as fast
/// as the socket allows, and each one used to be a file write on the school's own thread.
/// </summary>
public int MinSaveIntervalMilliseconds { get; set; } = 1000;
/// <summary>Length of one fixed step.</summary>
public double FixedDeltaTime => 1d / TickRate;
public TimeSpan TickInterval => TimeSpan.FromSeconds(1d / TickRate);
public TimeSpan SaveInterval => TimeSpan.FromSeconds(SaveIntervalSeconds);
public TimeSpan MinSaveInterval => TimeSpan.FromMilliseconds(MinSaveIntervalMilliseconds);
}
@@ -158,6 +158,52 @@ public class ProtocolCodecTests
Assert.Equal(["Директор"], read.Nodes[1].Positions);
}
[Fact]
public void MapSnapshotSize_IsExactlyWhatTheWriterProduces()
{
var message = new ServerMapSnapshotMessage(7, [
new MapSnapshotNode(0, "yard", "", "Двор", [], []),
new MapSnapshotNode(2, "floor-1", "main", "Этаж 1", [], []),
new MapSnapshotNode(3, "office", "floor-1", "Кабинет директора", ["Стол", "Стул"], ["Директор"]),
]);
var size = ProtocolCodec.MapSnapshotSize(message);
var buffer = new byte[size];
Assert.Equal(size, ProtocolCodec.WriteMapSnapshot(buffer, message));
}
[Fact]
public void MapSnapshot_OfALargeSchoolSurvivesTheEightKilobyteLimit()
{
// A player who keeps clicking "Add room" in the create editor gets past 8 KiB somewhere
// around sixty furnished rooms. Sizing the buffer from the message is what keeps that
// school openable instead of killing its worker thread on the first snapshot.
var nodes = new List<MapSnapshotNode> { new(0, "yard", "", "Двор", [], []) };
for (var i = 1; i <= 200; i++)
{
nodes.Add(new MapSnapshotNode(
3,
$"principals-office-{i}",
"floor-1",
"Кабинет директора",
["Кресло директора", "Стол", "Стул"],
["Директор"]));
}
var message = new ServerMapSnapshotMessage(1, nodes);
var buffer = new byte[ProtocolCodec.MapSnapshotSize(message)];
Assert.True(buffer.Length > ProtocolConstants.MaxMessageSize);
var length = ProtocolCodec.WriteMapSnapshot(buffer, message);
var read = ProtocolCodec.ReadMapSnapshot(buffer.AsSpan(0, length));
Assert.Equal(nodes.Count, read.Nodes.Count);
Assert.Equal("principals-office-200", read.Nodes[^1].Id);
Assert.Equal(["Кресло директора", "Стол", "Стул"], read.Nodes[^1].Items);
}
[Fact]
public void Numbers_AreLittleEndian()
{