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

In make-tcp-socket, close stream (not just fd) when connect fails #505

Merged
merged 1 commit into from
Jul 9, 2024

Conversation

xrme
Copy link
Member

@xrme xrme commented Jul 8, 2024

In #504, we see a problem where a socket is getting unexpectedly closed.

See the comments in the issue for details.

It might make sense to rewrite make-tcp-stream so that it doesn't create a stream at all until we have successfully created and configured the OS-level socket.

@gmpalter
Copy link
Member

gmpalter commented Jul 9, 2024

Here's an alternative that doesn't involve pulling the logic of %socket-connect out into make-tcp-socket. It catches the error signaled by %socket-connect, closes the stream and resets fd, and then lets the signaling continue.

diff --git a/library/sockets.lisp b/library/sockets.lisp
index b653b203..cd4a545f 100644
--- a/library/sockets.lisp
+++ b/library/sockets.lisp
@@ -685,7 +685,18 @@ the socket is not connected."))
                                              :port remote-port
                                              :allow-other-keys t
                                              keys))))
-               (%socket-connect fd socket-address timeout-in-milliseconds)))
+               (handler-bind (((or socket-creation-error socket-error)
+                                (lambda (c)
+                                  (declare (ignore c))
+                                  ;; When connect fails, the fd is no longer usable,
+                                  ;; and must be closed.  Because we've already made
+                                  ;; a stream with this fd, close the fd by closing
+                                  ;; the stream.
+                                  (close socket)
+                                  ;; Don't try to close fd again in the unwind-protect
+                                  ;; cleanup form.
+                                  (setq fd -1))))
+                 (%socket-connect fd socket-address timeout-in-milliseconds))))
            (setq fd -1)
            socket))
     (unless (< fd 0)

@xrme xrme force-pushed the socket-connect-fail branch from 22da4b4 to 35eb257 Compare July 9, 2024 03:31
@xrme xrme merged commit 35eb257 into master Jul 9, 2024
1 check passed
@xrme xrme deleted the socket-connect-fail branch July 9, 2024 03:51
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