Open rob-deutsch opened 6 months ago
I have read #314 and #77.
When blocking calls are implemented this "issue" will move to the pion/datachannel package (or pion/webrtc package) where they try to replicate Javascript functionality.
@rob-deutsch I think when we do blocking calls, it will bubble up the blocking to datachannels as well, both breakaways from the JS functionality. Would that work?
I'm not an expert in pion's codebase, but I believe that would work.
To expand on my viewpoint for the future...
pion/datachannel
package should replicate the blocking functionality of this underlying pion/sctp
package.pion/webrtc
package would/should be responsibile for replicating the JS functionality ontop of the blocking pion/datachannel
package.A decision will need to be made about how to replicate the JS functionality. Specifically a decision about locking a mutex when running the dc.onBufferedAmountLow
callback is running.
The implementation currently in pion/sctp
only works in a single-threaded JS environment. To work in a Go environment the datachannel needs to be locked when the dc.OnBuffferedAmountLow
callback is running so that no race conditions happen.
Summary
It seems like there is currently no reliable way for an application to be notified when a
Stream
buffer has become empty.Description
The "intuitive" way to be notified on an empty
Stream
buffer doesn't work because it introduces a race condition.The "intuitive" way is to use the
onBufferedAmountLow
callback, but if you're using that to feed the buffer you'll have to useStream.SetBufferedAmountLowThreshold(1)
within the callback, and this is a race condition.Its a race condition because the buffer might've been emptied between the time of the callback starting and
Stream.SetBufferedAmountLowThreshold(1)
being called.Relevant code https://github.com/pion/sctp/blob/80ec14ed0187c64a4ec29950ff029e2013be17b7/stream.go#L427C1-L432C3
Comparison to Javascript
I think this isn't an issue for the Javascript webrtc implementation because it is single threaded. The SCTP component can not process the stream while the onBufferedAmountLow callback is running.
Solutions
This will likely require additional features not included in the Javascript webrtc spec.
The solution should 1) be consistent with other additions and deviations that pion/webrtc has made, and 2) be consistent with the pion/webrtc API style. I am not an expert in either of these.
The simplest way to solve it would be to add a field to the
Config
calledAtomicCallbacks
and if itstrue
calls.lock.Unlock()
after the callback has been run.Drawbacks of this approach are: 1) This functionality would need to be bubbled up to
datachannel.Datachannel
andwebrtc.Datachannel
, and 2) This would represent "hidden state" whereby different "instances" of datachannels would behave differently.