Regression in Node.js v21.6.0

#51255 makes the following test fail:

import { test } from 'node:test'
import assert from 'node:assert'

test('repro', async (t) => {
  const buf = new Uint8Array(1)
  const readable = new ReadableStream({
    start (controller) {
      controller.enqueue(buf)
      controller.close()
    }
  })

  const [out1, out2] = readable.tee()
  const cloned = structuredClone(out2, { transfer: [out2] })

  for await (const chunk of cloned) {
    assert.deepStrictEqual(chunk, buf)
  }

  for await (const chunk of out2) {
    assert.deepStrictEqual(chunk, buf)
  }

  for await (const chunk of out1) {
    assert.deepStrictEqual(chunk, buf)
  }
})

with:

✖ repro (5.626125ms)
  'Promise resolution is still pending but the event loop has already resolved'

ℹ tests 1
ℹ suites 0
ℹ pass 0
ℹ fail 0
ℹ cancelled 1
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 8.293458

✖ failing tests:

test at file:/Users/matteo/Repositories/node/repro.mjs:6:1
✖ repro (5.626125ms)
  'Promise resolution is still pending but the event loop has already resolved'

It seems that #51255 unveiled a problem of a lingering promise not being resolved properly after the two MessagePorts have been closed.

This emerged in undici, see nodejs/undici#2617 for more details.

@jasnell

cc @jasnell

Do you think the #51131 can fix both issues? Or did we just find a new bug after fixing the old bug?

I think this unveiled a new bug. I'm not sure if #51131 fixes it, I would not expect it to, but I haven't time to track down that lingering promise.

I tested it on main but cannot reproduce it

if [ ! -r node ] || [ ! -L node ]; then ln -fs out/Release/node node; fi
14:46:43 ~/Documents/projects/forks/node main $ ./node test/parallel/test-regression.js
✔ repro (7.64375ms)
ℹ tests 1
ℹ suites 0
ℹ pass 1
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 10.764625

@marco-ippolito

This reproduces solidly on my end and undici CI on both Linux and Mac OS X, have you pulled the latest commits @marco-ippolito?

Yes, head at e133e5115a829aa7a445cfd8754df3a3dd586b0a cleaned up build too

fun! :D

@marco-ippolito

@marco-ippolito are you running it as a .mjs?

@marco-ippolito

@marco-ippolito are you running it as a .mjs?

Right I was running it as cjs 😮 so its mjs only infact as mjs I can reproduce

Even more fun!

Using .mjs and #51131:

✔ repro (8.221ms)
ℹ tests 1
ℹ suites 0
ℹ pass 1
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 11.25954

I rebase main before running the test.

Edit: I needed to revert #51255 in order to make the test work.

Based on nodejs/undici#2617 (comment), I propose we revert #51255 asap.

Am I mistaken or should this be closed now that #51491 has landed and 21.6.1 has been released?

yes

Is it intended to throw when you try to transfer an already detached array buffer?

var ab = new ArrayBuffer(1); 
structuredClone(ab, { transfer: [ab] }); 
structuredClone(ab, { transfer: [ab] });

[Fix] `ES2015`+: `DetachArrayBuffer`: node v21.0.0+ structuredClone t…

@anonrig

@anonrig you might want to open a fresh issue

Navigation Menu

Repository navigation

Regression in Node.js v21.6.0 #51486

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Regression in Node.js v21.6.0 #51486

Description

Activity

mcollina commented on Jan 16, 2024

H4ad commented on Jan 16, 2024

mcollina commented on Jan 16, 2024

marco-ippolito commented on Jan 16, 2024

mcollina commented on Jan 16, 2024

marco-ippolito commented on Jan 16, 2024

mcollina commented on Jan 16, 2024

RafaelGSS commented on Jan 16, 2024

marco-ippolito commented on Jan 16, 2024

mcollina commented on Jan 16, 2024

H4ad commented on Jan 16, 2024

mcollina commented on Jan 16, 2024

Trott commented on Jan 22, 2024

mcollina commented on Jan 22, 2024

anonrig commented on Feb 28, 2024

mcollina commented on Feb 28, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions