Adrian Siekierka
Assigned
Reported
Comments
As of 1.14.2 Pre-Release 2 (not sure if earlier), this seems to be once again/still an issue.
The client->server custom payload packet frees the ByteBuf created in the packet read method after the custom payload handler has executed via "if (data != null) { data.release(); }". Vanilla does not, ever, create copies of "data". So far, so good.
The server->client custom payload packet frees the ByteBuf created by accessing the packet's "data" "getter". However, this "getter" actually creates a copy of the ByteBuf, which is then released - the original ByteBuf is never freed, unlike in the client->server custom payload.
As copies are made in the S->C "data" "getter", a quick fix would be to use the same release() call for the original "data" object as is used in the C->S packet processing method.
Generally speaking, there are multiple places in the Minecraft codebase where "invalid namespaced string" exceptions are not being caught, causing the rest of the loading process to crash and burn horribly.
In general, one of the workarounds we have employed locally is to use lwjgl-jemalloc 3.2.1 with the rest of lwjgl being at 3.1.6 on Linux - this hasn't caused any issues so far, and may be a consideration-worthy option.
Two important preceding cases:
Quasiconnectivity was originally an unintended behaviour of the engine. Due to the massive demand of it by players, it was kept in the PC version, albeit removed in the "Innovation" codebase. Grum has confirmed that it was, indeed, a "bug turned feature".
Minecart boosting was originally a bug in the engine related to minecart bounding boxes. Upon the addition of powered rails, which effectively deprecated them, it was removed.
In other words, Minecraft has had cases of both bugs becoming features and bugs not becoming features.
Indeed, which could be deemed a working, if disappointing, solution. However, the second part of the issue (parties who are not Mojang potentially being able to capture the data) is still present. Once you're sending data which can potentially identify a user, you should take extra care to ensure it's transmitted in a safe manner.
(Also, I do believe ways of uniquely identifying users without compromising their privacy should be looked into as well.)
I've been doing extensive testing with an user "creeper123123321" as part of hooking the networking system; I am very confident that, while the copy itself is fine (and probably a good idea!), the underlying ByteBuf being copied is never freed in S->C (whereas it is in C->S), and this causes a leak.