videojs / m3u8-parser

An m3u8 parser.
Other
471 stars 98 forks source link

Fixable memory leak in Chromium based browsers #164

Open zsilbi opened 1 year ago

zsilbi commented 1 year ago

Hi!

I found a nice memory leak issue in Chromium based browsers due to a V8 engine string slicing problem. More about it here: https://bugs.chromium.org/p/v8/issues/detail?id=2869

This causes a longer DVR live stream (tested with 10 hours long) to crash in 30-40 minutes in video.js 7+ due to extreme memory consuption.

235371930-1a7810ff-a42c-4610-aa49-d4a21186c960

235372738-9bb0a8fc-8502-4376-aae8-2075b417d5f6

A possible solution is just a small hack that's required for every parsed string.

For example:

if (line[0] !== "#") {
  this.trigger("data", {
    type: "uri",
    uri: (" " + line).substring(1), // <-------
  });
  return;
}
match = /^#EXT-X-PROGRAM-DATE-TIME:(.*)$/.exec(newLine);

if (match) {
  event = {
    type: 'tag',
    tagType: 'program-date-time'
  };

  if (match[1]) {
    event.dateTimeString = (" " + match[1]).substring(1);  // <-------
    event.dateTimeObject = new Date(match[1]);
  }

  this.trigger('data', event);
  return;
}

These modifications reduced the players memory consuption from 2 GB to 30 MB.

Best regards, zsilbi

gkatsev commented 1 year ago

Thanks for the detailed issue! We've definitely had this come up before, but we didn't realize that the issue was the sliced strings!

Since we use RegExp to parse the m3u8, I wonder if the /./.exec("a") trick is the more "correct" one to fix this?

zsilbi commented 1 year ago

Since we use RegExp to parse the m3u8, I wonder if the /./.exec("a") trick is the more "correct" one to fix this?

I saw that, but I didn't have time to test that. To be honest I am in hurry atm, so after I had patched my parser I moved on to finish the stuff I am working on. Hopefully next week I can take a look at that.

I did one more test at night with a 12 hour DVR stream to make sure everything is OK and the heap snapshot was 26.8 MB in the morning (for the whole project).

gkatsev commented 1 year ago

Running a quick test locally with the exec thing, but I'm not sure that the DVR streams I have access too are long enough to make a big different.

It's possible that doing both would be even better, who knows. After your event, if you have local changes and are able to submit a PR, that would be amazing!

zsilbi commented 1 year ago

Running a quick test locally with the exec thing, but I'm not sure that the DVR streams I have access too are long enough to make a big different.

I sent you an email with my development stream test url. If you open the link and take a heap snapshot you'll see this:

image

When the DVR stream is running live, a big raw m3u8 string appears after each parse, that's why it fills the memory so fast.

gkatsev commented 1 year ago

Thanks!