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

httpresponse pause()/resume() leads to out-of-memory error when using local server #2074

Closed
gyunaev opened this issue Jul 25, 2019 · 6 comments

Comments

@gyunaev
Copy link

gyunaev commented Jul 25, 2019

  • Node.js Version 8.15.2:
  • Linux OpenSuSE LEAP 15:

Consider the following HTTP client which receives HTTP data stream using data event. This stream contains commands which have to be executed in a sequence. Thus the client pauses the receiver while processing the received data buffer, and then resumes it after it is ready again.
Here is the sample client code, where processing delay is implemented using timeouts:

const http = require('http');
let agent_options = { host:  "127.0.0.1", port: 7777, path: "/", method: 'GET' };
let request = http.request( agent_options, function(res) {
   
    res.on('data', (data)=> {
       
        res.pause();
        setTimeout( ()=>{ res.resume(); }, 1 );
    });
});
request.end();

setInterval( ()=>{ console.log( "Memory use: %sMb", process.memoryUsage().heapUsed >> 20); }, 1000 );

This works fine with the server on Internet. However with a server running locally it starts eating memory, and very fast runs out of memory and crashes:

node test.js
Memory use: 47Mb
Memory use: 93Mb
Memory use: 140Mb
Memory use: 189Mb
Memory use: 235Mb
Memory use: 283Mb
Memory use: 340Mb
Memory use: 385Mb
Memory use: 429Mb
Memory use: 487Mb
Memory use: 526Mb
Memory use: 574Mb

Here is an example of the simple server which reproduces the issue:

const http = require('http');
const { Readable } = require("stream");

const zerostream = new Readable({
    read(size) {
        this.push( Buffer.alloc( 128 ) );
    }
});

// Create the web service
const webserver = http.createServer( function( request, response ) {
    request.on('data', (chunk) => {} );

    request.on('end', () => {
        
        response.writeHead( 200, { 'Content-Type' : 'application/octet-stream', 'Transfer-Encoding' : 'chunked' } );
        zerostream.pipe( response );
    });
});
        
webserver.listen( { "host" : "127.0.0.1", "port" : 7777 } );
@Hakerh400
Copy link

Node.js Version 8.15.2

That version does not exist.

This works fine with the server on Internet. However with a server running locally it starts eating memory, and very fast runs out of memory and crashes

Tested using v12.6.0 and unable to reproduce the issue. The memory usage stays at 3MB.

@gyunaev
Copy link
Author

gyunaev commented Jul 25, 2019

I have also tested it on Node 12.7.0 and it does not reproduce there:

../node-v12.7.0-linux-x64/bin/node q.js
Memory use: 3Mb
Memory use: 3Mb
Memory use: 3Mb
Memory use: 4Mb

However it DOES reproduce on Node v10.16.0:

../node-v10.16.0-linux-x64/bin/node q.js
Memory use: 58Mb
Memory use: 108Mb
Memory use: 163Mb
Memory use: 211Mb
Memory use: 256Mb
Memory use: 310Mb

And it also reproduces on was 8.15.1 (my typo).

@Hakerh400
Copy link

Hakerh400 commented Jul 26, 2019

Thanks for the update. This is indeed the same issue as nodejs/node#26957, which is fixed in nodejs/node#26965 (v12.x). Backport requests to v8.x and v10.x have been raised, but have not landed to these versions yet (see reasons why). There is v10.16.1 proposal, which includes the commit 274b97c that fixed the issue, but it is still not merged.

@gyunaev
Copy link
Author

gyunaev commented Jul 26, 2019

Thank you very much for the explanation! Here is my workaround which works fine on node 8.x and 10.x, can you please let me know whether it is valid workaround or there are some underwater issues here?

const http = require('http');
let agent_options = { host:  "127.0.0.1", port: 7777, path: "/", method: 'GET' };
let request = http.request( agent_options, function(res) {
   
    const writableStream = new Writable({
        
        write(data, encoding, callback) {
           setTimeout( ()=>{ callback(); }, 1 );
            return true;
        }
    });

    response.pipe( writableStream );
});
request.end();

setInterval( ()=>{ console.log( "Memory use: %sMb", process.memoryUsage().heapUsed >> 20); }, 1000 );

@PoojaDurgad
Copy link

@gyunaev , is this issue still outstanding? looks like this can be closed now. let me know if that is not the case

@gyunaev
Copy link
Author

gyunaev commented Sep 21, 2020

No, it no longer exists in the latest versions of node.js, thanks!

@gyunaev gyunaev closed this as completed Sep 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants