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 crash when sending ack on removed connection #143

Merged
merged 1 commit into from
Nov 24, 2021

Conversation

deepbluev7
Copy link
Contributor

The emit might interrupt our function and causes us to return to the
event loop. In that time frame the other and might close the connection,
causing our connection to be removed. In that case we call writeAck on a
dangling pointer, in the best case causing a crash. So lets finish our
business before emitting our signal.

fixes #138

@deepbluev7
Copy link
Contributor Author

At least I hope I understood the issue correctly. Seems to fix Nheko-Reborn/nheko#825.

@deepbluev7 deepbluev7 marked this pull request as draft November 24, 2021 04:37
@deepbluev7 deepbluev7 marked this pull request as ready for review November 24, 2021 04:42
Copy link
Collaborator

@Shatur Shatur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Here is one minor suggestion.

singleapplication_p.cpp Outdated Show resolved Hide resolved
The emit might interrupt our function and causes us to return to the
event loop. In that time frame the other and might close the connection,
causing our connection to be removed. In that case we call writeAck on a
dangling pointer, in the best case causing a crash. So lets finish our
business before emitting our signal.

fixes itay-grudev#138
@itay-grudev itay-grudev merged commit 3d7c1ea into itay-grudev:master Nov 24, 2021
@itay-grudev
Copy link
Owner

@deepbluev7 Good spot. Thank you very much.

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.

Latest commit caused app crashed constantly
3 participants