Handle incomplete MSG_WAITALL when receiving from the game

[SakhnevichKirill]

The problem

When the game sends the launcher a packet larger than about 8 KB, the launcher can pass a damaged copy of it to the server and then lose track of where the next packet begins.

Every packet from the game arrives as a 4-byte length followed by that many bytes. ReceiveFromGame() asks for the whole packet in one recv() with MSG_WAITALL, but never checks how many bytes actually came back. MSG_WAITALL is allowed to return early — exactly the case RecvWaitAll() in src/Network/VehicleEvent.cpp was written for ("happens frequently in wine, and can also happen natively when the OS pauses the execution"). That helper is used for the socket facing the server; the socket facing the game still uses a plain recv().

When the read does return early, two things go wrong at once:

1. The buffer was resized before the read, so the part that never arrived stays as zero bytes, and the launcher sends the server a full-length packet with a tail of NULs. The server cannot parse it.
2. The bytes that were not read are still sitting in the socket. The next read takes 4 bytes out of the middle of the old packet and treats them as the length of a new one. From that point on, the launcher sends the server packets the game never produced.

What this looks like in practice

Any vehicle whose spawn data is bigger than 8188 bytes — large modded cars, trailers with many parts — breaks the session. On our server (launcher 2.8.1, server 3.9.3, BeamNG 0.39.4, Wine on macOS) attaching a trailer kicked the player every time. The server logs Failed to parse vehicle data as json, and the connection is terminated shortly after.

Two lines from the launcher log that show what is happening. First, a packet that leaves the launcher compressed far better than a healthy packet of the same size, because most of its tail is zeros:

[DEBUG] zlib compressed 10509 B to 1125 B
[DEBUG] (Launcher->Server) Bytes sent: 10509 : Os:0:{"pro

Second, a packet the game never sent — it is a slice out of the middle of the previous vehicle config, produced after the framing was lost:

[DEBUG] (Launcher->Server) Bytes sent: 1285 : Yl:0-1:{"d"dually"}}

Where the 8188 comes from: the game writes the length and the data with a single socket:send(), and LuaSocket splits that into 8192-byte writes. So the launcher can wake up after the first chunk (4 bytes of length + 8188 bytes of data) while the rest is still on the way. In two separate incidents the data that reached the server was cut at exactly that boundary.

This is the same kind of failure as #185, on the other socket.

The change

RecvExactly() keeps reading until the buffer is full, the way RecvWaitAll() already does for the server-facing socket. RecvHeader() and ReceiveFromGame() now use it. The error messages and the exceptions thrown are unchanged.

EINTR is retried instead of being treated as a failure. Without that, a read interrupted before any data has arrived turns into a recv() failed disconnect — the corruption would simply be traded for a dropped connection.

Testing

A small standalone program was used to compare the old and the new behaviour: it sends the same 10507-byte packet through a socket pair in 8192-byte chunks and interrupts the read once, then checks what each version assembled.

Payload: 10507 bytes (frame 10511)

[before]   frame=10511B -> received 10507B, NUL-tail=2319B, intact=NO
[after]    frame=10511B -> received 10507B, NUL-tail=0B,    intact=yes

The NUL tail on the first line is the same symptom seen on the live server. Happy to attach that test program if it is useful.

---

By creating this pull request, I understand that code that is AI generated or otherwise automatically generated may be rejected without further discussion.
I declare that I fully understand all code I pushed into this PR, and wrote all this code myself and own the rights to this code.

[WiserTixx]

Where the 8188 comes from: the game writes the length and the data with a single socket:send(), and LuaSocket splits that into 8192-byte writes. So the launcher can wake up after the first chunk (4 bytes of length + 8188 bytes of data) while the rest is still on the way. In two separate incidents the data that reached the server was cut at exactly that boundary.

Are you sure LuaSocket is doing this or is it the OS?

[SakhnevichKirill]

LuaSocket, and it is checkable on both ends.

Sender side. BeamNG ships LuaSocket 3.0-rc1 (lua/common/libs/luasocket, and the version string is in the game binary), and BeamMP writes the frame with a single TCPLauncherSocket:send(packet) in MPGameNetwork.sendData(). That call does not become one send() syscall — sendraw() in luasocket's src/buffer.c loops in 8192-byte steps:

#define STEPSIZE 8192
...
while (total < count && err == IO_DONE) {
    size_t step = (count-total <= STEPSIZE)? count-total: STEPSIZE;
    err = io->send(io->ctx, data+total, step, &done, tm);
    total += done;
}

So a 10511-byte frame leaves the game as an 8192-byte write plus the remainder, and the launcher can wake up in between.

Receiver side. This kicked players on our server 15 times over the last few days. In all 15 cases the JSON that arrived at the server was cut in exactly the same place: 8183 bytes of JSON plus the 5-byte Os:0: prefix = 8188 bytes of payload, which is exactly 8192 bytes with the 4-byte header. Two different vehicles, always the same byte. A TCP segmentation boundary would not land on the same byte every time; a fixed 8192-byte step in userspace does. (The server logs the packet in full — TServer.cpp:376 — so that length is what was received, not a truncated log line.)

That said, the answer does not change the fix. Whoever splits the data — luasocket's stepping, TCP segmentation, or wine returning early from MSG_WAITALL — recv() is allowed to return fewer bytes than asked for, so the receiving side has to loop. That is exactly why RecvWaitAll() exists for the server-facing socket; this PR gives the game-facing socket the same treatment.

One more detail I should have put in the description: data is declared outside the read loop in both callers (GlobalHandler.cpp:250, Core.cpp:362). After a short read the tail of the buffer is therefore either zeros or leftover bytes from a previous packet, and that is what gets forwarded to the server.

I can attach the small reproduction program if it is useful — it feeds the same frame to both the current and the patched implementation over a socketpair and interrupts the read once.

[SakhnevichKirill]

Worth adding, since "this looks like a wine problem" is the natural next thought: in practice it is wine-specific, and the reason is concrete.

Wine does not implement the flag at all. dlls/ntdll/unix/socket.c translates only MSG_OOB and MSG_PEEK into the unix flags, and for MSG_WAITALL it just logs:

if (params.msg_flags & AFD_MSG_OOB)
    unix_flags |= MSG_OOB;
if (params.msg_flags & AFD_MSG_PEEK)
    unix_flags |= MSG_PEEK;
if (params.msg_flags & AFD_MSG_WAITALL)
    FIXME( "MSG_WAITALL is not supported\n" );

So under wine the call behaves like a plain recv() and returns whatever has already arrived. On Windows the documented contract is the opposite — the request completes only when "the buffer supplied by the caller is completely full", the connection is closed, or the request is cancelled / an error occurs — which is why a Windows player does not run into this.

It also explains why our boundary is so repeatable rather than random: the game hands the frame to the socket in 8192-byte steps, and the launcher returns after the first one.

That said, it is not purely a wine problem: #185 was filed on Windows and reproduced by suspending the launcher process, the Linux build can return early on EINTR, and the flag never promised a full buffer on any platform. That is the same reasoning as #248 — "Fixes running the launcher inside of wine as well as the rare instances where this happens natively" — which fixed the server-facing socket. This PR does the same for the game-facing one.

[WiserTixx]

Could this be removed? It's pretty obvious it receives exactly. The rest is better suited for a commit message.

[SakhnevichKirill]

Done — trimmed to /// Throws!!! like the neighbouring helpers; the rationale now lives in the commit message.

[WiserTixx]

The final parameter what seems a bit inefficient, especially since this function is only used twice. I know the compiler will probably optimize it out because of inline but still.

[SakhnevichKirill]

Removed. Both callers now throw the same recv() failed: <strerror>; the old of header / of data wording is gone. Happy to bring it back as two thin wrappers if you'd rather keep it.