Emit 'close' on destroy(), so that pipe is cleaned up #3759

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants

This patch emits 'close' when you call destroy on a zlib stream.

For pipe to end properly, destroy needs to eventually emit 'close' once.

This is necessary is order to properly clean up any pipes that come off the stream.
pipe's event listeners will be cleaned up, and the call to destroy() will be propagated down the piped streams,
by calling dest.destroy().

If close is not emitted, the rest of the stream will be expecting another call to write() or end() but it will never come.

Owner

bnoordhuis commented Jul 26, 2012

A test case would be appreciated. Can you rework the commit message to something like zlib: emit 'close' on destroy()?

sure, will do.

fixed error message, and added test.

isaacs commented Jul 27, 2012

We can't do this, sorry. Discussed in IRC.

15:53 < dominictarr> isaacs, is there a pull request of the previous attempt, or something?
16:00 < isaacs> dominictarr: i don't know
16:00 < isaacs> it would have been about a year ago
16:01 < isaacs> honestly, though, i'm not interested in any stream interface changes that (a) make 'destroy' or 'close' events even more of a first-class 
                citizen in the interface, or (b) add things to pipe in a way that does not address the fundamental problems, or (c) have any chance of 
                breaking existing programs.
16:01 < isaacs> this does all three ;)

isaacs closed this Jul 27, 2012

@isaacs this is different issue. you are refering to joyent#3778

this is only about having zlib streams emit 'close' when destroy is called.

isaacs commented Jul 29, 2012

Oh, indeed. My mistake.

isaacs reopened this Jul 29, 2012

Owner

bnoordhuis commented Aug 2, 2012

@isaacs You're the zlib guru - is this a bug fix eligible for v0.8?

isaacs commented Aug 4, 2012

Landed on f4a4ef7.

/me weeps for our cluttered streams interface.

isaacs closed this Aug 4, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment