node-http2 icon indicating copy to clipboard operation
node-http2 copied to clipboard

Clean up ended streams to free leaked memory

Open argon opened this issue 8 years ago • 8 comments

Fixes #64

When a stream ends remove references in _streamIds and _streamPriorities. This allows streams to be freed by the GC. (Verified with heap profiling)

argon avatar May 01 '16 22:05 argon

LGTM 👍

JakeChampion avatar May 11 '16 21:05 JakeChampion

I am testing this in pressure with following code by cmd: node client.js 'https://localhost:8080/localhost.key':


var fs = require('fs');
var path = require('path');
var http2 = require('..');
var urlParse = require('url').parse;

// Setting the global logger (optional)
http2.globalAgent = new http2.Agent({
  rejectUnauthorized: true,
  log: require('../test/util').createLogger('client')
});

// Sending the request
var url = process.argv.pop();
var options = urlParse(url);

// Optionally verify self-signed certificates.
if (options.hostname == 'localhost') {
  options.key = fs.readFileSync(path.join(__dirname, '/localhost.key'));
  options.ca = fs.readFileSync(path.join(__dirname, '/localhost.crt'));
}

var r=0;
var finished = 0;
var max_req = 100;
var times = 10000;

function request() {
    var request = process.env.HTTP2_PLAIN ? http2.raw.get(options) : http2.get(options);
    // Receiving the response
    request.on('response', function(response) {
        response.on('data', function(data) {
        });
        response.on('end', finish);
    });
    request.on('socket', (socket) => {
        //console.log("socket event");
        //socket.emit('agentRemove');
    });
}

run_once();
// Quitting after both the response and the associated pushed resources have arrived

function run_once() {
    for (r = 0; r < max_req; r++) 
    {
        //console.log("Making request ", r);
        request();
    }
}

function finish() {
  //console.log("finished = ", finished);
  finished += 1;
  if (finished === max_req/*(1 + push_count)*/) {
    finished = 0;
    if (times-- === 0)
    {
        //process.exit();
        setInterval(function() {
            //idle for GC
//          global.gc();
            //console.log("end global agent: ", http2.globalAgent);

        }.bind(this), 1 * 1000);
    }
    else {
        run_once();
    }
    //global.gc();
    //run_once();
  }
}

but get error like this at server:(just from server.js of example)

13:36:45.274Z INFO server/http: New incoming HTTP/2 connection (e=97, client=::ffff:127.0.0.1:62351, SNI=localhost) 13:36:45.275Z INFO server/http: New incoming HTTP/2 connection (e=98, client=::ffff:127.0.0.1:62352, SNI=localhost) 13:36:45.277Z INFO server/http: New incoming HTTP/2 connection (e=99, client=::ffff:127.0.0.1:62353, SNI=localhost) 13:36:45.311Z ERROR server/connection: Invalid incoming stream ID. (e=0, stream_id=1, lastIncomingStream=199) 13:36:45.311Z FATAL server/endpoint: Fatal error, closing connection (e=0, source=connection, message=PROTOCOL_ERROR) 13:36:45.312Z ERROR server/stream: Received illegal frame. (e=0, s=100, error=PROTOCOL_ERROR, state=IDLE) frame: { "id": 0, "type": "WINDOW_UPDATE", "flags": [], "stream": 1, "window_size": 887 } ^[[A^[[A

gridnow avatar May 30 '16 13:05 gridnow

Original code with huge memory leak!

gridnow avatar May 30 '16 13:05 gridnow

Any updates on this?

argon avatar Sep 13 '16 10:09 argon

cc @nwgh

ide avatar Sep 22 '16 03:09 ide

Can we please get an update on merging this? It's been months

AndrewBarba avatar Oct 19 '16 03:10 AndrewBarba

@AndrewBarba I think none of the maintainers are active ... the last one to commit anything was @nwgh and that was in September 2016 ...

ShaharHD avatar Mar 28 '17 00:03 ShaharHD

merged https://github.com/kaazing/node-http2/commit/db0936244be639cbfd9a45c85377d0923a123b46

hthetiot avatar Aug 23 '17 19:08 hthetiot