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 async function in NicerConnection #1114

Merged
merged 2 commits into from
Jan 8, 2018

Conversation

jcague
Copy link
Contributor

@jcague jcague commented Jan 8, 2018

Description

We found a bunch of issues in NicerConnection that could be caused by a wrong smart pointer handling in NicerConnection::async function.

[] It needs and includes Unit Tests

Changes in Client or Server public APIs

Not needed.

[] It includes documentation for these changes in /doc.

Copy link
Contributor

@lodoyun lodoyun left a comment

Choose a reason for hiding this comment

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

Good catch! 👍 LGTM!

@jcague jcague merged commit b0e566c into lynckia:master Jan 8, 2018
@jcague jcague deleted the fix/nicer_connection_close_pointer branch January 8, 2018 12:58
@kekkokk
Copy link
Contributor

kekkokk commented Jan 8, 2018

I need to tell you that even with this update I find Nicer very unstable. As I said in another issue, to verify my theory simply open one publisher and one page with ?onlySubscribe=true.

cmd+r (reloading) the subscriber page makes erizoJs to crash very easily (sometimes even at first try)

I'll consider reviewing our libnice update PR since is way more reliable and could make ICE restart mechanism possible.

@jcague
Copy link
Contributor Author

jcague commented Jan 8, 2018

we keep investigating the issues you mention, but it doesn't happen to us. We have been using nicer with millions of minutes and saw the same amount of issues we had before with libnice, but with much more stable connections instead (not having unexpected ICE fails). As I said before, ICE restart is also possible with nICEr.
I tried to reproduce it several times with ?onlySubscribe=true and it didn't crash. But I'm curious about any possible difference between our setups that could make it crash in your case. Please, send me an email and we may meet to check possible reasons.

Arri98 pushed a commit to Arri98/licode that referenced this pull request Apr 6, 2021
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