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: write(cb) not called if destroy():ed before 'connect' #30841

Closed
ronag opened this issue Dec 7, 2019 · 7 comments · Fixed by #45922
Closed

net: write(cb) not called if destroy():ed before 'connect' #30841

ronag opened this issue Dec 7, 2019 · 7 comments · Fixed by #45922
Labels
confirmed-bug Issues with confirmed bugs. net Issues and PRs related to the net subsystem.

Comments

@ronag
Copy link
Member

ronag commented Dec 7, 2019

See #30839

_writeGeneric waits for 'connect'

node/lib/net.js

Line 759 in cf5ce2c

this.once('connect', function connect() {
which might never be emitted due to

node/lib/net.js

Lines 1106 to 1108 in cf5ce2c

if (self.destroyed) {
return;
}

I believe this will fail:

const socket = createSocketBeforeConnect();
socket.write('asd', common.mustCall());
socket.destroy();
@ronag ronag changed the title net: write(cb) not called if destroyed net: write(cb) not called if destroy():ed before 'connect' Dec 7, 2019
@ronag ronag changed the title net: write(cb) not called if destroy():ed before 'connect' net: write(cb) not called if destroy():ed before 'connect' Dec 7, 2019
@lpinca lpinca added the net Issues and PRs related to the net subsystem. label Dec 7, 2019
@ronag
Copy link
Member Author

ronag commented Jan 3, 2020

This is fixed

@ronag ronag closed this as completed Jan 3, 2020
@lpinca
Copy link
Member

lpinca commented Jan 5, 2020

If the expectation is that callback is called this is not fixed.

const assert = require('assert');
const net = require('net');

const socket = new net.Socket();

socket.connect({
  port: 80,
  host: 'example.com',
  lookup() {}
});

assert(socket.connecting);

let called = false;

socket.write('foo', function() {
  called = true;
});

socket.destroy();

process.on('exit', function() {
  assert(called);
});

@ronag
Copy link
Member Author

ronag commented Jan 5, 2020

Yea, we should add a test for that.

This seems related to the whole constructing state which in this case never gets resolved, i.e. _write never finishes https://github.com/nodejs/node/blob/master/lib/net.js#L763. I can dig into this once/if #29656 lands.

@lpinca
Copy link
Member

lpinca commented Jan 5, 2020

yes, _writeGeneric() is never called as 'connect' is never emitted.

@ronag
Copy link
Member Author

ronag commented Jan 6, 2020

pending resolution to discussion in #31179

@ronag
Copy link
Member Author

ronag commented Apr 19, 2020

'use strict';

const common = require('../common');
const assert = require('assert');
const net = require('net');

const socket = new net.Socket();

socket.connect({
  port: 80,
  host: 'example.com',
  lookup() {}
});

assert(socket.connecting);

socket.write('foo', common.mustCall());
socket.destroy();

@ronag
Copy link
Member Author

ronag commented Apr 19, 2020

This requires #29656 in order to be fully resolved.

@ronag ronag added the confirmed-bug Issues with confirmed bugs. label Apr 19, 2020
santigimeno added a commit to santigimeno/node that referenced this issue Dec 20, 2022
Make sure that when calling `write()` on a connecting socket, the
callback is called if the socket is destroyed before the connection is
established.

Fixes: nodejs#30841
santigimeno added a commit to santigimeno/node that referenced this issue Dec 26, 2022
Make sure that when calling `write()` on a connecting socket, the
callback is called if the socket is destroyed before the connection is
established.

Fixes: nodejs#30841
santigimeno added a commit to santigimeno/node that referenced this issue Dec 27, 2022
Make sure that when calling `write()` on a connecting socket, the
callback is called if the socket is destroyed before the connection is
established.

Fixes: nodejs#30841
nodejs-github-bot pushed a commit that referenced this issue Jan 1, 2023
Make sure that when calling `write()` on a connecting socket, the
callback is called if the socket is destroyed before the connection is
established.

Fixes: #30841
PR-URL: #45922
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
RafaelGSS pushed a commit that referenced this issue Jan 2, 2023
Make sure that when calling `write()` on a connecting socket, the
callback is called if the socket is destroyed before the connection is
established.

Fixes: #30841
PR-URL: #45922
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
RafaelGSS pushed a commit that referenced this issue Jan 4, 2023
Make sure that when calling `write()` on a connecting socket, the
callback is called if the socket is destroyed before the connection is
established.

Fixes: #30841
PR-URL: #45922
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
RafaelGSS pushed a commit that referenced this issue Jan 5, 2023
Make sure that when calling `write()` on a connecting socket, the
callback is called if the socket is destroyed before the connection is
established.

Fixes: #30841
PR-URL: #45922
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
juanarbol pushed a commit that referenced this issue Jan 26, 2023
Make sure that when calling `write()` on a connecting socket, the
callback is called if the socket is destroyed before the connection is
established.

Fixes: #30841
PR-URL: #45922
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
juanarbol pushed a commit that referenced this issue Jan 31, 2023
Make sure that when calling `write()` on a connecting socket, the
callback is called if the socket is destroyed before the connection is
established.

Fixes: #30841
PR-URL: #45922
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
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
confirmed-bug Issues with confirmed bugs. net Issues and PRs related to the net subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants