Commit 88eb619
stream: fix duplicate logic in stream destroy
Fix duplicate logic in stream destroy as the same logic is being shared
across methods and thus can be encapsulated into a single method.
PR-URL: #35727
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>1 parent e0a1541 commit 88eb619
1 file changed
Lines changed: 16 additions & 22 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -8,6 +8,20 @@ const { Symbol } = primordials; | |||
| 8 | 8 | const kDestroy = Symbol('kDestroy'); | |
| 9 | 9 | const kConstruct = Symbol('kConstruct'); | |
| 10 | 10 | ||
| 11 | + function checkError(err, w, r) { | ||
| 12 | + if (err) { | ||
| 13 | + // Avoid V8 leak, https://github.com/nodejs/node/pull/34103#issuecomment-652002364 | ||
| 14 | + err.stack; | ||
| 15 | + | ||
| 16 | + if (w && !w.errored) { | ||
| 17 | + w.errored = err; | ||
| 18 | + } | ||
| 19 | + if (r && !r.errored) { | ||
| 20 | + r.errored = err; | ||
| 21 | + } | ||
| 22 | + } | ||
| 23 | + } | ||
| 24 | + | ||
| 11 | 25 | // Backwards compat. cb() is undocumented and unused in core but | |
| 12 | 26 | // unfortunately might be used by modules. | |
| 13 | 27 | function destroy(err, cb) { | |
@@ -24,20 +38,10 @@ function destroy(err, cb) { | |||
| 24 | 38 | return this; | |
| 25 | 39 | } | |
| 26 | 40 | ||
| 27 | - if (err) { | ||
| 28 | - // Avoid V8 leak, https://github.com/nodejs/node/pull/34103#issuecomment-652002364 | ||
| 29 | - err.stack; | ||
| 30 | - | ||
| 31 | - if (w && !w.errored) { | ||
| 32 | - w.errored = err; | ||
| 33 | - } | ||
| 34 | - if (r && !r.errored) { | ||
| 35 | - r.errored = err; | ||
| 36 | - } | ||
| 37 | - } | ||
| 38 | 41 | ||
| 39 | 42 | // We set destroyed to true before firing error callbacks in order | |
| 40 | 43 | // to make it re-entrance safe in case destroy() is called within callbacks | |
| 44 | + checkError(err, w, r); | ||
| 41 | 45 | ||
| 42 | 46 | if (w) { | |
| 43 | 47 | w.destroyed = true; | |
@@ -66,17 +70,7 @@ function _destroy(self, err, cb) { | |||
| 66 | 70 | ||
| 67 | 71 | called = true; | |
| 68 | 72 | ||
| 69 | - if (err) { | ||
| 70 | - // Avoid V8 leak, https://github.com/nodejs/node/pull/34103#issuecomment-652002364 | ||
| 71 | - err.stack; | ||
| 72 | - | ||
| 73 | - if (w && !w.errored) { | ||
| 74 | - w.errored = err; | ||
| 75 | - } | ||
| 76 | - if (r && !r.errored) { | ||
| 77 | - r.errored = err; | ||
| 78 | - } | ||
| 79 | - } | ||
| 73 | + checkError(err, w, r); | ||
| 80 | 74 | ||
| 81 | 75 | if (w) { | |
| 82 | 76 | w.closed = true; | |
0 commit comments