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

Handle notify after ready #734

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Handle notify after ready #734

wants to merge 5 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Dec 31, 2018

While digging into connectivity issues as part of jitsi/jitsi-meet#3738 , I discovered a bug (and potential fix) in the way that connectivity-readiness is notified out to other participants/channels during a conference.

In short, are two paths which both need to be called in order for a connection to become 'ready'.

However, only one of these locations currently performs the before & after check to see if the connection state has changed from unready to ready. This should be innocent and just result in more notifySctpConnectionReady calls than necessary, but there appear to be side-effects from that call which result in the connection looping. 7492b47 can be merged in isolation to see the effect of that by enhancing the debug logs a little.

Not yet certain this is the correct or full fix - I think something further down the chain from notifySctpConnectionReady also needs to be handled here, but this helped me to make a bit of progress.

NB: Tests aren't yet working due to an obscure dual-classloaders issue, and commits are out-of-order from some rebasing.

JVB 2018-12-27 09:07:39.204 WARNING: [682] org.jitsi.videobridge.EndpointMessageTransport.log() SCTP connection with Bunny not ready yet.
JVB 2018-12-27 09:07:39.205 WARNING: [682] org.jitsi.videobridge.EndpointMessageTransport.log() No available transport channel, can't send a message
JVB 2018-12-27 09:07:39.211 WARNING: [682] org.jitsi.videobridge.EndpointMessageTransport.log() SCTP connection with Bunny not ready yet.
JVB 2018-12-27 09:07:39.211 WARNING: [682] org.jitsi.videobridge.EndpointMessageTransport.log() No available transport channel, can't send a message
JVB 2018-12-27 09:07:41.208 WARNING: [682] org.jitsi.videobridge.EndpointMessageTransport.log() SCTP connection with Bunny not ready yet.
JVB 2018-12-27 09:07:41.208 WARNING: [682] org.jitsi.videobridge.EndpointMessageTransport.log() No available transport channel, can't send a message

@jitsi-jenkins
Copy link

Hi, thanks for your contribution!
If you haven't already done so, could you please make sure you sign our CLA (https://jitsi.org/icla for individuals and https://jitsi.org/ccla for corporations)? We would unfortunately be unable to merge your patch unless we have that piece :(.

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.

2 participants