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.

Sign up for GitHub

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

Jump to bottom

http, https, http2: Server add Symbol.asyncDispose #48548

Closed
wants to merge 6 commits into from
Closed

http, https, http2: Server add Symbol.asyncDispose #48548

wants to merge 6 commits into from

Conversation

atlowChemi
Copy link
Member

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Jun 25, 2023
@atlowChemi atlowChemi requested review from benjamingr and MoLow June 25, 2023 17:54
@atlowChemi atlowChemi force-pushed the http-server-async-dispose branch from 7fff95c to 6f92f87 Compare June 25, 2023 18:00
benjamingr
benjamingr reviewed Jun 25, 2023
View reviewed changes
doc/api/http.md Outdated Show resolved Hide resolved
benjamingr
benjamingr reviewed Jun 25, 2023
View reviewed changes
lib/_http_server.js Outdated Show resolved Hide resolved
benjamingr
benjamingr reviewed Jun 25, 2023
View reviewed changes
Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good left some comments

@atlowChemi atlowChemi force-pushed the http-server-async-dispose branch from 6f92f87 to d293a6a Compare June 25, 2023 18:03
@atlowChemi atlowChemi force-pushed the http-server-async-dispose branch from d293a6a to 81f630f Compare June 25, 2023 18:16
@atlowChemi atlowChemi changed the title http: Server add Symbol.asyncDispose http, http2: Server add Symbol.asyncDispose Jun 25, 2023
@atlowChemi atlowChemi added the http2 Issues or PRs related to the http2 subsystem. label Jun 25, 2023
@atlowChemi atlowChemi requested a review from benjamingr June 25, 2023 20:30
@atlowChemi atlowChemi force-pushed the http-server-async-dispose branch from bd52b5a to 7577fd8 Compare June 25, 2023 20:30
@atlowChemi atlowChemi added the https Issues or PRs related to the https subsystem. label Jun 25, 2023
@atlowChemi atlowChemi changed the title http, http2: Server add Symbol.asyncDispose http, https, http2: Server add Symbol.asyncDispose Jun 25, 2023
@atlowChemi atlowChemi added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 25, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 25, 2023
@nodejs-github-bot
Copy link
Collaborator

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

@github-actions github-actions bot mentioned this pull request Jun 26, 2023
Open
21 tasks
@atlowChemi atlowChemi added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 26, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 26, 2023
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

ShogunPanda
ShogunPanda approved these changes Jun 26, 2023
View reviewed changes
Copy link
Contributor

@ShogunPanda ShogunPanda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

MoLow pushed a commit that referenced this pull request Jun 29, 2023
@atlowChemi @MoLow
http: server add async dispose
909b43f 
PR-URL: #48548
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
MoLow pushed a commit that referenced this pull request Jun 29, 2023
@atlowChemi @MoLow
http2: server add asyncDispose
b7bfb17 
PR-URL: #48548
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
MoLow pushed a commit that referenced this pull request Jun 29, 2023
@atlowChemi @MoLow
https: server add asyncDispose
e8810b9 
PR-URL: #48548
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@atlowChemi atlowChemi deleted the http-server-async-dispose branch June 29, 2023 08:55
This was referenced Jun 30, 2023
Open
Open
Open
Open
RafaelGSS pushed a commit that referenced this pull request Jul 3, 2023
@atlowChemi @RafaelGSS
http: server add async dispose
94ebb02 
PR-URL: #48548
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Jul 3, 2023
@atlowChemi @RafaelGSS
http2: server add asyncDispose
5378ad8 
PR-URL: #48548
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Jul 3, 2023
@atlowChemi @RafaelGSS
https: server add asyncDispose
97a58c5 
PR-URL: #48548
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Jul 3, 2023
Merged
@github-actions github-actions bot mentioned this pull request Jul 4, 2023
Open
22 tasks
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
@atlowChemi @Ceres6
http: server add async dispose
377a9f4 
PR-URL: nodejs#48548
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
@atlowChemi @Ceres6
http2: server add asyncDispose
1aad923 
PR-URL: nodejs#48548
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
@atlowChemi @Ceres6
https: server add asyncDispose
a9efe2e 
PR-URL: nodejs#48548
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
@atlowChemi @Ceres6
http: server add async dispose
9df83ae 
PR-URL: nodejs#48548
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
@atlowChemi @Ceres6
http2: server add asyncDispose
0af65d0 
PR-URL: nodejs#48548
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
@atlowChemi @Ceres6
https: server add asyncDispose
6fcb5d2 
PR-URL: nodejs#48548
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
ruyadorno pushed a commit that referenced this pull request Sep 11, 2023
@atlowChemi @ruyadorno
http: server add async dispose
3edeeac 
PR-URL: #48548
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
ruyadorno pushed a commit that referenced this pull request Sep 11, 2023
@atlowChemi @ruyadorno
http2: server add asyncDispose
3ecc607 
PR-URL: #48548
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
ruyadorno pushed a commit that referenced this pull request Sep 11, 2023
@atlowChemi @ruyadorno
https: server add asyncDispose
9e9b8a3 
PR-URL: #48548
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@ruyadorno ruyadorno mentioned this pull request Sep 11, 2023
Merged
@ruyadorno
Copy link
Member

ruyadorno commented Sep 12, 2023 •
edited
Loading

This PR is not landing cleanly on v18.x-staging since #48383 had to be removed. It's going to be blocked pending the backport of that PR.

@ruyadorno ruyadorno added the backport-blocked-v18.x PRs that should land on the v18.x-staging branch but are blocked by another PR's pending backport. label Sep 12, 2023
ruyadorno pushed a commit that referenced this pull request Sep 13, 2023
@atlowChemi @ruyadorno
http: server add async dispose
fcd0424 
PR-URL: #48548
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
ruyadorno pushed a commit that referenced this pull request Sep 13, 2023
@atlowChemi @ruyadorno
http2: server add asyncDispose
7a00bda 
PR-URL: #48548
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
H4ad pushed a commit to H4ad/node that referenced this pull request Sep 14, 2023
@atlowChemi @H4ad
https: server add asyncDispose
8bb84b3 
PR-URL: nodejs#48548
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
H4ad pushed a commit to H4ad/node that referenced this pull request Sep 17, 2023
@atlowChemi @H4ad
http: server add async dispose
b84f745 
PR-URL: nodejs#48548
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
H4ad added a commit to H4ad/node that referenced this pull request Sep 17, 2023
@H4ad
http2: server add asyncDispose
85a5b17 
PR-URL: nodejs#48548
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MoLow MoLow MoLow approved these changes

@benjamingr benjamingr benjamingr approved these changes

@ShogunPanda ShogunPanda ShogunPanda approved these changes

@mcollina mcollina mcollina approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backport-blocked-v18.x PRs that should land on the v18.x-staging branch but are blocked by another PR's pending backport. commit-queue-failed An error occurred while landing this pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. http Issues or PRs related to the http subsystem. http2 Issues or PRs related to the http2 subsystem. https Issues or PRs related to the https subsystem. needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@atlowChemi @nodejs-github-bot @MoLow @ruyadorno @mcollina @ShogunPanda @benjamingr