Skip to content

Commit

Permalink
Fix MultiThread debug element memory leak
Browse files Browse the repository at this point in the history
I saw a memory leak which may (still under tests) be the source of my
recent failing tests on LG TVs (the application restarted after one or
two hours of playback).

It only appears in a combination of experimental features, so not
dramatic either:
  - multithread is enabled
  - the RxPlayer's debug element is shown on screen

I noticed that the create Promise's `reject` callback was correctly
linked to the `Init`'s CancellationSignal but was never unbound from it.

As such, that `CancellationSignal`, which is alive as long as the
content is playing, would keep a reference to that reject's function,
and this seems to extend to other objects linked to that Promise. I was
for example under the impression that even that Promise's resolved value
was retained by looking at Chrome's inspector.

The solution is simply to unbound `reject` from the CancellationSignal
when either "rejecting" or "resolving" the Promise.

---

I also profited from this fix to transform the `resolvers` object from a
simple JS Object to a `Map` as it seems much more appropriate: we
very frequently insert and remove values from it, and we never need to
serialize that object.

Also that Object's keys were number and to my understanding JS Object
keys are always stringified (or at least you cannot have e.g. `[1]`
and `["1"]` point to different values) so that didn't seem natural to
me, plus the fact that I initially feared that it was an array.
  • Loading branch information
peaBerberian committed Oct 5, 2024
1 parent 4956854 commit a6fada4
Showing 1 changed file with 15 additions and 7 deletions.
22 changes: 15 additions & 7 deletions src/main_thread/init/multi_thread_content_initializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,13 @@ export default class MultiThreadContentInitializer extends ContentInitializer {
private _currentMediaSourceCanceller: TaskCanceller;

/**
* Stores the resolvers and the current messageId that is sent to the web worker to receive segment sink metrics.
* Stores the resolvers and the current messageId that is sent to the web worker to
* receive segment sink metrics.
* The purpose of collecting metrics is for monitoring and debugging.
*/
private _segmentMetrics: {
lastMessageId: number;
resolvers: Record<number, (value: ISegmentSinkMetrics | undefined) => void>;
resolvers: Map<number, (value: ISegmentSinkMetrics | undefined) => void>;
};

/**
Expand All @@ -135,7 +136,7 @@ export default class MultiThreadContentInitializer extends ContentInitializer {
this._currentContentInfo = null;
this._segmentMetrics = {
lastMessageId: 0,
resolvers: {},
resolvers: new Map(),
};
this._queuedWorkerMessages = null;
}
Expand Down Expand Up @@ -1121,10 +1122,9 @@ export default class MultiThreadContentInitializer extends ContentInitializer {
if (this._currentContentInfo?.contentId !== msgData.contentId) {
return;
}
const resolveFn = this._segmentMetrics.resolvers[msgData.value.messageId];
const resolveFn = this._segmentMetrics.resolvers.get(msgData.value.messageId);
if (resolveFn !== undefined) {
resolveFn(msgData.value.segmentSinkMetrics);
delete this._segmentMetrics.resolvers[msgData.value.messageId];
} else {
log.error("MTCI: Failed to send segment sink store update");
}
Expand Down Expand Up @@ -1589,11 +1589,19 @@ export default class MultiThreadContentInitializer extends ContentInitializer {
value: { messageId },
});
return new Promise((resolve, reject) => {
this._segmentMetrics.resolvers[messageId] = resolve;
const rejectFn = (err: CancellationError) => {
delete this._segmentMetrics.resolvers[messageId];
cancelSignal.deregister(rejectFn);
this._segmentMetrics.resolvers.delete(messageId);
return reject(err);
};
this._segmentMetrics.resolvers.set(
messageId,
(value: ISegmentSinkMetrics | undefined) => {
cancelSignal.deregister(rejectFn);
this._segmentMetrics.resolvers.delete(messageId);
resolve(value);
},
);
cancelSignal.register(rejectFn);
});
};
Expand Down

0 comments on commit a6fada4

Please sign in to comment.