Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix potential hang after restoring keyframe due to invalid blocklist pointers. #503

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

PeterN
Copy link
Contributor

@PeterN PeterN commented Aug 21, 2024

Fix potential hang after restoring keyframe due to invalid blocklist pointers.

When restoring a keyframe, the blockmap is not cleared and blocklist pointers are modified during P_UnArchiveThinkers. The blockmap is then restored in UnArchiveBlockLinks, but this can leave blocklist pointers in an invalid state.

This can then lead to a hang later when the blocklist is iterated and may get stuck in an infinite loop.

The hang is fairly easy to reproduce using Legacy of Rust when Vassago are present after using keyframe restore.

I chose to simply wipe the blocklist pointers during restoring as this avoids adding extra functions or modifying much existing code. It's basically brute-forcing it.

…pointers.

When restoring a keyframe, the blockmap is not cleared and blocklist pointers are modified during P_UnArchiveThinkers. The blockmap is then restored in UnArchiveBlockLinks, but this can leave blocklist pointers in an invalid state.

This can then lead to a hang later when the blocklist is iterated and may get stuck in an infinite loop.
@kraflab
Copy link
Owner

kraflab commented Aug 22, 2024

Thanks for the summary and solution! While it looks completely safe, I'll still need to run extensive regression testing as this code is extremely sensitive for TAS demo recording, so it might sit here for a bit until I have a chance to tackle that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants