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

Disconnect from channel #107

Merged
merged 3 commits into from
Aug 17, 2021
Merged

Conversation

fedulvtubudul
Copy link
Contributor

This patch fixes the crash during an attempt to disconnect from a channel.

Related issues:

Currently I'm working on the memory leaks that are still there, but they are a bit harder to verify than the crash itself.

@@ -110,7 +110,6 @@ +(void)nativeClose:(NSValue *)nativeTransport {
MSC_TRACE();

[TransportWrapper extractNativeTransport:nativeTransport]->Close();
[nativeTransport release];
Copy link
Contributor

Choose a reason for hiding this comment

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

hi @fedulvtubudul , I'm curious why is this not necessary.. when/where is it released instead?
not an expert btw but i've been using this library and faced some issues.
thanks!

Copy link
Contributor Author

@fedulvtubudul fedulvtubudul Aug 14, 2021

Choose a reason for hiding this comment

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

This fragment looks fragile. If user does not close the transport, the transport object will leak. If user will try to close the transport twice, app will crash while accessing already deallocated object.

With this change, app does not crash after leaving a channel (or ending a call). BUT yes, it has memory leaks, lots of them actually. I'm trying to solve the leaks, you can check the progress here: VLprojects@bb2295c, but it's not finished yet.

@pablito25sp replying to your answer, I believe each "native" object lifecycle should be bound to it's obj-c wrapper object lifecycle. So we should call new from init and free from dealloc. That should make library usage more safe.

@ethand91 ethand91 merged commit d0a0d35 into ethand91:master Aug 17, 2021
@fedulvtubudul fedulvtubudul deleted the disconnect-from-channel branch November 18, 2021 06:49
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.

3 participants