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

https.Server does not call http.close #48373

Closed
mm-mbinder opened this issue Jun 7, 2023 · 1 comment · Fixed by #48383
Closed

https.Server does not call http.close #48373

mm-mbinder opened this issue Jun 7, 2023 · 1 comment · Fixed by #48383

Comments

@mm-mbinder
Copy link

mm-mbinder commented Jun 7, 2023

Version

v18.7.0

Platform

Linux ubuntu 5.15.0-73-generic #80~20.04.1-Ubuntu SMP Wed May 17 14:58:14 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

https

What steps will reproduce the bug?

Running this script:

const http = require('http');
const https = require('https');

const httpServer = http.createServer({});
const httpsServer = https.createServer({});

httpServer.listen(9900, () => {
        console.log('HTTP Server before close:', httpServer);

        httpServer.close(() => {
                console.log('HTTP Server after close:', httpServer);
        });
});

httpsServer.listen(9901, () => {
        console.log('HTTPS Server before close:', httpsServer);

        httpsServer.close(() => {
                console.log('HTTPS Server after close:', httpsServer);
        });
});

How often does it reproduce? Is there a required condition?

It occurs on Node Versions >= 18 as far as i can tell.

What is the expected behavior? Why is that the expected behavior?

Calling https.Server.close should also call http.Server.close to cleanup the connectionsCheckingInterval (at least that's what the docs are suggesting).

What do you see instead?

The connectionsCheckingInterval of an https.Server is not cleaned up.
See this output:

HTTP Server before close: <ref *1> Server {
  // ...
  [Symbol(http.server.connectionsCheckingInterval)]: Timeout {
    _idleTimeout: 30000,
    _idlePrev: [Timeout],
    _idleNext: [TimersList],
    _idleStart: 23,
    _onTimeout: [Function: bound checkConnections],
    _timerArgs: undefined,
    _repeat: 30000,
    _destroyed: false,
    [Symbol(refed)]: false,
    [Symbol(kHasPrimitive)]: false,
    [Symbol(asyncId)]: 2,
    [Symbol(triggerId)]: 1
  },
  [Symbol(kUniqueHeaders)]: null
}
HTTPS Server before close: <ref *1> Server {
  // ...
  [Symbol(http.server.connectionsCheckingInterval)]: Timeout {
    _idleTimeout: 30000,
    _idlePrev: [TimersList],
    _idleNext: [TimersList],
    _idleStart: 52,
    _onTimeout: [Function: bound checkConnections],
    _timerArgs: undefined,
    _repeat: 30000,
    _destroyed: false,
    [Symbol(refed)]: false,
    [Symbol(kHasPrimitive)]: false,
    [Symbol(asyncId)]: 3,
    [Symbol(triggerId)]: 1
  }
}
HTTP Server after close: Server {
  // ...
  [Symbol(http.server.connectionsCheckingInterval)]: Timeout {
    _idleTimeout: -1,
    _idlePrev: null,
    _idleNext: null,
    _idleStart: 23,
    _onTimeout: null,
    _timerArgs: undefined,
    _repeat: 30000,
    _destroyed: true, // <--- expected
    [Symbol(refed)]: false,
    [Symbol(kHasPrimitive)]: false,
    [Symbol(asyncId)]: 2,
    [Symbol(triggerId)]: 1
  },
  [Symbol(kUniqueHeaders)]: null
}
HTTPS Server after close: Server {
  // ...
  [Symbol(http.server.connectionsCheckingInterval)]: Timeout {
    _idleTimeout: 30000,
    _idlePrev: [TimersList],
    _idleNext: [TimersList],
    _idleStart: 52,
    _onTimeout: [Function: bound checkConnections],
    _timerArgs: undefined,
    _repeat: 30000,
    _destroyed: false, // <--- unexpected
    [Symbol(refed)]: false,
    [Symbol(kHasPrimitive)]: false,
    [Symbol(asyncId)]: 3,
    [Symbol(triggerId)]: 1
  }
}

Additional information

No response

@Linkgoron
Copy link
Member

Linkgoron commented Jun 7, 2023

I think I see the issue, I'll (hopefully) take care of it

Linkgoron added a commit to Linkgoron/node that referenced this issue Jun 7, 2023
The connection interval should close when httpsServer.close is called
similarly to how it gets cleared when httpServer.close is called.

fixes: nodejs#48373
nodejs-github-bot pushed a commit that referenced this issue Jun 12, 2023
The connection interval should close when httpsServer.close is called
similarly to how it gets cleared when httpServer.close is called.

fixes: #48373
PR-URL: #48383
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
RafaelGSS pushed a commit that referenced this issue Jul 3, 2023
The connection interval should close when httpsServer.close is called
similarly to how it gets cleared when httpServer.close is called.

fixes: #48373
PR-URL: #48383
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this issue Aug 14, 2023
The connection interval should close when httpsServer.close is called
similarly to how it gets cleared when httpServer.close is called.

fixes: nodejs#48373
PR-URL: nodejs#48383
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this issue Aug 14, 2023
The connection interval should close when httpsServer.close is called
similarly to how it gets cleared when httpServer.close is called.

fixes: nodejs#48373
PR-URL: nodejs#48383
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
ruyadorno pushed a commit that referenced this issue Sep 7, 2023
The connection interval should close when httpsServer.close is called
similarly to how it gets cleared when httpServer.close is called.

fixes: #48373
PR-URL: #48383
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
sparist pushed a commit to Distributive-Network/node that referenced this issue Sep 13, 2023
The connection interval should close when httpsServer.close is called
similarly to how it gets cleared when httpServer.close is called.

fixes: nodejs#48373
PR-URL: nodejs#48383
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
sparist pushed a commit to Distributive-Network/node that referenced this issue Sep 13, 2023
The connection interval should close when httpsServer.close is called
similarly to how it gets cleared when httpServer.close is called.

fixes: nodejs#48373
PR-URL: nodejs#48383
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
H4ad pushed a commit to H4ad/node that referenced this issue Sep 14, 2023
The connection interval should close when httpsServer.close is called
similarly to how it gets cleared when httpServer.close is called.

fixes: nodejs#48373
PR-URL: nodejs#48383
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
H4ad pushed a commit to H4ad/node that referenced this issue Oct 16, 2023
The connection interval should close when httpsServer.close is called
similarly to how it gets cleared when httpServer.close is called.

fixes: nodejs#48373
H4ad pushed a commit to H4ad/node that referenced this issue Oct 16, 2023
The connection interval should close when httpsServer.close is called
similarly to how it gets cleared when httpServer.close is called.

fixes: nodejs#48373

Backport-PR-URL: nodejs#50194
targos pushed a commit that referenced this issue Nov 27, 2023
The connection interval should close when httpsServer.close is called
similarly to how it gets cleared when httpServer.close is called.

Fixes: #48373
PR-URL: #48383
Backport-PR-URL: #50194
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
sercher added a commit to sercher/graaljs that referenced this issue Apr 25, 2024
The connection interval should close when httpsServer.close is called
similarly to how it gets cleared when httpServer.close is called.

Fixes: nodejs/node#48373
PR-URL: nodejs/node#48383
Backport-PR-URL: nodejs/node#50194
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
sercher added a commit to sercher/graaljs that referenced this issue Apr 25, 2024
The connection interval should close when httpsServer.close is called
similarly to how it gets cleared when httpServer.close is called.

Fixes: nodejs/node#48373
PR-URL: nodejs/node#48383
Backport-PR-URL: nodejs/node#50194
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
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

Successfully merging a pull request may close this issue.

2 participants