Page MenuHomeWildfire Games

Clogger crashes, when the message is too long
ActivePublic

Authored by Imarok on Nov 7 2017, 6:49 PM.
Index: binaries/data/mods/public/gui/pregame/mainmenu.js
===================================================================
--- binaries/data/mods/public/gui/pregame/mainmenu.js (Revision 20419)
+++ binaries/data/mods/public/gui/pregame/mainmenu.js (Arbeitskopie)
@@ -195,6 +195,16 @@
}
}
+function crashTheThing()
+{
+ let someString = "";
+ for(let i = 0; i < 16367; ++i)
+ someString += "a";
+
+ warn("someStrings length: " + someString.length);
+ warn("someString: " + someString);
+}
+
/**
* Opens the menu by revealing the screen which contains the menu.
*/
Index: binaries/data/mods/public/gui/pregame/mainmenu.xml
===================================================================
--- binaries/data/mods/public/gui/pregame/mainmenu.xml (Revision 20419)
+++ binaries/data/mods/public/gui/pregame/mainmenu.xml (Arbeitskopie)
@@ -507,9 +507,9 @@
size="4 132 100%-4 160"
tooltip_style="pgToolTip"
>
- <translatableAttribute id="caption">Exit</translatableAttribute>
- <translatableAttribute id="tooltip">Exits the game.</translatableAttribute>
- <action on="Press">exitGamePressed();</action>
+ <translatableAttribute id="caption">Crash</translatableAttribute>
+ <translatableAttribute id="tooltip">Crash the game.</translatableAttribute>
+ <action on="Press">crashTheThing();</action>
</object>
</object>

Event Timeline

Imarok edited the content of this paste. (Show Details)Nov 7 2017, 6:49 PM
Imarok changed the title of this paste from untitled to Clogger crashes, when the message is too long.
Imarok updated the paste's language from autodetect to autodetect.
elexis added a subscriber: elexis.Nov 7 2017, 6:56 PM

We run into this when trying to warn(uneval(g_CivInfo)) or some entity simstate.
Should we support extremely long messages or just show the N first characters and possibly an additional warning about a too long message?

I vote for the second option, we don't really need to support these long messages (just write them to some log if needed).

Imarok added a comment.Nov 7 2017, 7:01 PM

At least we should do anything, but not crashing.
The limit of 16366 chars in warning comes from a limit of 16384 in debug_printf.
This limit already was changed from 4096 to 16384 in rP10491.

Imarok changed the visibility from "All Users" to "Public (No Login Required)".Nov 7 2017, 7:02 PM
elexis added a comment.Nov 7 2017, 7:13 PM

Leaning towards that too.

While at it:
The only way I ran into this segfault was by doing warn(uneval(hugeObject)).
D401 does exactly that if the object couldn't be converted, do we want that?

Imarok added a comment.Nov 7 2017, 7:24 PM

maybe just show a part of the message and append that with ....(too long message)?

Anyone knows, why 16384 was choosen as limit?

A power of 2 greater than the too short 4096 without being completely excessive?
(I'm leaning towards having D401 abandoned, but I don't want to be rude and maybe I didn't discover the use case yet)

In P91#662, @elexis wrote:

A power of 2 greater than the too short 4096 without being completely excessive?
(I'm leaning towards having D401 abandoned, but I don't want to be rude and maybe I didn't discover the use case yet)

So no :D
(My paste has not really anything to do with D401)

Basically what elexis said. There was some hwdetect work done back then (just checked the logs, no specific mention of that). As to why that should be as small as possible is because that is on the stack, and stack sizes are small, and if we end up running into some ENSURE or similar in a deep callstack we might be short on available stack. That and nobody needed more. Yes crashing in that case is wrong, we should truncate, though I seem to recall a diff (or commit?) that attempted to limit the length of things passed to one of the debug functions.