fix: Patch Memory Leaks (#218)

* Fixed leaks for custom-media-element and media-tracks

* Added leak fixes for castable-video

* Add null guards for renditionList in addRendition and removeRendition

* Restructured connectedCallback for custom media element

---------

Co-authored-by: Santiago Puppo <spuppo@mux.com>

Author: R-Delfino95

packages/castable-video/castable-mixin.js

@@ -39,6 +39,11 @@ export const CastableMediaMixin = (superclass) =>
       if (this.#remote) return this.#remote;
 
       if (requiresCastFramework()) {
+        // Don't create a new RemotePlayback when the element is disconnected.
+        // super.disconnectedCallback() may trigger property access that reaches
+        // this getter, which would re-create a RemotePlayback after cleanup.
+        if (!this.isConnected) return undefined;
+
         // No need to load the Cast framework if it's disabled.
         if (!this.disableRemotePlayback) {
           loadCastFramework();
@@ -58,6 +63,13 @@ export const CastableMediaMixin = (superclass) =>
       return privateProps.get(this.remote)?.getCastPlayer?.();
     }
 
+    disconnectedCallback() {
+      this.#remote?.destroy();
+      this.#remote = null;
+      privateProps.delete(this);
+      super.disconnectedCallback?.();
+    }
+
     attributeChangedCallback(attrName, oldValue, newValue) {
       super.attributeChangedCallback(attrName, oldValue, newValue);
 

packages/castable-video/castable-remote-playback.js

@@ -56,6 +56,7 @@ export class RemotePlayback extends EventTarget {
   #available = false;
   #callbacks = new Set();
   #callbackIds = new WeakMap();
+  #onTextTrackChange = () => this.#updateRemoteTextTrack();
 
   constructor(media) {
     super();
@@ -73,6 +74,21 @@ export class RemotePlayback extends EventTarget {
     this.#init();
   }
 
+  destroy() {
+    this.#media?.textTracks?.removeEventListener('change', this.#onTextTrackChange);
+
+    // Remove controller listeners if element is removed while casting
+    // (in that case #disconnect() won't be called).
+    if (this.#remoteListeners && this.#remotePlayer?.controller) {
+      Object.entries(this.#remoteListeners).forEach(([event, listener]) => {
+        this.#remotePlayer.controller.removeEventListener(event, listener);
+      });
+    }
+
+    if (this.#media) castElementRef.delete(this.#media);
+    this.#isInit = false;
+  }
+
   get #castPlayer() {
     if (castElementRef.has(this.#media)) return this.#remotePlayer;
     return undefined;
@@ -248,7 +264,7 @@ export class RemotePlayback extends EventTarget {
      * loaded manually in the load() method. This will require a new load() call
      * for each added/removed track w/ src.
      */
-    this.#media.textTracks.addEventListener('change', () => this.#updateRemoteTextTrack());
+    this.#media.textTracks.addEventListener('change', this.#onTextTrackChange);
 
     this.#onCastStateChanged();
 

packages/custom-media-element/custom-media-element.ts

@@ -267,6 +267,7 @@ export function CustomMediaMixin<T extends Constructor<HTMLElement>>(superclass:
     #nativeEl: HTMLVideoElement | HTMLAudioElement | null = null;
     #childMap = new Map<MediaChild, MediaChild>();
     #childObserver?: MutationObserver;
+    #slotChangeHandler?: () => void;
 
     get: ((prop: string) => any) | undefined;
     set: ((prop: string, val: any) => void) | undefined;
@@ -342,12 +343,17 @@ export function CustomMediaMixin<T extends Constructor<HTMLElement>>(superclass:
         this.#upgradeProperty(prop);
       }
 
+      this.#setupListeners();
+    }
+
+    #setupListeners(): void {
       this.#childObserver = new MutationObserver(this.#syncMediaChildAttribute.bind(this));
-      this.shadowRoot!.addEventListener('slotchange', () => this.#syncMediaChildren());
+      this.#slotChangeHandler = () => this.#syncMediaChildren();
+      this.shadowRoot?.addEventListener('slotchange', this.#slotChangeHandler);
       this.#syncMediaChildren();
 
       for (const type of (this.constructor as typeof CustomMedia).Events) {
-        this.shadowRoot!.addEventListener(type, this, true);
+        this.shadowRoot?.addEventListener(type, this, true);
       }
     }
 
@@ -446,7 +452,36 @@ export function CustomMediaMixin<T extends Constructor<HTMLElement>>(superclass:
     }
 
     connectedCallback(): void {
+      // this.#init will check this.#isInit
       this.#init();
+      // Re-mount: re-setup listeners cleaned up in disconnectedCallback.
+      // This is also done in this.init()
+      if (!this.#slotChangeHandler) {
+        this.#setupListeners();
+      }
+    }
+
+    disconnectedCallback(): void {
+      this.#childObserver?.disconnect();
+      this.#childObserver = undefined;
+
+      if (this.#slotChangeHandler) {
+        this.shadowRoot?.removeEventListener('slotchange', this.#slotChangeHandler);
+        this.#slotChangeHandler = undefined;
+      }
+
+      for (const type of (this.constructor as typeof CustomMedia).Events) {
+        this.shadowRoot?.removeEventListener(type, this, true);
+      }
+
+      this.#childMap.forEach((clone) => clone.remove());
+      this.#childMap.clear();
+
+      this.#nativeEl = null;
+      // Keep #isInit = true so that #init() is a no-op during the rest of
+      // the disconnect process. Other mixins in the chain may access properties
+      // (e.g. this.nativeEl, this.currentTime) which trigger #init() → init()
+      // and re-create the closures we just cleaned up.
     }
   };
 }

packages/media-tracks/src/audio-rendition-list.ts

@@ -4,7 +4,7 @@ import { RenditionEvent } from './rendition-event.js';
 import { getPrivate } from './utils.js';
 
 export function addRendition(track: AudioTrack, rendition: AudioRendition) {
-  const renditionList = getPrivate(track).media.audioRenditions;
+  const renditionList = getPrivate(track).media?.deref()?.audioRenditions;
 
   getPrivate(rendition).media = getPrivate(track).media;
   getPrivate(rendition).track = track;
@@ -20,28 +20,28 @@ export function addRendition(track: AudioTrack, rendition: AudioRendition) {
   }
 
   queueMicrotask(() => {
-    if (!track.enabled) return;
+    if (!renditionList || !track.enabled) return;
 
     renditionList.dispatchEvent(new RenditionEvent('addrendition', { rendition }));
   });
 }
 
 export function removeRendition(rendition: AudioRendition) {
-  const renditionList: AudioRenditionList = getPrivate(rendition).media.audioRenditions;
+  const renditionList: AudioRenditionList = getPrivate(rendition).media?.deref()?.audioRenditions;
   const track: AudioTrack = getPrivate(rendition).track;
   const renditionSet: Set<AudioRendition> = getPrivate(track).renditionSet;
   renditionSet.delete(rendition);
 
   queueMicrotask(() => {
     const track: AudioTrack = getPrivate(rendition).track;
-    if (!track.enabled) return;
+    if (!renditionList || !track.enabled) return;
 
     renditionList.dispatchEvent(new RenditionEvent('removerendition', { rendition }));
   });
 }
 
 export function selectedChanged(rendition: AudioRendition) {
-  const renditionList: AudioRenditionList = getPrivate(rendition).media.audioRenditions;
+  const renditionList: AudioRenditionList = getPrivate(rendition).media?.deref()?.audioRenditions;
 
   // Prevent firing a rendition list `change` event multiple times per tick.
   if (!renditionList || getPrivate(renditionList).changeRequested) return;
@@ -58,7 +58,8 @@ export function selectedChanged(rendition: AudioRendition) {
 }
 
 function getCurrentRenditions(renditionList: AudioRenditionList): AudioRendition[] {
-  const media: HTMLMediaElement = getPrivate(renditionList).media;
+  const media: HTMLMediaElement = getPrivate(renditionList).media?.deref();
+  if (!media) return [];
   return [...media.audioTracks]
     .filter((track: AudioTrack) => track.enabled)
     .flatMap((track: AudioTrack) => [...getPrivate(track).renditionSet]);

packages/media-tracks/src/audio-track-list.ts

@@ -4,7 +4,7 @@ import { getPrivate } from './utils.js';
 
 export function addAudioTrack(media: HTMLMediaElement, track: AudioTrack) {
   const trackList = media.audioTracks;
-  getPrivate(track).media = media;
+  getPrivate(track).media = new WeakRef(media);
 
   if (!getPrivate(track).renditionSet) {
     getPrivate(track).renditionSet = new Set();
@@ -31,7 +31,7 @@ export function addAudioTrack(media: HTMLMediaElement, track: AudioTrack) {
 }
 
 export function removeAudioTrack(track: AudioTrack) {
-  const trackList: AudioTrackList = getPrivate(track).media?.audioTracks;
+  const trackList: AudioTrackList = getPrivate(track).media?.deref()?.audioTracks;
   if (!trackList) return;
 
   const trackSet: Set<AudioTrack> = getPrivate(trackList).trackSet;
@@ -47,7 +47,7 @@ export function enabledChanged(track: AudioTrack) {
   // and whenever one that was enabled is disabled, the user agent must queue a
   // media element task given the media element to fire an event named `change`
   // at the AudioTrackList object.
-  const trackList: AudioTrackList = getPrivate(track).media.audioTracks;
+  const trackList: AudioTrackList = getPrivate(track).media?.deref()?.audioTracks;
 
   // Prevent firing a track list `change` event multiple times per tick.
   if (!trackList || getPrivate(trackList).changeRequested) return;

packages/media-tracks/src/mixin.ts

@@ -123,7 +123,7 @@ export function MediaTracksMixin<T>(MediaElementClass: T): WithMediaTracks<T> {
     let renditions = getPrivate(media).videoRenditions;
     if (!renditions) {
       renditions = new VideoRenditionList();
-      getPrivate(renditions).media = media;
+      getPrivate(renditions).media = new WeakRef(media);
       getPrivate(media).videoRenditions = renditions;
     }
     return renditions;
@@ -141,7 +141,7 @@ export function MediaTracksMixin<T>(MediaElementClass: T): WithMediaTracks<T> {
     let renditions = getPrivate(media).audioRenditions;
     if (!renditions) {
       renditions = new AudioRenditionList();
-      getPrivate(renditions).media = media;
+      getPrivate(renditions).media = new WeakRef(media);
       getPrivate(media).audioRenditions = renditions;
     }
     return renditions;
@@ -171,11 +171,11 @@ function getVideoTracks(media: any) {
         addVideoTrack(media, nativeTrack);
       }
 
-      nativeTracks.addEventListener('change', () => {
+      const onChange = () => {
         tracks.dispatchEvent(new Event('change'));
-      });
+      };
 
-      nativeTracks.addEventListener('addtrack', (event: TrackEvent) => {
+      const onAddTrack = (event: TrackEvent) => {
         // Note: adding native track instances to the shim track list here.
         // This works because the API is identical and change event is forwarded.
         // If tracks were manually added prevent native tracks from being added...
@@ -189,11 +189,15 @@ function getVideoTracks(media: any) {
         }
 
         addVideoTrack(media, event.track as VideoTrack);
-      });
+      };
 
-      nativeTracks.addEventListener('removetrack', (event: TrackEvent) => {
-        removeVideoTrack(event.track  as VideoTrack);
-      });
+      const onRemoveTrack = (event: TrackEvent) => {
+        removeVideoTrack(event.track as VideoTrack);
+      };
+
+      nativeTracks.addEventListener('change', onChange);
+      nativeTracks.addEventListener('addtrack', onAddTrack);
+      nativeTracks.addEventListener('removetrack', onRemoveTrack);
     }
   }
   return tracks;
@@ -214,11 +218,11 @@ function getAudioTracks(media: any) {
         addAudioTrack(media, nativeTrack);
       }
 
-      nativeTracks.addEventListener('change', () => {
+      const onChange = () => {
         tracks.dispatchEvent(new Event('change'));
-      });
+      };
 
-      nativeTracks.addEventListener('addtrack', (event: TrackEvent) => {
+      const onAddTrack = (event: TrackEvent) => {
         // Note: adding native track instances to the shim track list here.
         // This works because the API is identical and change event is forwarded.
         // If tracks were manually added prevent native tracks from being added...
@@ -232,11 +236,15 @@ function getAudioTracks(media: any) {
         }
 
         addAudioTrack(media, event.track as AudioTrack);
-      });
+      };
 
-      nativeTracks.addEventListener('removetrack', (event: TrackEvent) => {
+      const onRemoveTrack = (event: TrackEvent) => {
         removeAudioTrack(event.track as AudioTrack);
-      });
+      };
+
+      nativeTracks.addEventListener('change', onChange);
+      nativeTracks.addEventListener('addtrack', onAddTrack);
+      nativeTracks.addEventListener('removetrack', onRemoveTrack);
     }
   }
   return tracks;

packages/media-tracks/src/video-rendition-list.ts

@@ -4,7 +4,7 @@ import { RenditionEvent } from './rendition-event.js';
 import { getPrivate } from './utils.js';
 
 export function addRendition(track: VideoTrack, rendition: VideoRendition) {
-  const renditionList = getPrivate(track).media.videoRenditions;
+  const renditionList = getPrivate(track).media?.deref()?.videoRenditions;
 
   getPrivate(rendition).media = getPrivate(track).media;
   getPrivate(rendition).track = track;
@@ -20,28 +20,28 @@ export function addRendition(track: VideoTrack, rendition: VideoRendition) {
   }
 
   queueMicrotask(() => {
-    if (!track.selected) return;
+    if (!renditionList || !track.selected) return;
 
     renditionList.dispatchEvent(new RenditionEvent('addrendition', { rendition }));
   });
 }
 
 export function removeRendition(rendition: VideoRendition) {
-  const renditionList: VideoRenditionList = getPrivate(rendition).media.videoRenditions;
+  const renditionList: VideoRenditionList = getPrivate(rendition).media?.deref()?.videoRenditions;
   const track: VideoTrack = getPrivate(rendition).track;
   const renditionSet: Set<VideoRendition> = getPrivate(track).renditionSet;
   renditionSet.delete(rendition);
 
   queueMicrotask(() => {
     const track: VideoTrack = getPrivate(rendition).track;
-    if (!track.selected) return;
+    if (!renditionList || !track.selected) return;
 
     renditionList.dispatchEvent(new RenditionEvent('removerendition', { rendition }));
   });
 }
 
 export function selectedChanged(rendition: VideoRendition) {
-  const renditionList: VideoRenditionList = getPrivate(rendition).media.videoRenditions;
+  const renditionList: VideoRenditionList = getPrivate(rendition).media?.deref()?.videoRenditions;
 
   // Prevent firing a rendition list `change` event multiple times per tick.
   if (!renditionList || getPrivate(renditionList).changeRequested) return;
@@ -58,7 +58,8 @@ export function selectedChanged(rendition: VideoRendition) {
 }
 
 function getCurrentRenditions(renditionList: VideoRenditionList): VideoRendition[] {
-  const media: HTMLMediaElement = getPrivate(renditionList).media;
+  const media: HTMLMediaElement = getPrivate(renditionList).media?.deref();
+  if (!media) return [];
   return [...media.videoTracks]
     .filter((track: VideoTrack) => track.selected)
     .flatMap((track: VideoTrack) => [...getPrivate(track).renditionSet]);

packages/media-tracks/src/video-track-list.ts

@@ -4,7 +4,7 @@ import { getPrivate } from './utils.js';
 
 export function addVideoTrack(media: HTMLMediaElement, track: VideoTrack) {
   const trackList = media.videoTracks;
-  getPrivate(track).media = media;
+  getPrivate(track).media = new WeakRef(media);
 
   if (!getPrivate(track).renditionSet) {
     getPrivate(track).renditionSet = new Set();
@@ -31,7 +31,7 @@ export function addVideoTrack(media: HTMLMediaElement, track: VideoTrack) {
 }
 
 export function removeVideoTrack(track: VideoTrack) {
-  const trackList: VideoTrackList = getPrivate(track).media?.videoTracks;
+  const trackList: VideoTrackList = getPrivate(track).media?.deref()?.videoTracks;
   if (!trackList) return;
 
   const trackSet: Set<VideoTrack> = getPrivate(trackList).trackSet;
@@ -43,7 +43,7 @@ export function removeVideoTrack(track: VideoTrack) {
 }
 
 export function selectedChanged(selected: VideoTrack) {
-  const trackList: VideoTrackList = getPrivate(selected).media.videoTracks ?? [];
+  const trackList: VideoTrackList = getPrivate(selected).media?.deref()?.videoTracks ?? [];
   // If other tracks are unselected, then a change event will be fired.
   let hasUnselected = false;