implementing socketToFd by kazu-yamamoto · Pull Request #424 · haskell/network

kazu-yamamoto · 2019-09-03T03:09:31Z

@vdukhovni This is a PoC. I'm not sure that this implements what you want. Please give me your advice.

kazu-yamamoto · 2019-09-03T03:11:59Z

Originally discussed in #423.

vdukhovni · 2019-09-03T03:23:38Z

Network/Socket/Types.hsc

+socketToFd s = do
+    fd <- unsafeFdSocket s
+    -- FIXME: throw error no if -1
+    fd2 <- c_dup fd


[ IIRC, on windows INVALID_SOCKET is not -1 since SOCKET are unsigned on Windows. ]
Also, AFAIK, Windows does not have dup(2) for sockets, but it does have DuplicateHandle().
So making this portable requires may require some special-casing for Windows, unless suitably
special-cased wrappers are already available.

That said, yes, this is the general idea.

Mysteriously, AppVeyor can build this: https://ci.appveyor.com/project/kazu-yamamoto/network/builds/27135892

@Mistuke Would you suggest a proper way on Windows?

I actually think this may work fine. DuplicateHandle() is not correct for Sockets, it's explicitly called out not to do so on it's documentation page https://docs.microsoft.com/en-us/windows/win32/api/handleapi/nf-handleapi-duplicatehandle

You should not use DuplicateHandle to duplicate handles to the following objects: * I/O completion ports. No error is returned, but the duplicate handle cannot be used. * Sockets. No error is returned, but the duplicate handle may not be recognized by Winsock a the target process. Also, using DuplicateHandle interferes with internal reference counting on the underlying object. To duplicate a socket handle, use the WSADuplicateSocket function.

and also on the docs for Socket handles https://docs.microsoft.com/en-us/windows/win32/winsock/socket-handles-2.

But reason I think this may work is because on that same page it says

A socket handle can optionally be a file handle in Windows Sockets 2. A socket handle from a Winsock provider can be used with other non-Winsock functions such as ReadFile, WriteFile, ReadFileEx, and WriteFileEx.

So a socket can be a file handle in Winsock2, unsafeFdSocket is a call to _open_osfhandle which will return a descriptor for any system file handle, so should work with sockets. and so dup should work..

So easiest way is to test it and see.

Do you mean we should merge this PR and release a new version and see what will happen?

no, I mean write a simple test to see if the fd returned by this is usable. I can also use that to check if the associated kernel object's refcount decreases when you close it.

I can do both for you if you want but it'll have to wait for the weekend :)

No problem. I can wait. Thanks in advance!

vdukhovni · 2019-09-04T12:49:17Z

Network/Socket/Types.hsc

  touch ref
  return r

+-- | Socket is closed and a duplicated file descriptor is returned.


The documentation can then say, that the duplicated descriptor is no longer subject to the possibility of unexpectedly being closed if the socket is finalized. It is now the caller's responsibility to ultimately close the duplicated file descriptor.

Thanks and done.

kazu-yamamoto · 2019-09-16T00:01:48Z

@Mistuke Ping.

Mistuke · 2019-09-16T01:05:19Z

Ah sorry! , was/am away for work. I'm traveling back today and will take care of this tomorrow after I land.

Mistuke · 2019-09-17T23:30:05Z

This should work for Windows

diff --git a/Network/Socket/Types.hsc b/Network/Socket/Types.hsc
index 54b8744..bcb4295 100644
--- a/Network/Socket/Types.hsc
+++ b/Network/Socket/Types.hsc
@@ -174,6 +174,16 @@ withFdSocket (Socket ref _) f = do
 --   duplicated file descriptor.
 socketToFd :: Socket -> IO CInt
 socketToFd s = do
+#if defined(mingw32_HOST_OS)
+    fd <- unsafeFdSocket s
+    fd2 <- c_wsaDuplicate fd
+    -- FIXME: throw error no if -1
+    close s
+    return fd2
+
+foreign import ccall unsafe "wsaDuplicate"
+   c_wsaDuplicate :: CInt -> IO CInt
+#else
     fd <- unsafeFdSocket s
     -- FIXME: throw error no if -1
     fd2 <- c_dup fd
@@ -182,6 +192,7 @@ socketToFd s = do

 foreign import ccall unsafe "dup"
    c_dup :: CInt -> IO CInt
+#endif

 -- | Creating a socket from a file descriptor.
 mkSocket :: CInt -> IO Socket
diff --git a/cbits/initWinSock.c b/cbits/initWinSock.c
index e2e8008..e720b85 100644
--- a/cbits/initWinSock.c
+++ b/cbits/initWinSock.c
@@ -40,4 +40,19 @@ initWinSock ()
   return 0;
 }

+SOCKET
+wsaDuplicate (SOCKET s)
+{
+  WSAPROTOCOL_INFOW protocolInfo;
+  if (WSADuplicateSocketW (s, GetCurrentProcessId (), &protocolInfo) != 0)
+    return -1;
+
+  SOCKET res = WSASocketW(FROM_PROTOCOL_INFO, FROM_PROTOCOL_INFO,
+                          FROM_PROTOCOL_INFO, &protocolInfo, 0, 0);
+  if (res == SOCKET_ERROR)
+    return -1;
+
+  return res;
+}
+
 #endif
diff --git a/tests/Network/SocketSpec.hs b/tests/Network/SocketSpec.hs
index 262a412..9bea422 100644
--- a/tests/Network/SocketSpec.hs
+++ b/tests/Network/SocketSpec.hs
@@ -209,3 +209,13 @@ spec = do
                     threadDelay 10000
                     void $ recv sock 1024
             tcpTest client server
+
+    describe "socketToFd" $ do
+        it "socketToFd can send using fd" $ do
+            let server sock = do
+                    void $ recv sock 1024
+                client sock = do
+                    fd <- socketToFd sock
+                    s <- mkSocket fd
+                    sendAll s "HELLO WORLD"
+            tcpTest client server

Credit: Tamar Christina <[email protected]>

kazu-yamamoto · 2019-09-17T23:45:34Z

@Mistuke Thank you for your excellent work! This PR has been merged.

vdukhovni reviewed Sep 3, 2019

View reviewed changes

kazu-yamamoto mentioned this pull request Sep 3, 2019

Releasing 3.1.1.0 #421

Closed

kazu-yamamoto force-pushed the socket2fd branch from ab0c05a to e7312bb Compare September 4, 2019 12:33

vdukhovni reviewed Sep 4, 2019

View reviewed changes

kazu-yamamoto added 2 commits September 10, 2019 11:34

implementing socketToFd

72729e4

suggestion for doc from vdukhovni.

d0bba03

kazu-yamamoto force-pushed the socket2fd branch from 47885df to d0bba03 Compare September 10, 2019 02:34

Supporting socket2fd on Windows

27d9815

Credit: Tamar Christina <[email protected]>

kazu-yamamoto added a commit to kazu-yamamoto/network that referenced this pull request Sep 17, 2019

Merge PR haskell#424

f1f6356

kazu-yamamoto merged commit 27d9815 into haskell:master Sep 17, 2019

kazu-yamamoto deleted the socket2fd branch September 17, 2019 23:45

Conversation

kazu-yamamoto commented Sep 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kazu-yamamoto commented Sep 3, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vdukhovni Sep 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kazu-yamamoto commented Sep 16, 2019

Uh oh!

Mistuke commented Sep 16, 2019

Uh oh!

Mistuke commented Sep 17, 2019

Uh oh!

kazu-yamamoto commented Sep 17, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kazu-yamamoto commented Sep 3, 2019 •

edited

Loading

vdukhovni Sep 4, 2019 •

edited

Loading