Revert 51255 by mcollina · Pull Request #51491 · nodejs/node · GitHub

Adds a regression test.

This reverts commit 4d3923a.

Signed-off-by: Matteo Collina <hello@matteocollina.com>

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

Can we fast-track this?

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

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

@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

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?

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

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/

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

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

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

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

Landed in a772973...fbf1fb3

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>

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>

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>

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>

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>

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>

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>

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>

mcollina added 2 commits January 16, 2024 16:45

Revert "stream: fix cloned webstreams not being unref'd"

d286f1e

This reverts commit 4d3923a.

test: add regression test for 51586

6a38f83

Signed-off-by: Matteo Collina <hello@matteocollina.com>

mcollina requested review from RafaelGSS and jasnell January 16, 2024 16:01

nodejs-github-bot added needs-ci PRs that need a full CI run. web streams labels Jan 16, 2024

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 Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 16, 2024

H4ad approved these changes Jan 16, 2024

View reviewed changes

richardlau mentioned this pull request Jan 16, 2024

streams: fix cloned webstreams not being unref'd #51255

Merged

marco-ippolito approved these changes Jan 16, 2024

View reviewed changes

KhafraDev approved these changes Jan 16, 2024

View reviewed changes

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

RafaelGSS approved these changes Jan 16, 2024

View reviewed changes

mcollina added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 16, 2024

MoLow approved these changes Jan 16, 2024

View reviewed changes

fhinkel approved these changes Jan 17, 2024

View reviewed changes

benjamingr approved these changes Jan 18, 2024

View reviewed changes

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 added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Jan 19, 2024

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

mcollina mentioned this pull request Jan 19, 2024

bug: using structuredClone with ReadableStream prevents process from exiting #44985

Closed

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 closed this Jan 19, 2024

mcollina deleted the revert-51255 branch January 19, 2024 13:29

RafaelGSS mentioned this pull request Jan 19, 2024

v21.6.1 proposal #51530

Merged

Trott mentioned this pull request Jan 22, 2024

Regression in Node.js v21.6.0 #51486

Closed

Taahoyare reviewed Jan 31, 2024

View reviewed changes

Comment thread lib/internal/webstreams/readablestream.js

[kTransferList]() {

const { port1, port2 } = new MessageChannel();

port1.unref();

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

richardlau mentioned this pull request Mar 25, 2024

v20.12.0 proposal #52212

Merged

marco-ippolito mentioned this pull request May 2, 2024

v20.13.0 proposal #52793

Merged

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

Revert 51255 by mcollina · Pull Request #51491 · nodejs/node · GitHub

Uh oh!

Uh oh!

Revert 51255 - #51491

Revert 51255#51491mcollina wants to merge 2 commits intonodejs:mainnodejs/node:mainfrom mcollina:revert-51255mcollina/node:revert-51255Copy head branch name to clipboard

Conversation

mcollina commented Jan 16, 2024

Uh oh!

nodejs-github-bot commented Jan 16, 2024

Uh oh!

mcollina commented Jan 16, 2024

Uh oh!

github-actions Bot commented Jan 16, 2024

Uh oh!

nodejs-github-bot commented Jan 17, 2024

Uh oh!

RafaelGSS commented Jan 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aduh95 commented Jan 18, 2024

Uh oh!

nodejs-github-bot commented Jan 18, 2024

Uh oh!

BethGriggs commented Jan 18, 2024

Uh oh!

nodejs-github-bot commented Jan 18, 2024

Uh oh!

mhdawson commented Jan 18, 2024

Uh oh!

nodejs-github-bot commented Jan 19, 2024

Uh oh!

nodejs-github-bot commented Jan 19, 2024

Uh oh!

nodejs-github-bot commented Jan 19, 2024

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

Revert 51255#51491
mcollina wants to merge 2 commits into
nodejs:mainfrom
mcollina:revert-51255

RafaelGSS commented Jan 18, 2024 •
edited
Loading