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.

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

pipeline does not wait for full stream destruction #23214

Closed
ronag opened this issue Oct 2, 2018 · 4 comments
Closed

pipeline does not wait for full stream destruction #23214

ronag opened this issue Oct 2, 2018 · 4 comments
Labels
stream Issues and PRs related to the stream subsystem.

Comments

@ronag
Copy link
Member

ronag commented Oct 2, 2018

Related to #23133.

Even if we change so that open is emitted before destroy. pipeline will not wait for completed destruction for all streams. Which can cause subtle bugs with things like:

const rec = db.getRecord(blockId)
pipeline(
  req, 
  fs.createWriteStream(blockPath, { flags: 'wx' })
    .on('open', () => {
      // Oops, this can be called after rec.destroy() :/. I didn't expect that.
      rec.set({
        path: blockPath
      })
    }),
  err => {
    rec.destroy()
    callback(err)
  }
)
@ronag ronag changed the title pipeline can call callback too early? pipeline does not wait for full stream destruction Oct 2, 2018
@Fishrock123 Fishrock123 added the stream Issues and PRs related to the stream subsystem. label Oct 5, 2018
@Fishrock123
Copy link
Contributor

cc @nodejs/streams

@mcollina
Copy link
Member

mcollina commented Oct 5, 2018

I'm a bit lost on the difference to #23133. Can you remove the 'open' problem of fs from this issue? This seems a different thing.

What do you mean by "complete destruction of all streams"? .destroy() has currently a private callback. However the .destroy() method in the community does not always have a callback, so how could we wait for something that's not done?

@ronag
Copy link
Member Author

ronag commented Oct 5, 2018

However the .destroy() method in the community does not always have a callback, so how could we wait for something that's not done?

I don't know. All I know is that once the callback on the pipeline is invoked I would expect that no more events are emitted from any of the streams. Which is not how it works right now.

@mcollina
Copy link
Member

mcollina commented Oct 5, 2018

So it is a duplicate of #23133 and a problem specific to fs.

Also, this would possibly be fixed by #22795.

@ronag ronag closed this as completed Oct 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

No branches or pull requests

3 participants