VM aborts in Powerup.EndEffect can break the game

phantombeta · Post by **phantombeta** » Mon Aug 27, 2018 7:16 am

Pretty much what it says on the thread title. To be specific, the game will go on an infinite loop of VM aborts if you cause a VM abort in Powerup.EndEffect when starting a new game.
Weirdly enough, it'll still render and the process won't stop responding, though it won't respond to any player input at all.

Here's a screenshot, just to show off how weird this bug is:

Steps to reproduce:

Code: Select all

1. Give yourself the "PowerTest" item in the console ("give PowerTest")
2. Change the map with the map CCMD or start a new game.

Post by **Graf Zahl** » Mon Aug 27, 2018 7:29 am

This looks like some handler is catching the exception and does not properly forward it...

Post by **_mental_** » Mon Aug 27, 2018 8:17 am

The exception is caught during destruction from DThinker::DestroyAllThinkers(). The engine gives up but tries do the same next frame.
Wrapping DestroyThinkersInList() calls with try/catch won't help much. The game will no longer enters an infinite loop but it's still quite useless as VM abort happens on any attempt to start a new game.
I think proper solution is to destroy objects regardless of exception in this particular situation.

Post by **Graf Zahl** » Mon Aug 27, 2018 8:21 am

Actually, I think the only way to deal with this is to throw a fatal error instead of a recoverable one. This clearly throws the engine into an unrecoverable state so such an action seems appropriate. Just eating the exception and continue as if nothing happened is not a solution.

Post by **_mental_** » Mon Aug 27, 2018 9:25 am

Fatal errors behave very badly on shutdown. We already had one similar Will Not Fix some time ago. The game just crashes because of unhanded exception.

Post by **Graf Zahl** » Wed Oct 31, 2018 1:11 pm

Fixed.

The problem here was that the entire logic for thinker destruction was just facepalm inducing.
It's not the first time I had to redo such careless code of 'self-deleting' linked lists and I think there's a few more lurking in the shadows.

The main loop depended on the side effect that destroying a thinker would unlink it. Just too bad that this won't happen if an exception occurs - especially one that isn't handled properly.
I changed it so that the linked list is taken down entirely before even starting the deletion. This way I can be sure that even if some unknown exception passes through here it won't result in a stale list.

ZDoom