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

Revert 51255 #51491

Closed
wants to merge 2 commits into from
Closed

Revert 51255 #51491

wants to merge 2 commits into from

Conversation

mcollina
Copy link
Member

Reverts #51255
Fixes #51486

Adds a regression test.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. web streams labels Jan 16, 2024
@mcollina mcollina added commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. request-ci Add this label to start a Jenkins CI on a PR. and removed needs-ci PRs that need a full CI run. web streams labels Jan 16, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 16, 2024
@nodejs-github-bot
Copy link
Collaborator

@mcollina
Copy link
Member Author

Can we fast-track this?

@mcollina mcollina added the fast-track PRs that do not need to wait for 48 hours to land. label Jan 16, 2024
Copy link
Contributor

Fast-track has been requested by @mcollina. Please 👍 to approve.

@mcollina mcollina added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 16, 2024
@nodejs-github-bot
Copy link
Collaborator

@RafaelGSS
Copy link
Member

RafaelGSS commented Jan 18, 2024

@nodejs/tsc @nodejs/build I was planning to create a proposal yesterday with this revert but apparently some of our machines are suffering. Is that ok to force land it? I'm planning to create the proposal asap and release it tomorrow.

Considering it's a clean revert + a test case it wouldn't be a problem I assume.

cc: @nodejs/releasers

@aduh95
Copy link
Contributor

aduh95 commented Jan 18, 2024

apparently some of our machines are suffering

The issue seems to be the backlog for the osx CI is growing faster than the CI is eating it. Would it be possible to disable that job until the queue is gone instead?

@nodejs-github-bot
Copy link
Collaborator

@BethGriggs
Copy link
Member

The issue seems to be the backlog for the osx CI is growing faster than the CI is eating it. Would it be possible to disable that job until the queue is gone instead?

All the osx11-x64 machines are offline https://ci.nodejs.org/label/osx11-x64/

@nodejs-github-bot
Copy link
Collaborator

@mhdawson
Copy link
Member

Just a reminder that in a case like this opening an issue in the build repo may make it more visible that help is needed, I went ahead and did that here - nodejs/build#3612

@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 19, 2024
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/51491
✔  Done loading data for nodejs/node/pull/51491
----------------------------------- PR info ------------------------------------
Title      Revert 51255 (#51491)
Author     Matteo Collina  (@mcollina)
Branch     mcollina:revert-51255 -> nodejs:main
Labels     fast-track, commit-queue-rebase
Commits    2
 - Revert "stream: fix cloned webstreams not being unref'd"
 - test: add regression test for 51586
Committers 1
 - Matteo Collina 
PR-URL: https://github.com/nodejs/node/pull/51491
Reviewed-By: Vinícius Lourenço Claro Cardoso 
Reviewed-By: Marco Ippolito 
Reviewed-By: Matthew Aitken 
Reviewed-By: Rafael Gonzaga 
Reviewed-By: Moshe Atlow 
Reviewed-By: Franziska Hinkelmann 
Reviewed-By: Benjamin Gruenbaum 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/51491
Reviewed-By: Vinícius Lourenço Claro Cardoso 
Reviewed-By: Marco Ippolito 
Reviewed-By: Matthew Aitken 
Reviewed-By: Rafael Gonzaga 
Reviewed-By: Moshe Atlow 
Reviewed-By: Franziska Hinkelmann 
Reviewed-By: Benjamin Gruenbaum 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Tue, 16 Jan 2024 16:01:48 GMT
   ✔  Approvals: 7
   ✔  - Vinícius Lourenço Claro Cardoso (@H4ad): https://github.com/nodejs/node/pull/51491#pullrequestreview-1824043243
   ✔  - Marco Ippolito (@marco-ippolito): https://github.com/nodejs/node/pull/51491#pullrequestreview-1824108380
   ✔  - Matthew Aitken (@KhafraDev): https://github.com/nodejs/node/pull/51491#pullrequestreview-1824177345
   ✔  - Rafael Gonzaga (@RafaelGSS) (TSC): https://github.com/nodejs/node/pull/51491#pullrequestreview-1824440897
   ✔  - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/51491#pullrequestreview-1824680016
   ✔  - Franziska Hinkelmann (@fhinkel): https://github.com/nodejs/node/pull/51491#pullrequestreview-1826347736
   ✔  - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/51491#pullrequestreview-1830226110
   ℹ  This PR is being fast-tracked
   ✘  Last GitHub CI failed
   ℹ  Last Full PR CI on 2024-01-18T15:53:59Z: https://ci.nodejs.org/job/node-test-pull-request/56836/
- Querying data for job/node-test-pull-request/56836/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/7582152963

@nodejs-github-bot nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Jan 19, 2024
@nodejs-github-bot
Copy link
Collaborator

@mcollina mcollina added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jan 19, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 19, 2024
@nodejs-github-bot
Copy link
Collaborator

Landed in a772973...fbf1fb3

nodejs-github-bot pushed a commit that referenced this pull request Jan 19, 2024
This reverts commit 4d3923a.

PR-URL: #51491
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
nodejs-github-bot pushed a commit that referenced this pull request Jan 19, 2024
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #51491
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@mcollina mcollina deleted the revert-51255 branch January 19, 2024 13:29
RafaelGSS pushed a commit that referenced this pull request Jan 19, 2024
This reverts commit 4d3923a.

PR-URL: #51491
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Jan 19, 2024
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #51491
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Jan 19, 2024
marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Jan 22, 2024
This reverts commit 4d3923a.

PR-URL: nodejs#51491
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Jan 22, 2024
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: nodejs#51491
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@@ -598,8 +598,6 @@ class ReadableStream {

[kTransferList]() {
const { port1, port2 } = new MessageChannel();
port1.unref();

This comment was marked as spam.

This comment was marked as spam.

marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Feb 2, 2024
This reverts commit 4d3923a.

PR-URL: nodejs#51491
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Feb 2, 2024
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: nodejs#51491
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Feb 19, 2024
This reverts commit 4d3923a.

PR-URL: nodejs#51491
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Feb 19, 2024
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: nodejs#51491
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
richardlau pushed a commit that referenced this pull request Mar 25, 2024
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #51491
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
richardlau pushed a commit that referenced this pull request Mar 25, 2024
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #51491
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@richardlau richardlau mentioned this pull request Mar 25, 2024
marco-ippolito pushed a commit that referenced this pull request May 2, 2024
This reverts commit 4d3923a.

PR-URL: #51491
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. fast-track PRs that do not need to wait for 48 hours to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression in Node.js v21.6.0