jimhigson / oboe.js

A streaming approach to JSON. Oboe.js speeds up web applications by providing parsed objects before the response completes.
http://oboejs.com
Other
4.77k stars 209 forks source link

Memory leak on subscription #241

Open abramenko95 opened 2 years ago

abramenko95 commented 2 years ago

I am trying to use Oboe.js to subscription, receive and parse server data. I have problem with growing string inside function handleProgress() in file oboe-browser.js. In an hour and a half, the string grows from 1.4Mb to 150Mb. In a day, application (tab in browser) crashed. I am trying to use oboe.drop(), but it didn't work for me. I attach example of code and snapshots of memory samplings.

let subscription = oboe({
    url: 'SOME URL',
    cached: false,
  })
  .start(() => {
    // SOME COMMENTED CODE
  })
  .node('!.*', (status) => {
    // SOME COMMENTED CODE
    return oboe.drop();
  }).on('fail', function (error) {
    // SOME COMMENTED CODE
    return oboe.drop();
  });

snapshot2 spanshot1 Somebody know how fix this problem?

Aigeec commented 2 years ago

What broswer and browser version is this? There can be weird behaviour in certain browsers where the responseText from the xhr requests is the entire response rather than the newly received part.

What is the use case here, site sits there consuming streamed data forever?

abramenko95 commented 2 years ago

What broswer and browser version is this? There can be weird behaviour in certain browsers where the responseText from the xhr requests is the entire response rather than the newly received part.

What is the use case here, site sits there consuming streamed data forever?

Browser Google Chrome 95.0.4638.69. Site consume actual receive streaming data (data from last response). I tried remove all operations with this data to avoid links from another variables for correct garbage collection, but it doesn't fix my problem. The memory used increases even when I use a subscription without handlers.

Aigeec commented 2 years ago

So from what I can see doing some quick testing with xhr.responseText on chrome 95.0.4638.69 it seems it continues to grow over time. There is a stackoverflow post about it here https://stackoverflow.com/questions/21951175/comet-xmlhttprequest-responsetext-grows-endlessly.

function handleProgress () {
    if (String(xhr.status)[0] === '2') {
      var textSoFar = xhr.responseText
      var newText = (' ' + textSoFar.substr(numberOfCharsAlreadyGivenToCallback)).substr(1)

      /* Raise the event for new text.

       On older browsers, the new text is the whole response.
       On newer/better ones, the fragment part that we got since
       last progress. */

      if (newText) {
        emitStreamData(newText)
      }

      numberOfCharsAlreadyGivenToCallback = len(textSoFar)
    }
  }

So if xhr.responseText was huge, then it would be assigned to textSoFar which might explain the memory usage. will keep looking into it.

image

So if you could check your heap snaptshot and just check the path for those strings, I believe you'll see that it's all in the xhr object.

Aigeec commented 2 years ago

What broswer and browser version is this? There can be weird behaviour in certain browsers where the responseText from the xhr requests is the entire response rather than the newly received part. What is the use case here, site sits there consuming streamed data forever?

Browser Google Chrome 95.0.4638.69. Site consume actual receive streaming data (data from last response). I tried remove all operations with this data to avoid links from another variables for correct garbage collection, but it doesn't fix my problem. The memory used increases even when I use a subscription without handlers.

What version of Oboe are you using?

nikkov commented 2 years ago

What version of Oboe are you using?

2.1.4 As it seems to me the problem is described in this issue: https://github.com/grpc-ecosystem/grpc-gateway/issues/1783 and we are also use grpc-gateway with server-stream. A possible solution is to use in oboe the fetch API, but the pull request is still not applied. #

paulsmithkc commented 2 years ago

@Aigeec From what I understand xhr.responseText is supposed to contain the entire content of the response, per the official specification.