Fix #6322: Attempt to crash the script instead of the whole game #9047
Conversation
|
It might still be possible to kill a 32 bit build of the game by forcing lots of little allocations in lots of AI scripts, but it'd be difficult to guard against absolutely every case I think, the way Squirrel's memory allocations are structured. |
…an be allocated instead of crashing the whole game
Even with the 64 bit build it is still possible to crash the game with lots and lots of small allocations, but then you run into the territory of not having the required free memory to clean up that mess. That can of course be mostly worked around by letting the script allocator allocate something like 1 MB that it can free upon malloc/realloc returning null, so the allocations for the cleanup are more likely to succeed but that sounds like an even worse solution. Having said that, maybe the memory limit the user has "set" for the scripts might then be too high for their environment and they rather should reduce that. But alas, it is attempting not to crash the game and with the "extreme" (usually buggy) allocations it will do that job quite nicely. When your system is severely memory starved, yes... then this is not guaranteed going to help but there is not much that can be done about that. And if it is that memory starved, even the standard crash handling routines would probably crash as well as there is no memory to allocate the window for an message box, even the string to show in the error window, or anything related to making crash saves. |
LordAro
added
the
backport requested
LordAro
added
backported
Motivation / Problem
See #6322. A script allocating (way) too much memory can crash the game which is arguably worse than crashing just the script.
Description
The solution is two fold:
As side effect some extra information about the allocation was added to the error message that could potentially help the script's developer, but that required changing the error message in Script_FatalError from char* to std::string&.
Limitations
Scripts can now be killed earlier, and probably even during saving, when the memory limit is reached. Previously if the memory was returned before the save or tick finished, the script would just continue.
It is a somewhat open question whether the old behavior of limit testing should be maintained, so the AI can for short bursts and saving use more memory. That would mean this change does not impact the scripts as much, however the benefit of doing it when the allocation happens is that the log will show exactly where the limit got exceeded and by how much. This could then help the script developer see where a huge amount of memory got allocated (in error) and that would make fixing the underlying issues in the scripts easier.
In case the OS is out of memory, and there is not even enough memory to go through the script engine cleanup the game will still crash out with MallocError. Though, not going through the script engine cleanup will leave some "backups" to be not restored so the game would be in an undefined state and it would likely crash at another place.
Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.