Notify initial permissions (#2595)

* Notify initial permissions

NOTE: This does add an initial subscription permission notification
which should be fine, but something to watch for.

A stress test combining
- mute/unmute on publisher side.
- allowing/revoking permission for subscriber from publisher side.
- subscribing/unsubscribing from subscriber side.
results in a scenario where a subscription permission update of
`not_allowed` being sent and on a re-subscribe, an `allowed` update does
not happen.

It happens like so
- Subscription revoke cloes the down track of subscriber.
- The subscription is still desired.
- So, a subscription reconcile runs and sees `permission: false`. This
  sends subscription permission of `not_allowed`.
- Unsubscribe request comes in and sets `desired: false`.
- Reconsiler runs again and sees `desired: false` and `subscribedTrack:
  nil`. This cleans up the subscription.
- Publisher grants permission for the subscriber.
- Subscriber subscribes to the track again. A new subscription is
  created.
- Reconciler runs and sees `permission: true`, but there is no
  permission change as it is a new subscription object. So, `allowed`
  subscription permission update is not sent and the client is stuck at
  `not_allowed`.

Fix, maintain if permission has been initialized. Has the effect of
sending an initial update which should be fine.

* clean up comment

* no default
This commit is contained in:
Raja Subramanian
2024-03-22 23:22:20 +05:30
committed by GitHub
parent ffb831aa8c
commit 95f5c94b4d
+17 -18
View File
@@ -497,8 +497,6 @@ func (m *SubscriptionManager) subscribe(s *trackSubscription) error {
s.setPublisher(res.PublisherIdentity, res.PublisherID)
// since hasPermission defaults to true, we will want to send a message to the client the first time
// that we discover permissions were denied
permChanged := s.setHasPermission(res.HasPermission)
if permChanged {
m.params.Participant.SubscriptionPermissionUpdate(s.getPublisherID(), trackID, res.HasPermission)
@@ -722,19 +720,20 @@ type trackSubscription struct {
trackID livekit.TrackID
logger logger.Logger
lock sync.RWMutex
desired bool
publisherID livekit.ParticipantID
publisherIdentity livekit.ParticipantIdentity
settings *livekit.UpdateTrackSettings
changedNotifier types.ChangeNotifier
removedNotifier types.ChangeNotifier
hasPermission bool
subscribedTrack types.SubscribedTrack
eventSent atomic.Bool
numAttempts atomic.Int32
bound bool
kind atomic.Pointer[livekit.TrackType]
lock sync.RWMutex
desired bool
publisherID livekit.ParticipantID
publisherIdentity livekit.ParticipantIdentity
settings *livekit.UpdateTrackSettings
changedNotifier types.ChangeNotifier
removedNotifier types.ChangeNotifier
hasPermissionInitialized bool
hasPermission bool
subscribedTrack types.SubscribedTrack
eventSent atomic.Bool
numAttempts atomic.Int32
bound bool
kind atomic.Pointer[livekit.TrackType]
// the later of when subscription was requested OR when the first failure was encountered OR when permission is granted
// this timestamp determines when failures are reported
@@ -746,8 +745,6 @@ func newTrackSubscription(subscriberID livekit.ParticipantID, trackID livekit.Tr
subscriberID: subscriberID,
trackID: trackID,
logger: l,
// default allow
hasPermission: true,
}
}
@@ -796,9 +793,11 @@ func (s *trackSubscription) setDesired(desired bool) bool {
func (s *trackSubscription) setHasPermission(perm bool) bool {
s.lock.Lock()
defer s.lock.Unlock()
if s.hasPermission == perm {
if s.hasPermissionInitialized && s.hasPermission == perm {
return false
}
s.hasPermissionInitialized = true
s.hasPermission = perm
if s.hasPermission {
// when permission is granted, reset the timer so it has sufficient time to reconcile