Skip to content

Commit

Permalink
Fix theoretical memory leak on reload in stream PlaybackObserver crea…
Browse files Browse the repository at this point in the history
…tion code

When working on the WebWorker branch (#1272), I noticed a (small) memory leak
where a created PlaybackObserver could still be running after a reload
for the same content, whereas it should not.

This removes that leak by ensuring all logic is cleaned-up on each
reload.

However, we do have memory-leak tests checking for leaks in that area
and it didn't find anything, running the corresponding test before and after
the commit is applied seems to result in roughly the same JS heap size.
We may have to look if the test does not do what we want it to do or if the
leak in question is very insignifance/does not apply in our current code.
  • Loading branch information
peaBerberian committed Sep 25, 2023
1 parent 0da1a8a commit b3f3a1b
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 14 deletions.
7 changes: 4 additions & 3 deletions src/core/init/media_source_content_initializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -445,13 +445,14 @@ export default class MediaSourceContentInitializer extends ContentInitializer {
}
}, { clearSignal: cancelSignal, emitCurrentValue: true });

const streamObserver = createStreamPlaybackObserver(manifest,
playbackObserver,
const streamObserver = createStreamPlaybackObserver(playbackObserver,
{ autoPlay,
manifest,
initialPlayPerformed,
initialSeekPerformed,
speed,
startTime: initialTime });
startTime: initialTime },
cancelSignal);

const rebufferingController = this._createRebufferingController(playbackObserver,
manifest,
Expand Down
29 changes: 18 additions & 11 deletions src/core/init/utils/create_stream_playback_observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ import Manifest from "../../../manifest";
import createSharedReference, {
IReadOnlySharedReference,
} from "../../../utils/reference";
import { CancellationSignal } from "../../../utils/task_canceller";
import TaskCanceller, {
CancellationSignal,
} from "../../../utils/task_canceller";
import {
IPlaybackObservation,
IReadOnlyPlaybackObserver,
Expand All @@ -30,6 +32,8 @@ import { IStreamOrchestratorPlaybackObservation } from "../../stream";
export interface IStreamPlaybackObserverArguments {
/** If true, the player will auto-play when `initialPlayPerformed` becomes `true`. */
autoPlay : boolean;
/** Manifest of the content being played */
manifest : Manifest;
/** Becomes `true` after the initial play has been taken care of. */
initialPlayPerformed : IReadOnlySharedReference<boolean>;
/** Becomes `true` after the initial seek has been taken care of. */
Expand All @@ -42,34 +46,37 @@ export interface IStreamPlaybackObserverArguments {

/**
* Create PlaybackObserver for the `Stream` part of the code.
* @param {Object} manifest
* @param {Object} playbackObserver
* @param {Object} srcPlaybackObserver
* @param {Object} args
* @returns {Object}
*/
export default function createStreamPlaybackObserver(
manifest : Manifest,
playbackObserver : PlaybackObserver,
srcPlaybackObserver : PlaybackObserver,
{ autoPlay,
initialPlayPerformed,
initialSeekPerformed,
manifest,
speed,
startTime } : IStreamPlaybackObserverArguments
startTime } : IStreamPlaybackObserverArguments,
fnCancelSignal : CancellationSignal
) : IReadOnlyPlaybackObserver<IStreamOrchestratorPlaybackObservation> {
return playbackObserver.deriveReadOnlyObserver(function transform(
return srcPlaybackObserver.deriveReadOnlyObserver(function transform(
observationRef : IReadOnlySharedReference<IPlaybackObservation>,
cancellationSignal : CancellationSignal
parentObserverCancelSignal : CancellationSignal
) : IReadOnlySharedReference<IStreamOrchestratorPlaybackObservation> {
const canceller = new TaskCanceller();
canceller.linkToSignal(parentObserverCancelSignal);
canceller.linkToSignal(fnCancelSignal);
const newRef = createSharedReference(constructStreamPlaybackObservation(),
cancellationSignal);
canceller.signal);

speed.onUpdate(emitStreamPlaybackObservation, {
clearSignal: cancellationSignal,
clearSignal: canceller.signal,
emitCurrentValue: false,
});

observationRef.onUpdate(emitStreamPlaybackObservation, {
clearSignal: cancellationSignal,
clearSignal: canceller.signal,
emitCurrentValue: false,
});
return newRef;
Expand Down

0 comments on commit b3f3a1b

Please sign in to comment.