[LLVM][Cygwin] add workaround for blocking connect/accept in AF_UNIX sockets

[jeremyd2019]

On Cygwin, UNIX sockets involve a handshake between connect and accept to enable SO_PEERCRED/getpeereid handling. This necessitates accept being called before connect can return, but at least the tests in llvm/unittests/Support/raw_socket_stream_test do both on the same thread (first connect and then accept), resulting in a deadlock. Add a call to both places sockets are created that turns off the handshake (and SO_PEERCRED/getpeereid support).

References:

• https://github.com/cygwin/cygwin/blob/cec8a6680ea1fe38f38001b06c34ae355a785209/winsup/cygwin/fhandler/socket_local.cc#L1462-L1471
• https://inbox.sourceware.org/cygwin/[email protected]/T/#u

[llvmbot]

@llvm/pr-subscribers-llvm-support

Author: None (jeremyd2019)

Changes

On Cygwin, UNIX sockets involve a handshake between connect and accept to enable SO_PEERCRED/getpeereid handling. This necessitates accept being called before connect can return, but at least the tests in llvm/unittests/Support/raw_socket_stream_test do both on the same thread (first connect and then accept), resulting in a deadlock. Add a call to both places sockets are created that turns off the handshake (and SO_PEERCRED/getpeereid support).

---

Full diff: https://github.com/llvm/llvm-project/pull/140353.diff

1 Files Affected:

• (modified) llvm/lib/Support/raw_socket_stream.cpp (+18)

diff --git a/llvm/lib/Support/raw_socket_stream.cpp b/llvm/lib/Support/raw_socket_stream.cpp
index 7a4be5759f900..fd1c681672138 100644
--- a/llvm/lib/Support/raw_socket_stream.cpp
+++ b/llvm/lib/Support/raw_socket_stream.cpp
@@ -81,6 +81,15 @@ static Expected<int> getSocketFD(StringRef SocketPath) {
                                          "Create socket failed");
   }
 
+#ifdef __CYGWIN__
+  // On Cygwin, UNIX sockets involve a handshake between connect and accept
+  // to enable SO_PEERCRED/getpeereid handling.  This necessitates accept being
+  // called before connect can return, but at least the tests in
+  // llvm/unittests/Support/raw_socket_stream_test do both on the same thread
+  // (first connect and then accept), resulting in a deadlock.  This call turns
+  // off the handshake (and SO_PEERCRED/getpeereid support).
+  setsockopt(Socket, SOL_SOCKET, SO_PEERCRED, NULL, 0);
+#endif
   struct sockaddr_un Addr = setSocketAddr(SocketPath);
   if (::connect(Socket, (struct sockaddr *)&Addr, sizeof(Addr)) == -1)
     return llvm::make_error<StringError>(getLastSocketErrorCode(),
@@ -147,6 +156,15 @@ Expected<ListeningSocket> ListeningSocket::createUnix(StringRef SocketPath,
     return llvm::make_error<StringError>(getLastSocketErrorCode(),
                                          "socket create failed");
 
+#ifdef __CYGWIN__
+  // On Cygwin, UNIX sockets involve a handshake between connect and accept
+  // to enable SO_PEERCRED/getpeereid handling.  This necessitates accept being
+  // called before connect can return, but at least the tests in
+  // llvm/unittests/Support/raw_socket_stream_test do both on the same thread
+  // (first connect and then accept), resulting in a deadlock.  This call turns
+  // off the handshake (and SO_PEERCRED/getpeereid support).
+  setsockopt(Socket, SOL_SOCKET, SO_PEERCRED, NULL, 0);
+#endif
   struct sockaddr_un Addr = setSocketAddr(SocketPath);
   if (::bind(Socket, (struct sockaddr *)&Addr, sizeof(Addr)) == -1) {
     // Grab error code from call to ::bind before calling ::close

[jeremyd2019]

Cc @mstorsjo @tyan0
See also https://github.com/cygwin/cygwin/blob/cec8a6680ea1fe38f38001b06c34ae355a785209/winsup/cygwin/fhandler/socket_local.cc#L1462-L1471 and https://inbox.sourceware.org/cygwin/[email protected]/T/#u

[mstorsjo]

LGTM, but please add the reference links (from the later comment you posted) in the PR description so they'll end up in the commit message. After reading the commit itself I had questions about exactly that, which those links explained, so I think they're essential for later understanding of this commit.

[jeremyd2019]

Does that work? BTW, I couldn't find any users of raw_socket_stream grepping in llvm-project git worktree, except for the test.

[mstorsjo]

Does that work?

Looks ok here; when merging, one gets to see how it would look in the plaintext form (and one is given a chance to tweak it if necessary).

BTW, I couldn't find any users of raw_socket_stream grepping in llvm-project git worktree, except for the test.

That's odd; perhaps it used to be used somewhere earlier.

[jeremyd2019]

With this commit, and a fix to Cygwin due to a deadlock reading and writing to a fifo from different threads in the same process, now the llvm tests don't deadlock on Cygwin anymore and we can look at fixing the ones that fail in an ordinary fashion 😁