-
-
Notifications
You must be signed in to change notification settings - Fork 188
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
Set PeerConnection threads to nullptr when no PeerConnection::Options is passed to constructor #122
Conversation
When creating a new PeerConnection without a PeerConnectionFactory, network, signaling and worker threads are created. After closing the peer connection, Handler will close those threads if they exist after receiving the OnSignalingChange for "close" state.
This reverts commit d48a07d.
When creating a new PeerConnection without PeerConnection::Options, set threads to nullptr so default destructor can take care of it on PeerConnection destruction.
Unrelated to this PR, but related with memory leaks, check this out: I was having leaks related to that, so I patched include/nlohmann/json.hpp and they're gone. |
Please, don't mix different tasks in the same PR.
Please keep in this PR just the changes for Thanks! |
This reverts commit 0ce5e52.
@jmillan sorry about mixing. Reverted las commit. |
This is not right, sorry. Thread members in 'PeerConnection' are hold in `std::unique_ptr', which means they are destructed when 'PeerConnection' gets out of the scope. Besides, this PR set's them to If you have some info of the initial issue (memory leak?) please comment that in a separate issue. |
Ok, not a C++ expert here. I assigned threads to nullptr to avoid dynamically assignment after reading this:
Source: https://www.geeksforgeeks.org/destructors-c/ Then I tested the solution within my implementation and it worked. |
Will debug and investigate deeper. I don't wanna waste your time. |
As @jmillan said, we use std::unique_por for those pointers so it's guaranteed that those are deallocated when the class instance is freed. |
When creating a new PeerConnection without PeerConnection::Options, set threads to nullptr so default destructor can take care of it on PeerConnection destruction.