Skip to content

[v14.x backport] stream: fix multiple Writable.destroy() calls - #38473

Closed
ronag wants to merge 1 commit into
nodejs:v14.x-stagingfrom
nxtedition:backport-38221-to-v14.x
Closed

[v14.x backport] stream: fix multiple Writable.destroy() calls#38473
ronag wants to merge 1 commit into
nodejs:v14.x-stagingfrom
nxtedition:backport-38221-to-v14.x

Conversation

@ronag

Copy link
Copy Markdown
Member

Calling Writable.destroy() multiple times in the same tick
could cause an assertion error.

Fixes: #38189

PR-URL: #38221
Reviewed-By: Matteo Collina matteo.collina@gmail.com
Reviewed-By: Luigi Pinca luigipinca@gmail.com
Reviewed-By: Nitzan Uziely linkgoron@gmail.com
Reviewed-By: Rich Trott rtrott@gmail.com

@ronag
ronagforce-pushed the backport-38221-to-v14.x branch from 6531b98 to a540afdCompareApril 29, 2021 18:47
@github-actionsgithub-actionsBot added needs-ci PRs that need a full CI run. v14.x labels Apr 29, 2021
@ronag
ronagforce-pushed the backport-38221-to-v14.x branch from a540afd to 33424e6CompareApril 29, 2021 18:49
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@ronag
ronagforce-pushed the backport-38221-to-v14.x branch from 33424e6 to fa4b816CompareApril 29, 2021 18:51
Calling Writable.destroy() multiple times in the same tick
could cause an assertion error.
Fixes: nodejs#38189
PR-URL: nodejs#38221
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Backport-PR-URL: nodejs#38473
@ronag
ronagforce-pushed the backport-38221-to-v14.x branch from fa4b816 to e516c0aCompareApril 29, 2021 18:51
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@ronagronag removed the needs-ci PRs that need a full CI run. label Apr 29, 2021
@ronag

Copy link
Copy Markdown
MemberAuthor

Not sure if there really is anything to backport here...

@targos

Copy link
Copy Markdown
Member

Not sure if there really is anything to backport here...

What do you mean? Does it cherry-pick without conflicts?

@ronag

Copy link
Copy Markdown
MemberAuthor

What do you mean? Does it cherry-pick without conflicts?

I mean that the only thing to backport is the test. The bug the commit fixes doesn't exist on 14.x.

@targos

Copy link
Copy Markdown
Member

Ah, I see. I think the backport was requested because of the lts-watch-v14.x label.

@targos

Copy link
Copy Markdown
Member

Do you know which commit/PR introduced the bug?

@danielleadams

Copy link
Copy Markdown
Contributor

Ah yes, it had the tag and when I tried to backport it didn't land cleanly. If there's nothing to backport, I guess we can just land the test if it covers the previous bug?

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

targos pushed a commit that referenced this pull request May 30, 2021
Calling Writable.destroy() multiple times in the same tick
could cause an assertion error.
Fixes: #38189
PR-URL: #38221
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Backport-PR-URL: #38473
@targos

Copy link
Copy Markdown
Member

Landed in 358ae8b

@targostargos closed this May 30, 2021
targos pushed a commit that referenced this pull request Jun 5, 2021
Calling Writable.destroy() multiple times in the same tick
could cause an assertion error.
Fixes: #38189
PR-URL: #38221
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Backport-PR-URL: #38473
targos pushed a commit that referenced this pull request Jun 5, 2021
Calling Writable.destroy() multiple times in the same tick
could cause an assertion error.
Fixes: #38189
PR-URL: #38221
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Backport-PR-URL: #38473
targos pushed a commit that referenced this pull request Jun 11, 2021
Calling Writable.destroy() multiple times in the same tick
could cause an assertion error.
Fixes: #38189
PR-URL: #38221
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Backport-PR-URL: #38473
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@ronag@nodejs-github-bot@targos@danielleadams@mcollina