No reliable way to be notified of a Stream buffer becoming empty

[rob-deutsch]

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 use Stream.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 called AtomicCallbacks and if its true call s.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 and webrtc.Datachannel, and 2) This would represent "hidden state" whereby different "instances" of datachannels would behave differently.

[rob-deutsch]

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.

[edaniels]

@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?

[rob-deutsch]

I'm not an expert in pion's codebase, but I believe that would work.

To expand on my viewpoint for the future...

1. In my opinion the pion/datachannel package should replicate the blocking functionality of this underlying pion/sctp package.
2. I would expect that the 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.