Cause minetest crash (serialization issue) #6
Comments
|
I was not aware there was a size limit on string serialization, it would be interesting to know what that limit was then I could at least prevent a crash, though it could still result in loss of inventory items. It may also be possible to compress the strings using zlib but I think a better long-term solution would be to move away from metadata storage to the new world storage api. Thank you for the detailed report and sorry about your loss. |
|
I just tried it but the problem is still here, and it crashed after the first use. I think it crashes because of the too large number of "meshed" blocks (that are using one entity each ?) |
|
Found: minetest/util/serialize.cpp |
|
They are indeed using one entity each as there is currently no way to create proper voxel area entities so you need to increase Thanks for the info on serialization, at least by knowing the limit I can include a check to prevent crashing, however, a better solution would be to use tables rather than strings for inventory items but I keep getting invalid key to 'next' error when I do |
|
You could save an id number to meta and use modstorage to save the ships, nodemeta is sent to clients and causes unneeded information sending. To fix the serialization problem you could serialize manually. |
|
Yes, mod storage is definitely the way to go in future versions but for now I think f35f481 should at least prevent crashes and does compress strings where possible, books for example could be problematic, I'm not sure. |
|
I got a crash with this one too. Same error with string length, but I didn't seen any warning. |
|
Strange, I felt sure it must be a string formatted inventory item (like a book) that could possibly exceed that limit. It must be some specific node that causes this, are you using anything in particular as building materials that could possibly contain very long strings in their meta? cdc3b1f might help to identify the culprit. |
|
I tested it on a terrain piece (it's easier than building an entire ship), so dirt, stone, grass, including "plantlike" grass, leaves, tree, ... How should I use cdc3b1f ? |
|
Sorry, the last commit merely compresses and checks all string data, if the problem persists then we know the issue is not with node-meta serialization. It's beginning to look that way as the building materials you mention should not contain much (if any) metadata. Yes, it looks like the controller entity is attempting to serialize the already serialized metadata when it statically saves. I will work on a fix asap, the nodemeta compression is likely not needed after all. |
|
Hopefully e99b25a will fix the problem, or at least fail a little more gracefully if a string is too long. |
|
Let see that: no difference ... |
|
As far as I can see, the only serialization (besides meta positions) is done by the controller node when statically saving. I have tried activating large volumes of landscape as you suggested but have still been unable to reproduce the serialization error, however, I have noticed some other problems with large volumes being removed along with the controller but that's another issue. I have moved the string length check back to the serialized inventory itemstacks which is the only place I can think they could possibly exceed the limit. If none of these attempts at fixing has actually produced the log message then I wonder if the limit is actually less than 65535? I am running out of ideas now :-/ |
|
The string size that trigger the alert message is defined here |
|
As there is never any warning about single string length, I can suppose that minetest tries to store a string bigger that the string in the table's fields. Maybe a concatenation of several lines. Maybe minetest is using serializestring for tables too. |
|
I am using linux myself, unless you mean that you are using something else? I would be interested to know if my latest commit makes any difference, though I suspect it will not, you could try reducing the limit to see if you can at least produce the log error rather than crashing. I will continue to look into this, I could always roll my own serializer as @HybridDog suggested but this will be much slower done in pure Lua. |
|
No, I'm using debian 8. The more I try each commit and search in the minetest source code, the less I understand how this bug can appear ... Downloaded from the stable 0.4 branch of the repo |
|
I am also using debian 8, but it seems strange as I just did a quick test and I do not even get that error when calling local hex = "0123456789ABCDEF"
local str = ""
for i = 1, 8192 do
str = str..hex
end
print(str:len())
local tab = {str}
local test = minetest.serialize(tab) |
|
Strange ... it doesn't crash for me too. And it doesn't crash when I serialize the string directly. |
|
The only conclusion we can get is that the issue occurs because a minetest subroutine calls serializeString() instead of serializeLongString where the lua binding minetest.serialize() checks for the string size. |
|
If this is caused by an internal call then it's really an engine bug IMO. What version are you using exactly? I am using 0.4.16 but am a bit behind current head, not sure if anything changed with serialization recently though. EDIT: I see you are using 0.4.16 also, it doesn't look like the code has changed since february :? |
|
This crash occurs for me too.
This crash occurred while a player in our server was flying, he lost his ship when the server was restarted. |
|
I wonder if this is related to blocks count. for example too wide to too many blocks used. |
This does not call serializeString - serialize is a Lua function defined which writes tables to a format similar to Lua syntax. seralizeString is a function that writes a string to a binary buffer before sending or storing it. I'd need to look into this mod though to see what the issue could be. I won't be able to do this for the next few weeks, but I can confirm that this certainly looks like an engine issue - whether or not that's caused by the stuff you're serializing being excessively long (in which case the crash should have a better error message anyway) |
|
We have a test world with only the meshnode mod and the default game enabled which consistently reproduces this crash for at least two different Linux environments. http://9skunks.tech/minetest/meshnode-only-test-world-testthis.tar.xz |
|
I was able to reproduce this with a git version of minetest. |
|
With a debug build. Backtrace. Full GDB log. - https://pastebin.com/j1HUq5aT |
|
@orbea Thanks for the info, I will look into this as soon as I have time. @rubenwardy Thank you for your interest in this issue, let's hope we can trace the problem :) |
|
Thanks! I would really like to see this mod work well. :) |
|
While it would be nice to get to the bottom of this for a number of reasons, I would not recommend anyone waste too much time on this mod as it will never be perfect. Better to work on true engine provided 'voxel area entities' instead, this is (or was) on the longer term roadmap for minetest, iirc. |
|
Yea, a proper solution will always be more desirable. I guess to rephrase I would really like to see the ideas behind this mod work well. :) |
I guy ! Your mod is a pure dream when it works !
Sometimes, when I use a ship (aroud 10m radius) it brings minetest to crash after few seconds during a flight. For now it always has happend during the first fly. After the first it seems to work always.
Here is the minetest output, (no problem before)
I think it can be a error during serialization of node's metadata.
When I restart the server, The ship has disappeared ( :-<)
The text was updated successfully, but these errors were encountered: