prevent ICETransport start/stop deadlock #2897
Conversation
edaniels
force-pushed
the
icetransportdeadlock
branch
from
d89c94f to
53c8eee
Compare
| if t.mux != nil { | ||
| var closeErrs []error | ||
| if shouldGracefullyClose && t.gatherer != nil { |
There was a problem hiding this comment.
I looked at these closes and they should be safe to call concurrently. If they aren't, we'll see some other problems crop up quickly. But I haven't found those to happen in any concurrent PC close test runs.
| @@ -157,6 +157,10 @@ func (t *ICETransport) Start(gatherer *ICEGatherer, params ICEParameters, role * | |||
| return err | |||
There was a problem hiding this comment.
Can't comment on the line but it looks like you're Locking the t.lock above and never unlocking; is that on purpose?
There was a problem hiding this comment.
yeah there's a defer unlock above. not sure how i feel about that pattern!
There was a problem hiding this comment.
I don't like it haha, but since it was pre-existing and I have basically no context here will approve.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2897 +/- ##
==========================================
- Coverage 79.03% 78.89% -0.15%
==========================================
Files 89 89
Lines 8477 8481 +4
==========================================
- Hits 6700 6691 -9
- Misses 1293 1302 +9
- Partials 484 488 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -157,6 +157,10 @@ func (t *ICETransport) Start(gatherer *ICEGatherer, params ICEParameters, role * | |||
| return err | |||
There was a problem hiding this comment.
I don't like it haha, but since it was pre-existing and I have basically no context here will approve.
No description provided.