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 segfault when quitting a level while grabbing #1758

Merged
merged 1 commit into from
Jun 6, 2021

Conversation

seanyeh
Copy link
Contributor

@seanyeh seanyeh commented Jun 2, 2021

Fixes #1739

Reproduction steps:
Jump on Mr ice block, grab, and abort level while grabbing. Segfaults

As explained in the issue, the player is destroyed before the grab listener. I believe it segfaults here:

for (const auto& entry : m_remove_listeners) {
entry->object_removed(this);
}

when the grab listener (associated with the now-destroyed player) does something.

By ungrabbing in the player destructor, we also remove this listener and avoid this issue. Let me know if this seems correct, thanks

@Semphriss Semphriss self-requested a review June 4, 2021 15:49
@Semphriss
Copy link
Member

I'll check this ASAP - the thing I'm afraid about is if the lamp gets destroyed first, then the player might cause a segfault now when ungrabbing the now-destroyed lamp.

@tobbi
Copy link
Member

tobbi commented Jun 6, 2021

@Semphriss Did this work? From a quick glance, it looks like it should.

@Grumbel
Copy link
Member

Grumbel commented Jun 6, 2021

To avoid these issues in the future I would recommend not using pointers directly, but holding GameObject by their UID and using GameObjectManager::get_object_by_uid() to retrieve the pointer when needed. Makes it much easier to deal with disappearing objects.

@Semphriss
Copy link
Member

On quick first try, it seems to have fixed the problem 👍

@tobbi tobbi merged commit 029f5c5 into SuperTux:master Jun 6, 2021
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.

Quitting a level while not carrying a lantern properly causes a segfault
4 participants