Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

memory leak and different behavior between async-await and Promise #29118

Closed
brandonstark opened this issue Aug 14, 2019 · 6 comments
Closed
Labels
memory Issues and PRs related to the memory management or memory footprint.

Comments

@brandonstark
Copy link

brandonstark commented Aug 14, 2019

I have been trying to debug a memory leak in my code for a very long time.
Running inspect, I can see that the memory that accumulates and is never released is a Promise.
These promises are retained by nothing but "global handles". I can't understand how this promise gets to be held indefinitely by the "global handles".
Note 1. This is not a matter of time. I can wait forever, and the promises are never freed,
Note 2. This is not a problem with axios. This behavior is the same with node-fetch, and request.

There is one function in my code that when I change it from async-await to a new Promise, the behavior is different.

with async-await

 async tts(text, source) {
    try {
      const rdata = this.requestData();

      const data = xmlbuilder.create('speak')
        .att('version', rdata.version)
        .att('xml:lang', rdata.lang)
        .ele('voice')
        .att('xml:lang', rdata.lang)
        .att('xml:gender', rdata.gender)
        .att('name', `${rdata.voiceName}`)
        .txt(text)
        .end()
        .toString();

      const headers = {
        'Authorization': 'Bearer ' + ((await getToken(this._conf)).value),
        'Content-Type': 'application/ssml+xml',
        'User-Agent': 'TTSNodeJS',
        'X-Microsoft-OutputFormat': `${rdata.format}-16khz-16bit-mono-pcm`
      };

      const response = await axios({
        cancelToken: source.token,
        data,
        encoding: null,
        headers,
        method: 'post',
        responseType: 'stream',
        url: rdata.url
      });

      if (response.status !== 200) {
        throw new Error('TTS Error: ' + response.status + ': ' + response.statusText);
      }

      return response.data;
    } catch (err) {
      throw new Error('TTS Error: ' + err);
    }
  }

with new Promise

tts(text, source) {
    return new Promise((resolve, reject) => {
      getToken(this._conf).then((token) => {
        const rdata = this.requestData();

        const data = xmlbuilder.create('speak')
          .att('version', rdata.version)
          .att('xml:lang', rdata.lang)
          .ele('voice')
          .att('xml:lang', rdata.lang)
          .att('xml:gender', rdata.gender)
          .att('name', `${rdata.voiceName}`)
          .txt(text)
          .end()
          .toString();

        const headers = {
          'Authorization': 'Bearer ' + token.value,
          'Content-Type': 'application/ssml+xml',
          'User-Agent': 'TTSNodeJS',
          'X-Microsoft-OutputFormat': `${rdata.format}-16khz-16bit-mono-pcm`
        };

        axios({
            cancelToken: source.token,
            data,
            encoding: null,
            headers,
            method: 'post',
            responseType: 'stream',
            url: rdata.url
          }).then((response) => {
            if (response.status !== 200) {
              reject(new Error('TTS Error: ' + response.status + ': ' + response.statusText));
            }
            resolve(response.data);
          })
          .catch((err) => {
            reject(new Error('TTS Error: ' + err));
          });
      });
    });
  }

The difference
In both cases, using inspect, I see many Promises held only by "global handles".
With the first code, the stuck Promises have PromiseStatus "resolved", and PomiseValue is an IncomingMessage (coming from axios).
With the second code, the stuck Promises have PromiseStatus "pending", and PomiseValue undefined . This causes the leak to be much smaller, because the Promise does not retain the PomiseValue.

This is starting to look like a bug in Node. Can anybody explain the different behavior?
Can anybody explain how a promise gets to be held indefinitely by "global handles"?

Using node v10.15.3.

inspect
12 hours with async-await
with async await
19 hours with new Promise
with promise

@bnoordhuis bnoordhuis added the memory Issues and PRs related to the memory management or memory footprint. label Aug 14, 2019
@bnoordhuis
Copy link
Member

How can we reproduce? If you have a test case, please try to make it minimal and - strongly preferred - use built-in modules only, no third-party modules.

@ronag
Copy link
Member

ronag commented Aug 16, 2019

@brandonstark: can you try to do a git bisect or go back in versions to figure out when this behaviour changed?

Also can you see if you have the same problem with Node 12 (latest)?

@brandonstark
Copy link
Author

We've been trying, but can't seem to isolate the problem. Any attempt to isolate this code, results in no leak. There is probably some scenario that we don't know how to simulate. We are still trying. If we succeed I will post code that can reproduce the problem.
The problem existed since the beginning of time (since our program started working), so I'm afraid git bisect won't help.
We tried node 12, and it did not behave any differently.
I was hoping somebody would be able to make sense of the different behavior between the promise and async-await, which should be exactly the same as far as I understand.
Or maybe someone can explain what is the purpose of this "global handles", and why it would hold on to a promise, after it was resolved, and its then() was already executed.

@bnoordhuis
Copy link
Member

Global handles are persistent strong or weak references to an object. Since there's a PromiseWrap in there, it might indicate an issue with async_hooks.

I can't tell from the info you posted whether you are using them directly or indirectly but I'd start with trying to exclude them first.

@brandonstark
Copy link
Author

I figured it might be related to async_hooks. We use cls-hooked which uses async_hooks. But disabling it completely did not make any difference. Another theory was that the inspector itself uses async_hooks, or is causing the leak some other way. But with the inspector disabled I still have a leak, though I can't tell what is leaking.

@orgads
Copy link
Contributor

orgads commented Aug 28, 2019

The leak resulted from our application. It did not resume paused streams on some cases.

@brandonstark: Please close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
memory Issues and PRs related to the memory management or memory footprint.
Projects
None yet
Development

No branches or pull requests

4 participants