Skip to content

Fix event handler unregistering ownership loop#925

Merged
chreden merged 1 commit intomasterfrom
bug/919
Apr 9, 2022
Merged

Fix event handler unregistering ownership loop#925
chreden merged 1 commit intomasterfrom
bug/919

Conversation

@chreden
Copy link
Copy Markdown
Owner

@chreden chreden commented Apr 3, 2022

In the various window managers the shared_ptr for the newly created window was being captured in a lambda so that it could be removed from the window collection on an event when it was closed. Unfortunately capturing the shared_ptr meant it outlived where it was stored, so there was a crash.
Change to capture a weak_ptr instead - it should have been this in the first place as the _closing_windows field is a vector of weak_ptrs anyway.
Closes #919

In the various window managers the `shared_ptr` for the newly created window was being captured in a lambda so that it could be removed from the window collection on an event when it was closed. Unfortunately capturing the `shared_ptr` meant it outlived where it was stored, so there was a crash.
Change to capture a `weak_ptr` instead - it should have been this in the first place as the `_closing_windows` field is a `vector` of `weak_ptr`s anyway.
Closes #919
@chreden chreden added the bug label Apr 3, 2022
@chreden chreden added this to the 2.0.0 milestone Apr 3, 2022
@chreden chreden requested a review from makotocchi April 3, 2022 23:17
@chreden chreden self-assigned this Apr 3, 2022
@chreden chreden merged commit 496ee32 into master Apr 9, 2022
@chreden chreden deleted the bug/919 branch April 9, 2022 21:58
chreden added a commit that referenced this pull request Apr 10, 2022
As part of the fix in #925 they were supposed to store the window to close, but I forgot to actually add that to these lines.
Store the window to close so it can be closed.
#919
chreden added a commit that referenced this pull request Apr 10, 2022
As part of the fix in #925 they were supposed to store the window to close, but I forgot to actually add that to these lines.
Store the window to close so it can be closed.
#919
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Intermittent error removing event tokens

1 participant