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

Make sure we know when deleting an emitting object #35423

Merged
merged 1 commit into from
Jan 22, 2020

Conversation

Faless
Copy link
Collaborator

@Faless Faless commented Jan 22, 2020

We used a lock signals in the signal_map while emitting, because it was not allowed to disconnect them while being emitted.
We used that lock to check if we where deleting an object during signal emission.

Now that we allow to disconnect signals while they are being emitted (#35088), if an object first disconnects, then gets deleted we can't know that a signal was being emitted during the destructor (like in #33290).

This commit adds a new _emitting boolean member to Object to be set while emitting and checked in the destructor, while removing the old signal lock which is now unused.

I hope I got this right 🙄

core/object.cpp Outdated Show resolved Hide resolved
@akien-mga akien-mga requested a review from reduz January 22, 2020 07:52
We used a lock signals in the signal_map while emitting, because it was
not allowed to disconnect them while being emitted.
We used that lock to check if we where deleting an object during signal
emission.
Now that we allow to disconnect signals while they are being emitted, if
an object first disconnects, then gets deleted we can't know that a
signal was being emitted during the destructor.

This commit adds a new `_emitting` boolean member to Object to be set
while emitting and checked in the destructor, while removing the old
signal lock which is now unused.
@Faless Faless force-pushed the fix/object_emit_free branch from 9d9015a to 41f59ec Compare January 22, 2020 13:08
@akien-mga akien-mga modified the milestones: 4.0, 3.2 Jan 22, 2020
@akien-mga akien-mga merged commit 409de53 into godotengine:master Jan 22, 2020
@akien-mga
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants