-
Notifications
You must be signed in to change notification settings - Fork 1k
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
webrtc: add a test for establishing many connections #2801
base: master
Are you sure you want to change the base?
Conversation
ad84c06
to
8fcca64
Compare
pc.OnConnectionStateChange(func(_ webrtc.PeerConnectionState) { | ||
switch pc.ConnectionState() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works fine, but the change makes it better in case there are ordering issues for the callback. The callback does need to guarantee that we can access connection state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently it doesn't work fine. pion/webrtc#2702 (comment)
So we do need this change.
24f7760
to
5a485d6
Compare
return | ||
} | ||
s.Write(b[:]) | ||
s.Read(b[:]) // peer will close the connection after read |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we expect an error here then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a check.
3c3731a
to
64836ad
Compare
22aa5d5
to
a611a26
Compare
No description provided.