fix: ensure all tracks are stopped when disposing a Publisher (#1677)

Continuation on #1676. We were still leaking tracks in iOS Safari.

### Implementation notes

Aggressively keep track of cloned tracks in Publisher and ensure all of
them are stopped when it is disposed of.

Author: oliverlaz

packages/client/src/rtc/Publisher.ts

@@ -31,6 +31,7 @@ export type PublisherConstructorOpts = BasePeerConnectionOpts & {
  */
 export class Publisher extends BasePeerConnection {
   private readonly transceiverCache = new TransceiverCache();
+  private readonly clonedTracks = new Set<MediaStreamTrack>();
   private publishOptions: PublishOption[];
 
   /**
@@ -79,6 +80,7 @@ export class Publisher extends BasePeerConnection {
   dispose() {
     super.dispose();
     this.stopAllTracks();
+    this.clonedTracks.clear();
   }
 
   /**
@@ -100,15 +102,15 @@ export class Publisher extends BasePeerConnection {
 
       // create a clone of the track as otherwise the same trackId will
       // appear in the SDP in multiple transceivers
-      const trackToPublish = track.clone();
+      const trackToPublish = this.cloneTrack(track);
 
       const transceiver = this.transceiverCache.get(publishOption);
       if (!transceiver) {
         this.addTransceiver(trackToPublish, publishOption);
       } else {
         const previousTrack = transceiver.sender.track;
         await transceiver.sender.replaceTrack(trackToPublish);
-        previousTrack?.stop();
+        this.stopTrack(previousTrack);
       }
     }
   };
@@ -153,7 +155,7 @@ export class Publisher extends BasePeerConnection {
 
       // take the track from the existing transceiver for the same track type,
       // clone it and publish it with the new publish options
-      const track = item.transceiver.sender.track!.clone();
+      const track = this.cloneTrack(item.transceiver.sender.track!);
       this.addTransceiver(track, publishOption);
     }
 
@@ -167,7 +169,7 @@ export class Publisher extends BasePeerConnection {
       );
       if (hasPublishOption) continue;
       // it is safe to stop the track here, it is a clone
-      transceiver.sender.track?.stop();
+      this.stopTrack(transceiver.sender.track);
       await transceiver.sender.replaceTrack(null);
     }
   };
@@ -209,7 +211,7 @@ export class Publisher extends BasePeerConnection {
     for (const item of this.transceiverCache.items()) {
       const { publishOption, transceiver } = item;
       if (!trackTypes.includes(publishOption.trackType)) continue;
-      transceiver.sender.track?.stop();
+      this.stopTrack(transceiver.sender.track);
     }
   };
 
@@ -218,7 +220,10 @@ export class Publisher extends BasePeerConnection {
    */
   stopAllTracks = () => {
     for (const { transceiver } of this.transceiverCache.items()) {
-      transceiver.sender.track?.stop();
+      this.stopTrack(transceiver.sender.track);
+    }
+    for (const track of this.clonedTracks) {
+      this.stopTrack(track);
     }
   };
 
@@ -433,4 +438,16 @@ export class Publisher extends BasePeerConnection {
       publishOptionId: publishOption.id,
     };
   };
+
+  private cloneTrack = (track: MediaStreamTrack): MediaStreamTrack => {
+    const clone = track.clone();
+    this.clonedTracks.add(clone);
+    return clone;
+  };
+
+  private stopTrack = (track: MediaStreamTrack | null | undefined) => {
+    if (!track) return;
+    track.stop();
+    this.clonedTracks.delete(track);
+  };
 }

packages/client/src/rtc/__tests__/Publisher.test.ts

@@ -116,6 +116,7 @@ describe('Publisher', () => {
           },
         ],
       });
+      expect(publisher['clonedTracks'].size).toBe(1);
     });
 
     it('should update an existing transceiver for a new track', async () => {
@@ -597,10 +598,22 @@ describe('Publisher', () => {
       );
       vi.spyOn(inactiveTrack, 'readyState', 'get').mockReturnValue('ended');
 
+      const audioTransceiver = new RTCRtpTransceiver();
+      const audioTrack = new MediaStreamTrack();
+      vi.spyOn(audioTrack, 'kind', 'get').mockReturnValue('audio');
+      vi.spyOn(audioTrack, 'enabled', 'get').mockReturnValue(true);
+      vi.spyOn(audioTransceiver.sender, 'track', 'get').mockReturnValue(
+        audioTrack,
+      );
+
       // @ts-expect-error incomplete data
       cache.add({ trackType: TrackType.VIDEO, id: 1 }, transceiver);
       // @ts-expect-error incomplete data
       cache.add({ trackType: TrackType.VIDEO, id: 2 }, inactiveTransceiver);
+      // @ts-expect-error incomplete data
+      cache.add({ trackType: TrackType.AUDIO, id: 3 }, audioTransceiver);
+
+      publisher['clonedTracks'].add(track).add(inactiveTrack).add(audioTrack);
     });
 
     it('negotiate should set up the local and remote descriptions', async () => {
@@ -666,13 +679,13 @@ describe('Publisher', () => {
 
     it('getPublishedTracks returns the published tracks', () => {
       const tracks = publisher.getPublishedTracks();
-      expect(tracks).toHaveLength(1);
+      expect(tracks).toHaveLength(2);
       expect(tracks[0].readyState).toBe('live');
     });
 
     it('getAnnouncedTracks should return all tracks', () => {
       const trackInfos = publisher.getAnnouncedTracks('');
-      expect(trackInfos).toHaveLength(2);
+      expect(trackInfos).toHaveLength(3);
       expect(trackInfos[0].muted).toBe(false);
       expect(trackInfos[0].mid).toBe('0');
       expect(trackInfos[1].muted).toBe(true);
@@ -701,15 +714,19 @@ describe('Publisher', () => {
     it('stopTracks should stop tracks', () => {
       const track = cache['cache'][0].transceiver.sender.track;
       vi.spyOn(track, 'stop');
+      expect(publisher['clonedTracks'].size).toBe(3);
       publisher.stopTracks(TrackType.VIDEO);
       expect(track!.stop).toHaveBeenCalled();
+      expect(publisher['clonedTracks'].size).toBe(1);
     });
 
     it('stopAllTracks should stop all tracks', () => {
       const track = cache['cache'][0].transceiver.sender.track;
       vi.spyOn(track, 'stop');
+      expect(publisher['clonedTracks'].size).toBe(3);
       publisher.stopAllTracks();
       expect(track!.stop).toHaveBeenCalled();
+      expect(publisher['clonedTracks'].size).toBe(0);
     });
   });
 });