Commit graph

18 commits

Author SHA1 Message Date
Kamron Batman
294dcd94a0
fix: Fixes send-path backpressure: consume IORingGroup 1.0.8, stop dropping packets silently (#2551)
## Summary

Two related fixes on the outbound path:

1. Consume **IORingGroup 1.0.8**, which allows more than one send in flight per socket, and expose the two settings that go with it.
2. Stop `NetState.Send` silently discarding packets when the send buffer fills — including an out-of-bounds write reachable in that state.

## 1. Send-path stall (RIO)

RIO reports send completion on **acknowledgement**, not on copy, so a completion cannot arrive sooner than one round trip. With one send in flight, `PostSend` refused to post again until the previous completion arrived — capping a connection at **one send per RTT** whenever it had data queued.

Measured on a 50ms-RTT production shard:

| | before | after |
|---|---|---|
| in-game latency, data flowing | **101–146 ms** | **48–51 ms** |
| p95 | ~135 ms | 52.8 ms |
| samples > 70 ms | 20 | **0** |

The control that confirms the mechanism: server-side post→completion was **unchanged** at median 92ms across both runs. The ACK-binding is inherent to RIO and did not move; only its propagation into application latency did.

Two things worth recording, because they explain why this went unnoticed:

- As little as **6 bytes** of queued data held the gate shut, so it reproduced in empty areas, not just crowded ones.
- The same measurement at loopback RTT is **microseconds**, so local testing could never surface it.

New settings, both restart-time:

- **`network.maxOutstandingSends`** (default 32) — sends in flight per connection. Honoured by RIO only; other backends complete sends on copy and report 1. Costs a request-queue and completion-queue slot per send, **not another buffer**, since every outstanding send addresses a different range of the same registered buffer. Worst-case added latency is roughly `completion RTT / value`.
- **`network.sendBufferSize`** (default 256KB) — per-connection send buffer, coerced to a power of two of at least the platform allocation granularity. This is the lever for the disconnects below, and the per-connection memory ceiling.

## 2. Send buffer full

`NetState.Send` had three failure modes once the buffer filled, none of them visible:

| writable | behaviour |
|---|---|
| `0` | `GetSendBuffer` returned false → **packet dropped**, no log, no disconnect |
| `4 … needed-1` | `Compress` returned 0 → `CommitWrite(0)` → **packet dropped** the same way |
| `1 … 3` | `safeOutputLength = (nuint)output.Length - 4` **underflows** → hot-loop bounds check never trips → **writes past the span** |

The first two leave a client connected while quietly missing game state, which is undiagnosable from either end. The third corrupts the in-flight region of the ring buffer, and is reachable precisely when a connection is congested, since callers only check for non-zero space.

`Compress` now refuses an output too small to bound, and `Send` reports exhaustion instead of dropping — logging and disconnecting with **needed / writable / unacked / capacity**. Those numbers separate a slow client holding the buffer from a buffer genuinely too small for the shard, which is the case that warrants raising `network.sendBufferSize`.

## Testing

`NetworkCompressionBoundsTests` covers the underflow using sentinel bytes around the output window. **Verified to fail without the guard** (4 failures from overwritten sentinels), confirming the out-of-bounds writes were real rather than theoretical.

Full suites green: **788 Server.Tests**, **597 UOContent.Tests**, Release build clean against the published 1.0.8.

## Notes for reviewers

- Upstream change: modernuo/IORingGroup#9.
- The buffer-full path is now *loud* where it used to be silent. If a shard has been quietly dropping packets under load, this will surface as disconnects — that is the intended outcome, and the log line says which setting to raise.
- Follow-up under discussion: promoting a connection to a larger buffer instead of disconnecting, which looks feasible on a live connection since buffers are referenced per-operation rather than bound to the request queue.
2026-07-27 23:06:53 -07:00
Kamron Batman
8b9bab20fd
fix(network): restore huffman code for symbol 0x19 (#2528)
## The bug

#2522 rewrote the outgoing huffman table in `NetworkCompression.cs` and transposed symbol `0x19`'s code from `0x1CE` to `0x12E` (both 9 bits, so the length distribution — and the Kraft sum — stayed valid, which is why nothing obvious tripped).

The real damage is that it broke prefix-freeness. `0x12E` is `100101110`, and symbol `0x0D`'s 8-bit code is `10010111` — a proper prefix of it. The client's decoder walks the tree bit by bit, so it hit a valid leaf at `0x0D` after 8 bits, emitted the wrong byte, and then reframed every subsequent code.

That is exactly what the reporter's capture shows. Server sends `BF 00 0C 00 19 02 00 00 00 01 00 00`; the client's post-decompression stream reads `BF 00 0C 00 0D 55 00 00 01 00 00` — the literal `0D` is the mis-decoded `0x19`, and the packet is now one byte short, so framing desyncs from there on.

## Impact

Any outgoing packet with byte `0x19` anywhere in its body (serials, coordinates, hues, lengths, text) corrupted the stream. Because the desync is in framing rather than a single field, the client silently stops applying server updates while still being able to send — no disconnect, no error.

`StatLockInfo` (`0xBF` subcommand `0x19`) is sent during login, so it reproduces on essentially every connection. This is also #2526: "can only walk a few steps, then the client stops responding" is the same desync, not a VPS sizing problem.

## Fix

One entry, restored to the canonical value:

```diff
-            0x9, 0x191, 0x9, 0x12E, 0x7, 0x03F, ...
+            0x9, 0x191, 0x9, 0x1CE, 0x7, 0x03F, ...
```

## Validation of the whole table

Rather than eyeball 257 entries, I diffed the current table against **every revision of it in this repo's history** — all 34, back through the renames to the original import. All 34 agree with each other, and `0x19` is the sole disagreement with #2522's rewrite. No other entry has ever changed.

I also validated the table structurally: all 257 lengths in `[2,11]`, every value fits its declared bit-length, Kraft–McMillan sum exactly 1, and no code is a prefix of any other. It passes on all counts now, and the prefix check is what located the bug in the first place.

Both checks were one-off validation scripts, not committed — see below.

## Test

A single known-answer test (`~10ms`) that compresses all 256 symbols and asserts the exact output bytes. The expected bytes were generated from the canonical table, *not* from the implementation, so the test isn't circular. Any single wrong table entry changes the output, so it pins all 256 entries plus the terminal code, and it exercises the encoder end to end.

A round-trip test would **not** catch this class of bug — encoder and decoder built from the same table agree with each other even when the table is wrong. The contract being violated is with the client's hard-coded tree, so the expected bytes have to come from outside the implementation.

The structural prefix-free check and a second `StatLockInfo` vector were deliberately dropped after they'd served their purpose: the table is now verified and effectively frozen, so the structural check was guarding a constant, and the `StatLockInfo` vector is a strict subset of the all-symbols one. What remains covers the risk that's still live — `Compress` is a hand-unrolled bit-packing loop that will get optimized again, and this is the guard against that rewrite silently corrupting the wire format, which is precisely what happened here.

Verified the test fails when the bug is reintroduced and passes when fixed. Full `Server.Tests` suite green: 727 passed.
2026-07-14 09:26:39 -07:00
Kamron Batman
e035768ef8
fix: Optimizes outgoing packet encoding. (#2522)
### Summary

* Fixes a regression in ModernUO huffman encoding compared to RunUO & ServUO.
* Optimizes the encoding by 1.6x-2x using.

### Benchmarks
```cs
| Method                 | Categories | Mean     | Error   | StdDev  | Ratio |
|----------------------- |----------- |---------:|--------:|--------:|------:|
| BenchmarkSUOAcctPacket | AcctPacket | 757.1 ns | 3.83 ns | 3.58 ns |  1.00 |
| BenchmarkMUOAcctPacket | AcctPacket | 941.4 ns | 3.27 ns | 2.73 ns |  1.24 |
| BenchmarkOptAcctPacket | AcctPacket | 530.7 ns | 1.24 ns | 1.10 ns |  0.70 |
|                        |            |          |         |         |       |
| BenchmarkSUOGump       | GumpPacket | 526.8 ns | 2.10 ns | 1.86 ns |  1.00 |
| BenchmarkMUOGump       | GumpPacket | 657.7 ns | 1.25 ns | 1.11 ns |  1.25 |
| BenchmarkOptGump       | GumpPacket | 285.0 ns | 0.77 ns | 0.68 ns |  0.54 |
```
2026-07-12 18:37:14 -07:00
Kamron Batman
ebaf104935
chore: Use var everywhere (#2294) 2025-12-27 16:47:28 -08:00
Kamron Batman
5b99402666
chore: Cleans up unused imports (#1550) 2023-10-15 11:45:42 -07:00
Kamron Batman
10a69bf754
feat: Adds a memory mirrored ring buffer for networking. (#1533)
## Breaking Changes

Incoming packet registration signature has changed to:
```cs
delegate* void OnReceiveCallback(NetState state, SpanReader reader, int packetLength);

IncomingPackets.Register(int packetID, int length, bool ingame, OnReceiveCallback onReceive);
```

For example, an incoming packet handler signature would now look like this:
```cs
public static void SomeIncomingPacket(NetState state, SpanReader reader, int packetLength)
{
    // Parse the data
}
```

## Summary

Updates the network Pipe class to use a mirrored memory technique. This technique involves mapping the same physical memory to two contiguous virtual memory spaces so the byte buffer appears duplicated. This allows writing to a double-sized array to wrap around without the need for the `CircularBuffer` classes.

In practice this allows us to use `Span<byte>` as if the buffer was a regular array.


### Bug Fixes

- [X] Fixes bad fixed length string parsing
2023-10-09 00:57:53 -07:00
Kamron Batman
f268d5d4e2
fix: Cleans up core code (#1187)
**Only one functional change**
* Fixes a bug in LogFactory where `Warning` is being logged as `Information`

Non-functional changes:
* Updates/Fixes copyright headers
* Removes namespace scopes for core files.

View with [whitespace off](https://github.com/modernuo/ModernUO/pull/1187/files?w=1).
2022-10-10 21:47:08 -07:00
Kamron Batman
8eaa859332
fix: Fixes code genning armor, clothing, and BODs (#810)
* Fixes code genning clothing
* Fixes code genning armor
* Fixes code genning BODs
2021-09-28 22:09:34 -07:00
Kamron Batman
97aa092f17
fix(core): Removes refs for network stack (#461) 2021-02-05 01:11:13 -08:00
Kamron Batman
126b80f74b
fix(core): Cleans up networking (#386)
- [X] Deletes some unused networking code.
2021-01-05 00:41:22 -08:00
Kamron Batman
ac76c57c39
fix(core): Converts account packets to spanwriter (#381)
- [X] Converts account packets to spanwriter
2021-01-04 18:47:20 -08:00
Kamron Batman
990e6fe188
Updates Pipe & Makes NetState more testable (#288)
Updates pipe eliminate result and segments from being allocated.

Bumps release version
2020-10-25 17:40:08 -07:00
Kamron Batman
293e691539
Changes Huffman to in-place & Adds CircularBuffer (#287)
- [X] Adds CircularBuffer
- [X] Updates Packet Encoding/Decoding
- [X] Changes Huffman to in-place

Bumps release version
2020-10-25 12:54:37 -07:00
Kamron Batman
3337cfa4e2
Adds PacketDecoder & Makes NetState testable (#286) 2020-10-24 21:49:28 -07:00
Kamron Batman
741e8d8300
Cleanup/Housekeeping (#242) 2020-09-12 15:31:21 -07:00
Kamron Batman
86b7b3aed1
Fixes formatting (#200) 2020-08-25 18:53:35 -07:00
Kamron Batman
2347533b9c
Bring ZLib in locally. (#132) 2020-05-06 20:07:28 -07:00
Kamron Batman
556a17aba8
Adds style cop (#109) 2020-04-26 00:16:02 -07:00
Renamed from Projects/Server/Network/Compression.cs (Browse further)