Repository navigation
Description
Version
version 20.13.0 and higher
Platform
all
Subsystem
webstreams
What steps will reproduce the bug?
No response
How often does it reproduce? Is there a required condition?
No response
What is the expected behavior? Why is that the expected behavior?
No response
What do you see instead?
I was curious about the freshness of the webstreams code in v20 versus main, so I looked at the commit history of the following folder:
https://github.com/nodejs/node/tree/main/lib/internal/webstreams
I noticed something odd about the following commits:
It appears there was an initial fix attempt that had some problem, then a revert, and finally a correct fix. These three commits seem identical in main branch and v22.x branch, while v21.x branch includes three equivalent commits in the same chronological order on slightly different dates. However v20.x branch looks different.
https://github.com/nodejs/node/tree/v20.x/lib/internal/webstreams
It appears the broken initial fix attempt never made it into v20.x branch in the first place. The first relevant commit seems to be the final correct fix. However a little over a month later, the older revert commit seems to have been applied over top of the correct fix. I'm guessing this was an accidental mistake?
If so, it looks like the mistaken revert modified a couple of test files, and also incorrectly removed a couple of "port.unref();" lines from the "transfer.js" file. If the commit notes listed in CHANGELOG_V20.md are correct, this presumably broken transfer.js file shipped in Node version 20.13.0, and is still broken in the current v20.14.0-proposal branch scheduled to ship this coming Tuesday 2024-05-28.
https://github.com/nodejs/node/tree/v20.14.0-proposal/lib/internal/webstreams
I don't know anything about the specifics of any of this stuff, so hopefully I'm not wasting someone's time by writing this. However it looks fishy enough to me that I thought it was worth reporting.
Thanks for your time.
Additional information
No response
Activity
avivkeller commentedon May 24, 2024
@tsctx
@mcollina
@jasnell
(Original commit authors)
climba03003 commentedon May 25, 2024
The same revert PR is being deployed in two different version.
f2dfe0fa80 which is
20.12.08d20b641a2 which is
20.13.0The proper fix d0a6f35 is deployed in
20.12.0I think it is accidentally included the Revert in
20.13.0since20.12.0is correctly excluded it.marco-ippolito commentedon May 25, 2024
I guess we can just revert "the revert commit" 8d20b641a2 that shouldnt have landed?
#53144
Revert "stream: revert fix cloned webstreams not being unref'd"
marco-ippolito commentedon May 29, 2024
Fixed in v20.14.0