Skip to content

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

Closed
atlowChemi wants to merge 6 commits intonodejs:mainfrom
atlowChemi:http-server-async-dispose
Closed

http, https, http2: Server add Symbol.asyncDispose#48548
atlowChemi wants to merge 6 commits intonodejs:mainfrom
atlowChemi:http-server-async-dispose

Conversation

@atlowChemi
Copy link
Member

@atlowChemi atlowChemi commented Jun 25, 2023

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 25, 2023

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 MoLow and benjamingr 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
benjamingr
benjamingr reviewed Jun 25, 2023
View reviewed changes
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

nodejs-github-bot commented Jun 25, 2023

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

nodejs-github-bot commented Jun 26, 2023

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 26, 2023

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 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

@mcollina mcollina mcollina approved these changes

@ShogunPanda ShogunPanda ShogunPanda approved these changes

@benjamingr benjamingr benjamingr 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. 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