Skip to content

Commit 5c637a6

Browse files
authored
[library_sockfs] Report socket connect failures via select() (#22818)
When connect fails on a NON_BLOCKING socket the socket is supposed to become writable and the status made available via SO_ERROR. Without this change `test_sockets_echo_client.c` will loop forever polling for read/write status on the socket fd.
1 parent 7cff758 commit 5c637a6

File tree

3 files changed

+33
-4
lines changed

3 files changed

+33
-4
lines changed

src/library_sockfs.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,7 @@ addToLibrary({
281281
#endif
282282

283283
Module['websocket'].emit('open', sock.stream.fd);
284+
sock.connecting = false;
284285

285286
try {
286287
var queued = peer.msg_send_queue.shift();
@@ -402,7 +403,15 @@ addToLibrary({
402403

403404
if ((dest && dest.socket.readyState === dest.socket.CLOSING) ||
404405
(dest && dest.socket.readyState === dest.socket.CLOSED)) {
405-
mask |= {{{ cDefs.POLLHUP }}};
406+
// When an non-blocking connect fails mark the socket as writable.
407+
// Its up to the calling code to then use getsockopt with SO_ERROR to
408+
// retrieve the error.
409+
// See https://man7.org/linux/man-pages/man2/connect.2.html
410+
if (sock.connecting) {
411+
mask |= {{{ cDefs.POLLOUT }}};
412+
} else {
413+
mask |= {{{ cDefs.POLLHUP }}};
414+
}
406415
}
407416

408417
return mask;
@@ -496,6 +505,7 @@ addToLibrary({
496505
// because we cannot synchronously block to wait for the WebSocket
497506
// connection to complete, we return here pretending that the connection
498507
// was a success.
508+
sock.connecting = true;
499509
},
500510
listen(sock, backlog) {
501511
if (!ENVIRONMENT_IS_NODE) {

test/sockets/test_sockets_echo_client.c

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#endif
3030

3131
typedef enum {
32+
MSG_CONNECT,
3233
MSG_READ,
3334
MSG_WRITE
3435
} msg_state_t;
@@ -109,6 +110,21 @@ void main_loop() {
109110
}
110111
}
111112

113+
if (server.state == MSG_CONNECT) {
114+
if (!FD_ISSET(server.fd, &fdw)) {
115+
return;
116+
}
117+
int error = 0;
118+
socklen_t len = sizeof(error);
119+
int ret = getsockopt(server.fd, SOL_SOCKET, SO_ERROR, &error, &len);
120+
if (error != 0) {
121+
printf("connect failed: %s\n", strerror(error));
122+
finish(EXIT_FAILURE);
123+
}
124+
printf("connected\n");
125+
server.state = MSG_WRITE;
126+
}
127+
112128
if (server.state == MSG_WRITE) {
113129
if (!FD_ISSET(server.fd, &fdw)) {
114130
return;
@@ -160,7 +176,7 @@ int main() {
160176
int res;
161177

162178
memset(&server, 0, sizeof(server_t));
163-
server.state = MSG_WRITE;
179+
server.state = MSG_CONNECT;
164180

165181
// setup the message we're going to echo
166182
memset(&echo_msg, 0, sizeof(msg_t));
@@ -191,7 +207,7 @@ int main() {
191207
perror("inet_pton failed");
192208
finish(EXIT_FAILURE);
193209
}
194-
210+
195211
res = connect(server.fd, (struct sockaddr *)&addr, sizeof(addr));
196212
if (res == -1 && errno != EINPROGRESS) {
197213
perror("connect failed");

test/test_sockets.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
import clang_native
1919
import common
2020
from common import BrowserCore, no_windows, create_file, test_file, read_file
21-
from common import parameterized, requires_native_clang, crossplatform, PYTHON
21+
from common import parameterized, requires_native_clang, crossplatform, PYTHON, NON_ZERO
2222
from tools import config, utils
2323
from tools.shared import EMCC, path_from_root, run_process, CLANG_CC
2424

@@ -292,6 +292,9 @@ def test_nodejs_sockets_echo(self, harness_class, port, args):
292292
expected = 'do_msg_read: read 14 bytes'
293293
self.do_runf('sockets/test_sockets_echo_client.c', expected, emcc_args=['-DSOCKK=%d' % harness.listen_port] + args)
294294

295+
def test_nodejs_sockets_connect_failure(self):
296+
self.do_runf('sockets/test_sockets_echo_client.c', 'connect failed: Connection refused', emcc_args=['-DSOCKK=666'], assert_returncode=NON_ZERO)
297+
295298
@requires_native_clang
296299
def test_nodejs_sockets_echo_subprotocol(self):
297300
# Test against a Websockified server with compile time configured WebSocket subprotocol. We use a Websockified

0 commit comments

Comments
 (0)