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.

Sign up for GitHub

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

implementing socketToFd #424

Merged
merged 3 commits into from
Sep 17, 2019
Merged

implementing socketToFd #424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
72729e4
implementing socketToFd
kazu-yamamoto Sep 3, 2019
d0bba03
suggestion for doc from vdukhovni.
kazu-yamamoto Sep 5, 2019
27d9815
Supporting socket2fd on Windows
kazu-yamamoto Sep 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Network/Socket.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ module Network.Socket
, withFdSocket
, unsafeFdSocket
, touchSocket
, socketToFd
, fdSocket
, mkSocket
, socketToHandle
Expand Down
28 changes: 28 additions & 0 deletions Network/Socket/Types.hsc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ module Network.Socket.Types (
, withFdSocket
, unsafeFdSocket
, touchSocket
, socketToFd
, fdSocket
, mkSocket
, invalidateSocket
Expand Down Expand Up @@ -166,6 +167,33 @@ withFdSocket (Socket ref _) f = do
touch ref
return r

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

@vdukhovni vdukhovni Sep 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks and done.

-- 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.
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
Copy link

Choose a reason for hiding this comment

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

[ 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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Mistuke Would you suggest a proper way on Windows?

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No problem. I can wait. Thanks in advance!

close s
return fd2

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

-- | Creating a socket from a file descriptor.
mkSocket :: CInt -> IO Socket
mkSocket fd = do
Expand Down
15 changes: 15 additions & 0 deletions cbits/initWinSock.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
10 changes: 10 additions & 0 deletions tests/Network/SocketSpec.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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