http: fix socket re-use races by ronag · Pull Request #32000 · nodejs/node

@ronag added the http

Issues or PRs related to the http subsystem.

label

Feb 28, 2020

ronag

@ronag ronag mentioned this pull request

Feb 28, 2020

2 tasks

mcollina

@ronag ronag added semver-minor

PRs that contain new features and should be released in the next minor version.

author ready

PRs that have at least one approval, no pending requests for changes, and a CI started.

labels

Feb 28, 2020

@mcollina mcollina added author ready

PRs that have at least one approval, no pending requests for changes, and a CI started.

and removed author ready

PRs that have at least one approval, no pending requests for changes, and a CI started.

labels

Feb 28, 2020

@ronag ronag removed the semver-minor

PRs that contain new features and should be released in the next minor version.

label

Feb 28, 2020

lundibundi

ronag

@ronag ronag removed the author ready

PRs that have at least one approval, no pending requests for changes, and a CI started.

label

Feb 28, 2020

@ronag ronag changed the title http: keep alive socket re-use races http: fix socket re-use races

Feb 28, 2020

ronag

@ronag

Whether and when a socket is destroyed or not after a timeout is up to
the user. This leaves an edge case where a socket that has emitted
'timeout' might be re-used from the free pool. Even if destroy is called
on the socket, it won't be removed from the freelist until 'close' which
can happen several ticks later.

Sockets are removed from the free list on the 'close' event.
However, there is a delay between calling destroy() and 'close'
being emitted. This means that it possible for a socket that has
been destroyed to be re-used from the free list, causing unexpected
failures.

@ronag

lundibundi

@ronag

ronag added a commit that referenced this pull request

Mar 7, 2020
Whether and when a socket is destroyed or not after a timeout is up to
the user. This leaves an edge case where a socket that has emitted
'timeout' might be re-used from the free pool. Even if destroy is called
on the socket, it won't be removed from the freelist until 'close' which
can happen several ticks later.

Sockets are removed from the free list on the 'close' event.
However, there is a delay between calling destroy() and 'close'
being emitted. This means that it possible for a socket that has
been destroyed to be re-used from the free list, causing unexpected
failures.

PR-URL: #32000
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

MylesBorins pushed a commit that referenced this pull request

Mar 9, 2020
Whether and when a socket is destroyed or not after a timeout is up to
the user. This leaves an edge case where a socket that has emitted
'timeout' might be re-used from the free pool. Even if destroy is called
on the socket, it won't be removed from the freelist until 'close' which
can happen several ticks later.

Sockets are removed from the free list on the 'close' event.
However, there is a delay between calling destroy() and 'close'
being emitted. This means that it possible for a socket that has
been destroyed to be re-used from the free list, causing unexpected
failures.

PR-URL: #32000
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

targos pushed a commit that referenced this pull request

Apr 20, 2020
Whether and when a socket is destroyed or not after a timeout is up to
the user. This leaves an edge case where a socket that has emitted
'timeout' might be re-used from the free pool. Even if destroy is called
on the socket, it won't be removed from the freelist until 'close' which
can happen several ticks later.

Sockets are removed from the free list on the 'close' event.
However, there is a delay between calling destroy() and 'close'
being emitted. This means that it possible for a socket that has
been destroyed to be re-used from the free list, causing unexpected
failures.

PR-URL: #32000
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

fengmk2

@ronag ronag mentioned this pull request

May 22, 2020

killagu added a commit to node-modules/agentkeepalive that referenced this pull request

Jun 14, 2020

fengmk2 pushed a commit to node-modules/agentkeepalive that referenced this pull request

Jun 15, 2020

mariodu