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

Fix TCP socket send() immediately back to back after connect() #22630

Merged

Conversation

juj
Copy link
Collaborator

@juj juj commented Sep 26, 2024

Previously, if an emulated TCP socket is connect()ed and one attempts to send() back to back right after connecting, the send would fail because the socket is not yet actually connected.

It is a bit hard to imagine where such behavior would be useful.

So this PR changes this behavior to be identical to how connectionless UDP sockets are emulated: instead, connect() calls are always pretended to succeed (since we cannot synchronously establish if the connect would fail, so presume it'll work), and all send() calls that are issued while the socket is connecting will be queued to be sent after the socket actually connects.

@juj juj force-pushed the fix_tcp_socket_send_immediately_after_connect branch from 7f2abe4 to 3f02d35 Compare September 26, 2024 14:19
Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm % nits

test/test_sockets.py Outdated Show resolved Hide resolved
@@ -350,6 +350,11 @@ def test_posix_proxy_sockets(self):
# Build and run the TCP echo client program with Emscripten
self.btest_exit('websocket/tcp_echo_client.c', args=['-lwebsocket', '-sPROXY_POSIX_SOCKETS', '-pthread', '-sPROXY_TO_PTHREAD'])

# Test that multiple pthreads calling send() on the same socket produces correct ordering semantics.
def test_sockets_send_immediately_after_connect(self):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about calling it test_sockets_send_while_connecting?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure

@juj juj enabled auto-merge (squash) September 27, 2024 12:13
… to send() back to back right after connecting, the send would fail because the socket is not yet actually connected.

It is a bit hard to imagine where that behavior would be useful.

So this PR changes this behavior to be identical to how connectionless UDP sockets are emulated: instead, connect() calls are always pretended to succeed (since we cannot synchronously establish if the connect would fail, so presume it'll work), and all send() calls that are issued while the socket is connecting will be queued to be sent after the socket actually connects.
@juj juj force-pushed the fix_tcp_socket_send_immediately_after_connect branch from 6fbbb76 to cdd575e Compare September 30, 2024 11:30
emscripten-core#22662

Merge remote-tracking branch 'origin/main' into fix_tcp_socket_send_immediately_after_connect
@juj juj merged commit 7df48f6 into emscripten-core:main Oct 2, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants