network's example echo server seems leaking socket on error binding

Hi folks, I just filed an issue https://github.com/haskell/network/issues/447 https://github.com/haskell/network/issues/447 with the network package, I'd like to post here too in seeking your wisedom at the same time. In top-level doc, the minimal example echo server uses Control.Exception.bracket like this: E.bracket (open addr) close loop where resolve = do let hints = defaultHints { addrFlags = [AI_PASSIVE] , addrSocketType = Stream } head <$> getAddrInfo (Just hints) mhost (Just port) open addr = do sock <- socket (addrFamily addr) (addrSocketType addr) (addrProtocol addr) setSocketOption sock ReuseAddr 1 withFdSocket sock setCloseOnExecIfNeeded bind sock $ addrAddress addr listen sock 1024 return sock loop sock = forever $ do (conn, _peer) <- accept sock void $ forkFinally (server conn) (const $ gracefulClose conn 5000) I happened to copy the configuration with another machine's IP address to run on my dev machine, then of course it failed binding to the IP, but in this case I suspect sock above is leaked without close, as open failed at all, so sock is not given to bracket for it to do the cleanup. Is my suspicion correct ? Or I missed something that the example actually won't leak on binding errors ? Best regards, Compl

On 9 May 2020, at 10:15, Compl Yue
wrote: Is my suspicion correct ? Or I missed something that the example actually won't leak on binding errors ?
You are correct, the allocation in "open" must be atomic (so either fully succeed or allocate nothing), else it will leak resources. The open here needs further bracketing internally to properly/safely handle this case. One solution would be to use bracketOnError (which only runs the cleanup on exception in the body), creating the socket in the "alloc" of bracketOnError, then do the binding inside the body of bracketOnError and returning the bound socket. So open addr = bracketOnError (socket (addrFamily addr) (addrSocketType addr) (addrProtocol addr)) close $ \sock -> do setSocketOption sock ReuseAddr 1 withFdSocket sock setCloseOnExecIfNeeded bind sock $ addrAddress addr listen sock 1024 return sock Alternatively this can be addressed by Acquire from the resourcet package. Cheers, Merijn

On Sat, 9 May 2020, Compl Yue wrote:
In top-level doc, the minimal example echo server uses Control.Exception.bracket like this:
E.bracket (open addr) close loop where resolve = do let hints = defaultHints { addrFlags = [AI_PASSIVE] , addrSocketType = Stream } head <$> getAddrInfo (Just hints) mhost (Just port) open addr = do sock <- socket (addrFamily addr) (addrSocketType addr) (addrProtocol addr) setSocketOption sock ReuseAddr 1 withFdSocket sock setCloseOnExecIfNeeded bind sock $ addrAddress addr listen sock 1024 return sock loop sock = forever $ do (conn, _peer) <- accept sock void $ forkFinally (server conn) (const $ gracefulClose conn 5000)
I happened to copy the configuration with another machine's IP address to run on my dev machine, then of course it failed binding to the IP, but in this case I suspect sock above is leaked without close, as open failed at all, so sock is not given to bracket for it to do the cleanup.
I think you are right. If 'bind' throws an exception then 'close' will not be called. You might move the block from 'setSocketOption' to 'listen' into 'loop' before 'forever'.
participants (3)
-
Compl Yue
-
Henning Thielemann
-
Merijn Verstraaten