Skip to content

Http2Server.prototype.close Does Not Work as Expected #28214

Closed

Description

@anliting

node-v12.4.0-linux-x64/bin/node a.js

let
    http2=require('http2'),
    server=http2.createServer()
server.listen(8000,'127.0.0.1',()=>{
    let client=http2.connect('http://127.0.0.1:8000',()=>{
        console.log('listener called')
        server.close(()=>{
            console.log('close')
        })
    })
})

Expected: server.close stops the server from accepting new connections.
Result: console.log('close') never get called.

I aware that HTTP/2 works different to HTTP/1.1. And I can and should call session.close() on every active session to stop them from accepting new requests.

server.close seems also wait until there is no active sessions to call the callback function, which is not mentioned in the documentation.

And, to make there is no active sessions, I must first of all stop the server from accepting new connections.

Activity

  1. lpinca commented on Jun 30, 2019

    @lpinca
    Member

    server.close seems also wait until there is no active sessions to call the callback function, which is not mentioned in the documentation

    I guess it wasn't documented because the behavior is the same of net.Server.prototype.close(). The callback is not called until there is an open connection.

  2. added
    http2Issues and PRs related to the http2 subsystem.
    on Jun 30, 2019
  3. cjihrig commented on Jul 1, 2019

    @cjihrig
    Contributor

    I think this is behaving as documented.

  4. anliting commented on Jul 3, 2019

    @anliting
    Author

    Oh, I see, it is documented. The callback is called when the server is closed, and the server waits, until there is no active connection, to close.

    But I think the point is not whether it is documented or not:

    An asynchronous operation usually call its callback function right after the operation is done.

    As net.Server.prototype.close is documented, the operation is defined as "Stops the server from accepting new connections and keeps existing connections.". But the callback also waits until the server is closed to be called, rather than right after the operation is done.

    Of course, it is not common does not mean it is bad. But think of the following situation:

    Now I want to force a server to be closed with the following algorithm:

    1. stops the server from accepting new connections
    2. close all active connections

    The second step must be executed after the first step. As the first step is an asynchronous operation, I would wait for its callback function to be called, to ensure that it is done. But the callback is called also implies that the second step is done. It is a dead lock.

    Another approach:

    1. close all active connections
    2. stops the server from accepting new connections

    It will not work, because once the server is still accepting new connections, there is no way to close all active connections in finite amount of steps.

  5. lpinca commented on Jul 6, 2019

    @lpinca
    Member
    1. Stop the server from accepting new connections.
    2. Close all active connections.
    3. Wait for the callback.

    There is no need to wait for the callback before closing connections. No new connection is established after server.close() is called.

  6. anliting commented on Jul 7, 2019

    @anliting
    Author

    No new connection is established after server.close() is called. @lpinca

    net.Server.prototype.close:

    Stops the server from accepting new connections and keeps existing connections. This function is asynchronous, the server is finally closed when all connections are ended and the server emits a 'close' event.

    If the function is defined to "stop the server from accepting new connections and keep existing connections", and the work is done before it returns, I would rather say that it is synchronous instead of asynchronous; saying that it is asynchronous means that, only through waiting that callback, I can ensure the work is done.

    However, the function is named close, and it, in a sense, closes the server; its current behavior still makes a sense (stop ..., and wait until all connections are ended). It can be explained in the documentation, that: The closing of a server, involves

    • stoping it from accepting new connection (synchronous), and
    • waiting until all connections are ended (asynchronous); and this function does the both.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    http2Issues and PRs related to the http2 subsystem.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions