http2: server.close not behaving like http1 or net

[mcollina]

• Version: 8, 9, 10, master
• Platform: Mac OS X
• Subsystem: http2

An http2 server inherits from net.Server, so a user might expect that the behavior of close() is the same (and we document it as being the same). However, the following will never call the close(cb)  callback.

'use strict';

const http2 = require('http2');
const assert = require('assert');

const server = http2.createServer();
let client;

server.listen(0, function() {
  client = http2.connect(`http://localhost:${server.address().port}`);
  client.on('connect', function() {
    console.log('connect');

    server.close(function() {
      console.log('the close callback');
    });
  });
});

server.on('session', (s) => {
  s.setTimeout(10, function () {
    console.log('timeout');
    s.destroy();
  });
});

This is the same code for net that works as expected:

'use strict';

const net = require('net');
const assert = require('assert');

const server = net.createServer();
let client;

server.listen(0, function() {
  client = net.connect(server.address().port);
  client.on('connect', function() {
    console.log('connect');

    server.close(function() {
      console.log('the close callback');
    });
  });
});

server.on('connection', (s) => {
  s.setTimeout(10, function () {
    console.log('timeout');
    s.destroy();
  });
});

[jasnell]

Yep, able to reproduce, not sure why it's happening yet. Will investigate!

[apapirovski]

Hmmm... I get ECONNRESET pretty consistently but the callback definitely does run.

(node:69948) ExperimentalWarning: The http2 module is an experimental API.
connect
the close callback
events.js:167
      throw er; // Unhandled 'error' event
      ^

Error: read ECONNRESET
    at TCP.onread (net.js:659:25)
Emitted 'error' event at:
    at emitClose (internal/http2/core.js:810:10)
    at process._tickCallback (internal/process/next_tick.js:63:19)

Something like this replicates it for me reliably though:

'use strict';

const http2 = require('http2');
const assert = require('assert');

const server = http2.createServer();
let client;

server.listen(0, function() {
  client = http2.connect(`http://localhost:${server.address().port}`);
  client.on('connect', function() {
    console.log('connect');
  });
});

server.on('session', (s) => {
  s.destroy();
  server.close(function() {
    console.log('the close callback');
  });
});

[mcollina]

@apapirovski the bug is about calling server.close() before session.destroy().

[apapirovski]

@mcollina That's not strictly true. The test case above is a more reliable way to test the same issue. To sum it up: the close callback is dependent upon sockets being destroyed which they currently aren't.

It's fixed by this PR: #19852

[mcollina]

@apapirovski it's nice it ends up being a PR that I've not finished yet. I completely forgot about that one. Thanks.

[apapirovski]

Fixed in #19852