net: noop destroyed socket #30839
net: noop destroyed socket #30839
Conversation
ronag
force-pushed
the
net-destroy
branch
3 times, most recently
from
b82e63a
to
0013206
Compare
_write and _read can be called from 'connect' after Socket.destroy() has been called. This should be a noop. Refs: nodejs#30837
| @@ -571,7 +572,11 @@ Socket.prototype._read = function(n) { | |||
|
|
|||
| if (this.connecting || !this._handle) { | |||
| debug('_read wait for connection'); | |||
| this.once('connect', () => this._read(n)); | |||
| this.once('connect', () => { | |||
| if (!this.destroyed) { | |||
There was a problem hiding this comment.
_handle.onread is already set to a noop after destroy so I don't think this is needed. Also 'connect' should not be emitted if the socket is destroyed while connecting.
There was a problem hiding this comment.
Hm, I see. I can remove this. I still kind of like it since it makes it more explicit.
There was a problem hiding this comment.
It would be code that is impossible to cover with tests and this is already explicit
Lines 1106 to 1108 in cf5ce2c
|
@lpinca: given your feedback this PR doesn't seem to do anything. However, we do have a different bug. The callback for |
|
I'm fine with either as long as it's consistent and documented. |
_write and _read can be called from 'connect' after
Socket.destroy() has been called. This should be a noop.
Refs: #30837
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes