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

[v12.x backport] stream: sync stream unpipe resume #32264

Closed
wants to merge 1 commit into from
Closed

[v12.x backport] stream: sync stream unpipe resume #32264

wants to merge 1 commit into from

Conversation

ronag
Copy link
Member

@ronag ronag commented Mar 14, 2020 •
edited
Loading

PR-URL: #31191

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. npm Issues and PRs related to the npm client dependency or the npm registry. v12.x labels Mar 14, 2020
@ronag ronag force-pushed the backport-31191-to-12.x branch from d56c5b2 to 0b1f537 Compare March 14, 2020 17:36
@ronag ronag requested a review from codebytere March 14, 2020 17:36
@ronag ronag added stream Issues and PRs related to the stream subsystem. and removed build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. npm Issues and PRs related to the npm client dependency or the npm registry. labels Mar 14, 2020
@ronag ronag mentioned this pull request Mar 14, 2020
Closed
4 tasks
@nodejs-github-bot
Copy link
Collaborator

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

@codebytere
Copy link
Member

@ronag can you also add BACKPORT-PR-URL?

@ronag
Copy link
Member Author

ronag commented Mar 14, 2020

@codebytere I did?

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@codebytere codebytere force-pushed the v12.x-staging branch 2 times, most recently from 63a03d2 to d577190 Compare March 31, 2020 23:57
@codebytere
Copy link
Member

@ronag could you please rebase this against latest staging?

MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Apr 1, 2020
@ronag @MylesBorins
stream: sync stream unpipe resume
347f8b7 
pipe() ondata should not control flow state if cleaned up.

Fixes: nodejs#31190

Backport-PR-URL: nodejs#32264
PR-URL: nodejs#31191
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@MylesBorins
Copy link
Contributor

I went ahead and got a rebase done but was unable to push to the branch

https://github.com/MylesBorins/node/tree/backport-31191-to-12.x

@nodejs-github-bot
Copy link
Collaborator

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

@ronag
stream: sync stream unpipe resume
1cdf6f6 
pipe() ondata should not control flow state if cleaned up.

Fixes: nodejs#31190

PR-URL: nodejs#31191
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Backport-PR-URL: nodejs#32264
@ronag ronag force-pushed the backport-31191-to-12.x branch from 0b1f537 to 1cdf6f6 Compare April 1, 2020 07:12
@ronag
Copy link
Member Author

ronag commented Apr 1, 2020

I went ahead and got a rebase done but was unable to push to the branch

Not sure what this means. Is there a setting I missed when creating this branch?

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@MylesBorins
Copy link
Contributor

Not sure what this means. Is there a setting I missed when creating this branch?

This comes from a fork in an org not a personal repo so I don't think others can push to it.

Looks like lots of failures so double checking that staging is still green

12.x-staging CI: https://ci.nodejs.org/job/node-test-commit/37126/

@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

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

@MylesBorins
Copy link
Contributor

landed in 12253f8

@MylesBorins MylesBorins closed this Apr 1, 2020
MylesBorins pushed a commit that referenced this pull request Apr 1, 2020
@ronag @MylesBorins
stream: sync stream unpipe resume
12253f8 
pipe() ondata should not control flow state if cleaned up.

Fixes: #31190

Backport-PR-URL: #32264
PR-URL: #31191
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Backport-PR-URL: #32264
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codebytere codebytere Awaiting requested review from codebytere

Assignees
No one assigned
Labels
stream Issues and PRs related to the stream subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ronag @nodejs-github-bot @codebytere @MylesBorins