Commit 27532f4
stream: cleanup() when unpiping all streams.
This PR makes sure the object emitted as the 'unpipe'
event in the destination stream is not shared between
destination, as it would be muted.
Refs: #12746
PR-URL: #18266
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>1 parent a13fbdd commit 27532f4
2 files changed
Lines changed: 55 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -754,7 +754,7 @@ Readable.prototype.unpipe = function(dest) { | |||
| 754 | 754 | state.flowing = false; | |
| 755 | 755 | ||
| 756 | 756 | for (var i = 0; i < len; i++) | |
| 757 | - dests[i].emit('unpipe', this, unpipeInfo); | ||
| 757 | + dests[i].emit('unpipe', this, { hasUnpiped: false }); | ||
| 758 | 758 | return this; | |
| 759 | 759 | } | |
| 760 | 760 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -31,3 +31,57 @@ source.unpipe(dest2); | |||
| 31 | 31 | source.unpipe(dest1); | |
| 32 | 32 | ||
| 33 | 33 | assert.strictEqual(source._readableState.pipes, null); | |
| 34 | + | ||
| 35 | + { | ||
| 36 | + // test `cleanup()` if we unpipe all streams. | ||
| 37 | + const source = Readable({ read: () => {} }); | ||
| 38 | + const dest1 = Writable({ write: () => {} }); | ||
| 39 | + const dest2 = Writable({ write: () => {} }); | ||
| 40 | + | ||
| 41 | + let destCount = 0; | ||
| 42 | + const srcCheckEventNames = ['end', 'data']; | ||
| 43 | + const destCheckEventNames = ['close', 'finish', 'drain', 'error', 'unpipe']; | ||
| 44 | + | ||
| 45 | + const checkSrcCleanup = common.mustCall(() => { | ||
| 46 | + assert.strictEqual(source._readableState.pipes, null); | ||
| 47 | + assert.strictEqual(source._readableState.pipesCount, 0); | ||
| 48 | + assert.strictEqual(source._readableState.flowing, false); | ||
| 49 | + | ||
| 50 | + srcCheckEventNames.forEach((eventName) => { | ||
| 51 | + assert.strictEqual( | ||
| 52 | + source.listenerCount(eventName), 0, | ||
| 53 | + `source's '${eventName}' event listeners not removed` | ||
| 54 | + ); | ||
| 55 | + }); | ||
| 56 | + }); | ||
| 57 | + | ||
| 58 | + function checkDestCleanup(dest) { | ||
| 59 | + const currentDestId = ++destCount; | ||
| 60 | + source.pipe(dest); | ||
| 61 | + | ||
| 62 | + const unpipeChecker = common.mustCall(() => { | ||
| 63 | + assert.deepStrictEqual( | ||
| 64 | + dest.listeners('unpipe'), [unpipeChecker], | ||
| 65 | + `destination{${currentDestId}} should have a 'unpipe' event ` + | ||
| 66 | + 'listener which is `unpipeChecker`' | ||
| 67 | + ); | ||
| 68 | + dest.removeListener('unpipe', unpipeChecker); | ||
| 69 | + destCheckEventNames.forEach((eventName) => { | ||
| 70 | + assert.strictEqual( | ||
| 71 | + dest.listenerCount(eventName), 0, | ||
| 72 | + `destination{${currentDestId}}'s '${eventName}' event ` + | ||
| 73 | + 'listeners not removed' | ||
| 74 | + ); | ||
| 75 | + }); | ||
| 76 | + | ||
| 77 | + if (--destCount === 0) | ||
| 78 | + checkSrcCleanup(); | ||
| 79 | + }); | ||
| 80 | + | ||
| 81 | + dest.on('unpipe', unpipeChecker); | ||
| 82 | + } | ||
| 83 | + | ||
| 84 | + checkDestCleanup(dest1); | ||
| 85 | + checkDestCleanup(dest2); | ||
| 86 | + source.unpipe(); | ||
| 87 | + } | ||
0 commit comments