Skip to content

Commit

Permalink
tls: do not rely on 'drain' handlers in StreamWrap
Browse files Browse the repository at this point in the history
`'drain'` event handlers may not be invoked if the stream
is currently finishing. Instead, use the fact that we know
when writes are active or not, and invoke the delayed shutdown
handler from our own after-write callback.

PR-URL: #24290
Refs: #24288
Refs: #24075
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Ouyang Yadong <oyydoibh@gmail.com>
  • Loading branch information
addaleax committed Nov 16, 2018
1 parent 62d053b commit cbbf64e
Showing 1 changed file with 11 additions and 2 deletions.
13 changes: 11 additions & 2 deletions lib/internal/wrap_js_stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const { ERR_STREAM_WRAP } = require('internal/errors').codes;

const kCurrentWriteRequest = Symbol('kCurrentWriteRequest');
const kCurrentShutdownRequest = Symbol('kCurrentShutdownRequest');
const kPendingShutdownRequest = Symbol('kPendingShutdownRequest');

function isClosing() { return this[owner_symbol].isClosing(); }
function onreadstart() { return this[owner_symbol].readStart(); }
Expand Down Expand Up @@ -79,6 +80,7 @@ class JSStreamWrap extends Socket {
this.stream = stream;
this[kCurrentWriteRequest] = null;
this[kCurrentShutdownRequest] = null;
this[kPendingShutdownRequest] = null;
this.readable = stream.readable;
this.writable = stream.writable;

Expand Down Expand Up @@ -115,8 +117,10 @@ class JSStreamWrap extends Socket {
// Working around that on the native side is not quite trivial (yet?),
// so for now that is supported here.

if (this[kCurrentWriteRequest] !== null)
return this.once('drain', () => this.doShutdown(req));
if (this[kCurrentWriteRequest] !== null) {
this[kPendingShutdownRequest] = req;
return 0;
}
assert.strictEqual(this[kCurrentWriteRequest], null);
assert.strictEqual(this[kCurrentShutdownRequest], null);
this[kCurrentShutdownRequest] = req;
Expand Down Expand Up @@ -189,6 +193,11 @@ class JSStreamWrap extends Socket {
this[kCurrentWriteRequest] = null;

handle.finishWrite(req, errCode);
if (this[kPendingShutdownRequest]) {
const req = this[kPendingShutdownRequest];
this[kPendingShutdownRequest] = null;
this.doShutdown(req);
}
}

doClose(cb) {
Expand Down

0 comments on commit cbbf64e

Please sign in to comment.