-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add PeerConnection.GracefulClose #2847
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2847 +/- ##
===========================================
- Coverage 78.94% 65.11% -13.84%
===========================================
Files 89 67 -22
Lines 8335 3262 -5073
===========================================
- Hits 6580 2124 -4456
+ Misses 1278 1011 -267
+ Partials 477 127 -350
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
cff4dd3
to
9dc532b
Compare
First test run had #2848 but verified it happens on |
a77c5e7
to
f057900
Compare
@Sean-Der this now depends on pion/ice#718. Tests will fail with |
@@ -129,6 +129,10 @@ func (m *Mux) readLoop() { | |||
} | |||
|
|||
if err = m.dispatch(buf[:n]); err != nil { | |||
if errors.Is(err, io.ErrClosedPipe) { |
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.
I don't see a reason to have this spuriously log when we are the ones closing the packetio.Buffer.
f057900
to
93ac274
Compare
Take 2: This is a more thorough attempt at closing gracefully. If anything, every path that graceful touches becomes opt-in, so it shouldn't affect any existing users and it makes it safe for backporting.
GracefulClose/Stop added to: