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

Add unsafeFdSocket and touchSocket (Fixes #418) #423

Merged
merged 3 commits into from
Sep 3, 2019

Conversation

kazu-yamamoto
Copy link
Collaborator

Instead of #422, I would like to merge this one.

@kazu-yamamoto
Copy link
Collaborator Author

Cc: @takano-akio, @fumieval and @bitc

@bitc
Copy link

bitc commented Sep 2, 2019

This looks good for my use case 👍 (described in #418).

But I am curious what the use case is for touchFdSocket. Is it for this use case:

"from now on, I only want to work with my Socket using the fd, including closing it via the fd when I'm done"

?
If my understanding is correct, then you will need to touchFdSocket indefinitely, even after you have closed the fd, because otherwise when the GC does finally destroy the Socket, the finalizer will re-close the fd (which has already been closed, and which may now refer to some other random socket/file). But if you touchFdSocket indefinitely, then your program will have a memory leak.

It would appear to me that the correct way to handle the above scenario would be a function unrefSocket :: Socket -> IO () that changes the internal fd of the Socket to -1, without actually closing the socket. Then you wouldn't need touchFdSocket at all.

So there must be some other use case for touchFdSocket that I can't think of

@vdukhovni
Copy link

But I am curious what the use case is for touchFdSocket. Is it for this use case:

"from now on, I only want to work with my Socket using the fd, including closing it via the fd when I'm done"
?

No, that use case is not supported, in the present API sockets must be closed via the Socket close function, which must not be bypassed by directly closing the fd. The use case is only keeping the socket "alive", not effectively destroying it and using the fd directly.

For the latter, one can dup the file descriptor and then close the original socket:

import qualified System.Posix.IO as P (closeFd, dup)
    ...
    bareFd <- (fdSocket s >>= P.dup) <* close s
    -- ... Use bareFd ...
    P.closeFd bareFd

Since close needs the socket and the IORef, ... the socket should stay alive during the dup and then be closed. One can then the duped file descriptor directly in FFI calls, ...

@vdukhovni
Copy link

import qualified System.Posix.IO as P (closeFd, dup)
    ...
    bareFd <- (fdSocket s >>= P.dup) <* close s
    -- ... Use bareFd ...
    P.closeFd bareFd

Since close needs the socket and the IORef, ... the socket should stay alive during the dup and then be closed. One can then the duped file descriptor directly in FFI calls, ...

Perhaps, there should be a socketToFd function that mimics the System.Posix.IO handleToFd function. The latter returns an duped Fd and closes the handle, leaving the Fd safe from races against further operations (especially hClose) on the handle. This would basically be the code snippet above.

That is the current fdSocket (or new unsafeFdSocket) is for temporary use of the Fd while the socket is kept alive, while socketToFd is for extracting an Fd from the socket which is immediately closed (of course it must still not be in use in any other thread).

kazu-yamamoto added a commit to kazu-yamamoto/network that referenced this pull request Sep 3, 2019
@kazu-yamamoto kazu-yamamoto merged commit 713aa97 into haskell:master Sep 3, 2019
@kazu-yamamoto kazu-yamamoto deleted the touch-socket branch September 3, 2019 02:27
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.

4 participants