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

net: add ability to reset a tcp socket #43112

Closed
wants to merge 5 commits into from
Closed

Conversation

PupilTong
Copy link
Contributor

@PupilTong PupilTong commented May 15, 2022

make it possible to forcibly rest a tcp socket:

  • add a new method Socket.prototype.resetAndDestroy

  • add a new private method Socket.prototype._reset

  • add a new flag resetAndClosing to make _destroy calls
    the _reset instead of close while destroying a Socket.

  • add new methods TCPWrap::Reset to be a wrap of uv_tcp_close_reset

  • change HandleWrap::state_ from private to protected.
    This is essential for keeping the same behaviour between
    TCPWrap::Reset and HandleWrap::Close

  • add test cases for the new method

Fixes: #27428

make it possible to forcibly rest a tcp socket:

* add a new method `Socket.prototype.resetAndDestroy`

* add a new flag `resetAndClosing` to make `_destroy` calls
the `reset` instead of close while destroying a Socket.

* add new methods `TCPWrap::Reset` to be a wrap of `uv_tcp_close_reset`

* change `HandleWrap::state_` from private to protected.
This is essential for keeping the same behaviour between
`TCPWrap::Reset` and `HandleWrap::Close`

Fixes: nodejs#27428
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented May 15, 2022

Review requested:

@nodejs-github-bot nodejs-github-bot added c++ lib / src needs-ci labels May 15, 2022
@ShogunPanda
Copy link
Contributor

@ShogunPanda ShogunPanda commented May 16, 2022

So far the approach LGTM! Good job!

@PupilTong
Copy link
Contributor Author

@PupilTong PupilTong commented May 16, 2022

Will start reviewing after adding some test cases, referencing those test cases related Socket.prototype.destroy. Thanks! :D

PupilTong added 3 commits May 18, 2022
make it possible to forcibly rest a tcp socket:

* add a new method `Socket.prototype.resetAndDestroy`

* add a new flag `resetAndClosing` to make `_destroy` calls
the `reset` instead of close while destroying a Socket.

* add new methods `TCPWrap::Reset` to be a wrap of `uv_tcp_close_reset`

* change `HandleWrap::state_` from private to protected.
This is essential for keeping the same behaviour between
`TCPWrap::Reset` and `HandleWrap::Close`

Fixes: nodejs#27428
make it possible to forcibly rest a tcp socket:

* add a new method `Socket.prototype.resetAndDestroy`

* add a new flag `resetAndClosing` to make `_destroy` calls
the `reset` instead of close while destroying a Socket.

* add new methods `TCPWrap::Reset` to be a wrap of `uv_tcp_close_reset`

* change `HandleWrap::state_` from private to protected.
This is essential for keeping the same behaviour between
`TCPWrap::Reset` and `HandleWrap::Close`

Fixes: nodejs#27428
make it possible to forcibly rest a tcp socket:

* add a new method `Socket.prototype.resetAndDestroy`

* add a new flag `resetAndClosing` to make `_destroy` calls
the `reset` instead of close while destroying a Socket.

* add new methods `TCPWrap::Reset` to be a wrap of `uv_tcp_close_reset`

* change `HandleWrap::state_` from private to protected.
This is essential for keeping the same behaviour between
`TCPWrap::Reset` and `HandleWrap::Close`

Fixes: nodejs#27428
@PupilTong PupilTong marked this pull request as ready for review May 18, 2022
make it possible to forcibly rest a tcp socket:

* add a new method `Socket.prototype.resetAndDestroy`

* add a new flag `resetAndClosing` to make `_destroy` calls
the `reset` instead of close while destroying a Socket.

* add new methods `TCPWrap::Reset` to be a wrap of `uv_tcp_close_reset`

* change `HandleWrap::state_` from private to protected.
This is essential for keeping the same behaviour between
`TCPWrap::Reset` and `HandleWrap::Close`

Fixes: nodejs#27428
Copy link
Contributor

@ShogunPanda ShogunPanda left a comment

LGTM! Good job!

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented May 23, 2022

@PupilTong
Copy link
Contributor Author

@PupilTong PupilTong commented May 23, 2022

48hrs+ PR's ready to go. :D

@ShogunPanda
Copy link
Contributor

@ShogunPanda ShogunPanda commented May 23, 2022

Yes, I'm taking care of the CI and then I'll land this.

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented May 23, 2022

CI: https://ci.nodejs.org/job/node-test-pull-request/44137/

@richardlau richardlau added the semver-minor label May 23, 2022
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented May 23, 2022

@ShogunPanda
Copy link
Contributor

@ShogunPanda ShogunPanda commented May 23, 2022

Landed in 360cda1

ShogunPanda pushed a commit that referenced this issue May 23, 2022
Fixes: #27428
PR-URL: #43112
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
bengl pushed a commit that referenced this issue May 30, 2022
Fixes: #27428
PR-URL: #43112
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
bengl added a commit that referenced this issue May 31, 2022
Notable changes:

* deps: update undici to 5.3.0 (Node.js GitHub Bot) #43197
* (SEMVER-MINOR) util: add parseArgs module (Benjamin Coe) #42675
* (SEMVER-MINOR) http: add uniqueHeaders option to request and createServer (Paolo Insogna) #41397
* deps: upgrade npm to 8.11.0 (npm team) #43210
* deps: patch V8 to 10.2.154.4 (Michaël Zasso) #43067
* (SEMVER-MINOR) deps: update V8 to 10.2.154.2 (Michaël Zasso) #42740
* (SEMVER-MINOR) fs: make params in writing methods optional (LiviaMedeiros) #42601
* (SEMVER-MINOR) http: add uniqueHeaders option to request and createServer (Paolo Insogna) #41397
* (SEMVER-MINOR) net: add ability to reset a tcp socket (pupilTong) #43112

PR-URL: TBD
@bengl bengl mentioned this pull request May 31, 2022
bengl added a commit that referenced this issue May 31, 2022
Notable changes:

* deps: update undici to 5.3.0 (Node.js GitHub Bot) #43197
* (SEMVER-MINOR) util: add parseArgs module (Benjamin Coe) #42675
* (SEMVER-MINOR) http: add uniqueHeaders option to request and createServer (Paolo Insogna) #41397
* deps: upgrade npm to 8.11.0 (npm team) #43210
* deps: patch V8 to 10.2.154.4 (Michaël Zasso) #43067
* (SEMVER-MINOR) deps: update V8 to 10.2.154.2 (Michaël Zasso) #42740
* (SEMVER-MINOR) fs: make params in writing methods optional (LiviaMedeiros) #42601
* (SEMVER-MINOR) http: add uniqueHeaders option to request and createServer (Paolo Insogna) #41397
* (SEMVER-MINOR) net: add ability to reset a tcp socket (pupilTong) #43112

PR-URL: #43266
bengl added a commit that referenced this issue Jun 1, 2022
Notable changes:

* deps: update undici to 5.3.0 (Node.js GitHub Bot) #43197
* (SEMVER-MINOR) util: add parseArgs module (Benjamin Coe) #42675
* (SEMVER-MINOR) http: add uniqueHeaders option to request and createServer (Paolo Insogna) #41397
* deps: upgrade npm to 8.11.0 (npm team) #43210
* deps: patch V8 to 10.2.154.4 (Michaël Zasso) #43067
* (SEMVER-MINOR) deps: update V8 to 10.2.154.2 (Michaël Zasso) #42740
* (SEMVER-MINOR) fs: make params in writing methods optional (LiviaMedeiros) #42601
* (SEMVER-MINOR) http: add uniqueHeaders option to request and createServer (Paolo Insogna) #41397
* (SEMVER-MINOR) net: add ability to reset a tcp socket (pupilTong) #43112
* (SEMVER-MINOR) Revert "build: make x86 Windows support temporarily experimental" (Michaël Zasso) [#42740](#42740)
  * This means 32-bit Windows binaries are back with this release.

PR-URL: #43266
bengl added a commit that referenced this issue Jun 1, 2022
Notable changes:

* deps: update undici to 5.3.0 (Node.js GitHub Bot) #43197
* (SEMVER-MINOR) util: add parseArgs module (Benjamin Coe) #42675
* (SEMVER-MINOR) http: add uniqueHeaders option to request and createServer (Paolo Insogna) #41397
* deps: upgrade npm to 8.11.0 (npm team) #43210
* deps: patch V8 to 10.2.154.4 (Michaël Zasso) #43067
* (SEMVER-MINOR) deps: update V8 to 10.2.154.2 (Michaël Zasso) #42740
* (SEMVER-MINOR) fs: make params in writing methods optional (LiviaMedeiros) #42601
* (SEMVER-MINOR) http: add uniqueHeaders option to request and createServer (Paolo Insogna) #41397
* (SEMVER-MINOR) net: add ability to reset a tcp socket (pupilTong) #43112
* (SEMVER-MINOR) Revert "build: make x86 Windows support temporarily experimental" (Michaël Zasso) [#42740](#42740)
  * This means 32-bit Windows binaries are back with this release.

PR-URL: #43266
bengl added a commit that referenced this issue Jun 1, 2022
Notable changes:

* deps: update undici to 5.4.0  (Node.js GitHub Bot) #43262
* (SEMVER-MINOR) util: add parseArgs module (Benjamin Coe) #42675
* (SEMVER-MINOR) http: add uniqueHeaders option to request and createServer (Paolo Insogna) #41397
* deps: upgrade npm to 8.11.0 (npm team) #43210
* deps: patch V8 to 10.2.154.4 (Michaël Zasso) #43067
* (SEMVER-MINOR) deps: update V8 to 10.2.154.2 (Michaël Zasso) #42740
* (SEMVER-MINOR) fs: make params in writing methods optional (LiviaMedeiros) #42601
* (SEMVER-MINOR) http: add uniqueHeaders option to request and createServer (Paolo Insogna) #41397
* (SEMVER-MINOR) net: add ability to reset a tcp socket (pupilTong) #43112
* (SEMVER-MINOR) Revert "build: make x86 Windows support temporarily experimental" (Michaël Zasso) [#42740](#42740)
  * This means 32-bit Windows binaries are back with this release.

PR-URL: #43266
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ lib / src needs-ci semver-minor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants