pipeline leaves a hanging error handler

@ronag

Version: 14.12.0
Platform: Linux SZM-DESKTOP 4.19.104-microsoft-standard #1 SMP Wed Feb 19 06:37:35 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Subsystem: stream

What steps will reproduce the bug?

const {pipeline, Duplex, PassThrough} = require('stream');

const a = new PassThrough();
a.end('foobar');

const b = new Duplex({
    write(chunk, encoding, callback) {
        callback();
    }
});

pipeline(a, b, error => {
    if (error) {
        throw error;
    }
    
    console.log(b.listenerCount('error'));
    setTimeout(() => {
        console.log(b.listenerCount('error'));
        b.destroy(new Error('no way'));
    }, 100);
});

How often does it reproduce? Is there a required condition?

Always.

What is the expected behavior?

0
0
[Uncaught] Error: no way

What do you see instead?

2
1

/cc @ronag

This is by design.

The docs say:

stream.pipeline() leaves dangling event listeners on the streams after the callback has been invoked. In the case of reuse of streams after failure, this can cause event listener leaks and swallowed errors.

This is by design.

Can this design be improved? Or is this a no?

Hmmm

/cc @nodejs/streams

I don’t think there is anything more to do here. Unless someone has suggestion?

@ronag

@ronag I agree. Using pipeline() means letting pipeline() manage the lifetime of the streams, after which the streams are destroyed and should not be touched anymore.

Using pipeline() means letting pipeline() manage the lifetime of the streams

It does not. b.destroyed is false. So the readable side is still open. The fix is either to remove the hanging handler or destroy the end stream.

I missed that the last stream in the example is a duplex stream. Not sure how that should behave, because the readable side can't be consumed.

the readable side can't be consumed.

What do you mean?

If it were pipeline(readableStream, duplexStream, writableStream) then the readable side of duplexStream would be piped into writableStream. But if duplexStream is the last stream in the pipeline, then no one is reading from it.

If we swap the duplex stream in your example for a writable stream, then it does get destroyed:

const {pipeline, PassThrough, Writable} = require('stream')

const a = new PassThrough()
a.end('foobar')

const b = new Writable({
  write (chunk, encoding, callback) {
    callback()
  }
})

pipeline(a, b, function (err) {
  if (err) {
    throw err
  }

  console.log(a.destroyed) // true
  console.log(b.destroyed) // true
})

But if duplexStream is the last stream in the pipeline, then no one is reading from it.

You don't know. It's readable so it's not destroyed. Something can be reading it a tick later.

Sure, would love to.

@szmarczak

@szmarczak any updates here?

Sorry, totally forgot about this one. Will sketch something in a few hours.

@szmarczak

Sorry, totally forgot about this one. Will sketch something in a few hours.

Count on me to help! @szmarczak

@joaoofreitas

@joaoofreitas awesome! Feel free to send a PR first and I'll chime in and let you know my thoughts :D

If this is still work in progress I'd be happy to help

Sure thing!

@mcollina

Hey @mcollina. Can I help in this? (If @lvndry isn't working on it anymore)

I'm new to NodeJS and to Open Source. Might be hard for me, but I'd like to try.

@Heikrana

@Heikrana this might end up a bit too hard. Maybe try something easier first? It's one of the hardest part of Node.

@Heikrana

@Heikrana this might end up a bit too hard. Maybe try something easier first? It's one of the hardest part of Node.

Okay, I see your point. I've been trying to understand the stream API and yeah, it's a bit confusing.
I'd still try to solve this issue (since no one else is working on it).

But in the meantime, can you suggest me some resources (which would help me contribute to NodeJS) or another issue which might be more of my level. I really want to be part of this community :)

stream: need to cleanup event listeners if last stream is readable

fix(stream): keep composed tail errors after pipeline completes

compat: corpus coverage waves 40-70 — Node 3,134/4,433 (70.7%), Bun 1…

Navigation Menu

Repository navigation

`pipeline` leaves a hanging error handler #35452

What steps will reproduce the bug?

How often does it reproduce? Is there a required condition?

What is the expected behavior?

What do you see instead?

3 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

pipeline leaves a hanging error handler #35452

Description

What steps will reproduce the bug?

How often does it reproduce? Is there a required condition?

What is the expected behavior?

What do you see instead?

Activity

ronag commented on Oct 1, 2020

ronag commented on Oct 1, 2020

szmarczak commented on Oct 1, 2020

possum1102 commented on Oct 3, 2020

targos commented on Dec 13, 2020

ronag commented on Dec 13, 2020

vweevers commented on Dec 13, 2020

szmarczak commented on Dec 13, 2020

vweevers commented on Dec 13, 2020

szmarczak commented on Dec 13, 2020

vweevers commented on Dec 13, 2020

szmarczak commented on Dec 13, 2020

3 remaining items

szmarczak commented on Dec 14, 2020

mcollina commented on Apr 22, 2021

szmarczak commented on Apr 22, 2021

joaoofreitas commented on Apr 22, 2021

szmarczak commented on Apr 22, 2021

lvndry commented on Jul 8, 2021

mcollina commented on Jul 8, 2021

Heikrana commented on Feb 1, 2022

mcollina commented on Feb 2, 2022

Heikrana commented on Feb 2, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

`pipeline` leaves a hanging error handler #35452