Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: writable is not set to false after socket has ended #36029

Closed
mscdex opened this issue Nov 8, 2020 · 1 comment
Closed

net: writable is not set to false after socket has ended #36029

mscdex opened this issue Nov 8, 2020 · 1 comment
Labels
net Issues and PRs related to the net subsystem. regression Issues related to regressions.

Comments

@mscdex
Copy link
Contributor

mscdex commented Nov 8, 2020

  • Version: v15.0.0-v15.1.0, master
  • Platform: Linux
  • Subsystem: net

What steps will reproduce the bug?

const net = require('net');

net.createServer(function(s) {
  this.close();
  s.end();
}).listen(0, 'localhost', function() {
  const socket = net.connect(this.address().port, 'localhost');
  socket.on('end', () => {
    console.log('socket.writable', socket.writable);
    if (socket.writable)
      socket.write('hello world');
  });
});

How often does it reproduce? Is there a required condition?

Every time.

What is the expected behavior?

No error emitted on the socket.

Output (on node v14.x and earlier):

socket.writable false

What do you see instead?

Output:

socket.writable true
node:events:304
      throw er; // Unhandled 'error' event
      ^

Error: This socket has been ended by the other party
    at Socket.writeAfterFIN [as write] (node:net:443:14)
    at Socket.<anonymous> (/tmp/test.js:11:14)
    at Socket.emit (node:events:339:22)
    at endReadableNT (node:internal/streams/readable:1289:12)
    at processTicksAndRejections (node:internal/process/task_queues:80:21)
Emitted 'error' event on Socket instance at:
    at emitErrorNT (node:internal/streams/destroy:188:8)
    at emitErrorCloseNT (node:internal/streams/destroy:153:3)
    at processTicksAndRejections (node:internal/process/task_queues:80:21) {
  code: 'EPIPE'
}

Additional information

The description for writable.writable in the streams docs reads:

Is true if it is safe to call writable.write(), which means the stream has not been destroyed, errored or ended.

So I believe this is a regression.

/cc @nodejs/net

@mscdex mscdex added net Issues and PRs related to the net subsystem. regression Issues related to regressions. labels Nov 8, 2020
@mscdex mscdex changed the title net: writable is not set to false after socket is destroyed net: writable is not set to false after socket has ended Nov 8, 2020
@mscdex
Copy link
Contributor Author

mscdex commented Nov 8, 2020

Looks like efefdd6 is the culprit.

/cc @ronag

ronag added a commit to nxtedition/node that referenced this issue Nov 8, 2020
Don't error if not ended.

Fixes: nodejs#36029
@aduh95 aduh95 closed this as completed in f7f0a6a Nov 10, 2020
codebytere pushed a commit that referenced this issue Nov 22, 2020
Don't error if not ended.

Fixes: #36029

PR-URL: #36043
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
net Issues and PRs related to the net subsystem. regression Issues related to regressions.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant