Make read notify use sync.Cond instead of chan

[edaniels]

We've hit a few cases where using a channel as a notify mechanism is subject to races when we are running multi consumer multi producer scenarios. A sync.Cond is more suitable for this given we can properly broadcast when read deadlines are hit, which this change supports.

I thought we were there with d55a60c but it then revealed #299 where reads can miss an event which is especially pertinent in the simulcast probing test.

Fixes: #299

[edaniels]

Note: This adds more allocs (per test). I'm not sure that truly matters.

[codecov]

Codecov Report

Attention: Patch coverage is 90.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 83.24%. Comparing base (d55a60c) to head (8be7cce).

Files	Patch %	Lines
udp/conn.go	33.33%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #304      +/-   ##
==========================================
+ Coverage   83.08%   83.24%   +0.15%     
==========================================
  Files          39       39              
  Lines        2726     2751      +25     
==========================================
+ Hits         2265     2290      +25     
+ Misses        335      334       -1     
- Partials      126      127       +1     

Flag	Coverage Δ
go	83.14% <90.00%> (+0.23%)	⬆️
wasm	65.30% <85.00%> (+0.20%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[paulwe]

the races should be sorted by the recent set of changes.

this proposal ads significant complexity and overhead with no clear benefit. composing channel reads is a lot easier than juggling multiple synchronization primitives. to use a cond we would have to either make changes to deadline.Deadline or bring that feature into Buffer so the deadline, write, and close behaviors could share the cond.

[edaniels]

the races should be sorted by the recent set of changes.

this proposal ads significant complexity and overhead with no clear benefit. composing channel reads is a lot easier than juggling multiple synchronization primitives. to use a cond we would have to either make changes to deadline.Deadline or bring that feature into Buffer so the deadline, write, and close behaviors could share the cond.

The race that is fixed is one present in #299 where a read is missed. The recent set of changes fixed the closing case (so, all future reads are satisfied). The same critical section /~https://github.com/pion/transport/blob/master/packetio/buffer.go#L262-L264 still has an issue with concurrent readers. I'll revise this into something simpler hopefully, but it needs to be addressed.

[paulwe]

The same critical section /~https://github.com/pion/transport/blob/master/packetio/buffer.go#L262-L264 still has an issue with concurrent readers

concurrent reads work as expected now. if there is more than one goroutine waiting in the read loop one makes progress after each write and when the buffer closes they both exit.