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

New exception behavior in recv causes existing code to break #215

Closed
snoyberg opened this issue Jul 28, 2016 · 10 comments
Closed

New exception behavior in recv causes existing code to break #215

snoyberg opened this issue Jul 28, 2016 · 10 comments

Comments

@snoyberg
Copy link

Consider the following code snippet:

#!/usr/bin/env stack
{- stack
      --resolver ghc-7.10.3
      runghc
      --package network-2.6.3.0
      --package async-2.1.0
      --package stm-2.4.4.1
 -}
{-# OPTIONS_GHC -Wall -Werror #-}
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE OverloadedStrings #-}
import Control.Concurrent.Async (race_)
import Control.Exception (bracket, bracketOnError)
import Control.Monad (void, forever, unless)
import Data.Function (fix)
import Network (listenOn, sClose, PortID(..), socketPort)
import Network.Socket
       (Socket, SockAddr(..), accept, PortNumber, socket, Family(AF_INET),
        SocketType(Stream), connect)
import Network.BSD (getProtocolNumber, getHostByName, hostAddress)
import Network.Socket.ByteString (recv, send)
import qualified Data.ByteString as S

main :: IO ()
main = bracket (listenOn (PortNumber 0)) sClose $ \listener -> do
    port <- socketPort listener
    race_ (server listener) (client port)

server :: Socket -> IO ()
server listener = forever $ bracket
    (accept listener)
    (sClose . fst)
    (\(s, _) -> void $ send s "Hello World\n")

client :: PortID -> IO ()
client (PortNumber port) = do
    s <- connectTo "localhost" port
    fix $ \loop -> do
        bs <- recv s 1024
        unless (S.null bs) $ do
            print bs
            loop
client p = error $ "Invalid PortID: " ++ show p

connectTo :: String -> PortNumber -> IO Socket
connectTo hostname port = do
    proto <- getProtocolNumber "tcp"
    bracketOnError
        (socket AF_INET Stream proto)
        (sClose)  -- only done if there's an error
        (\sock -> do
          he <- getHostByName hostname
          connect sock (SockAddrInet port (hostAddress he))
          return sock
        )

When I run it on my machine, I get the output:

"Hello World\n"
Main.hs: Network.Socket.recvBuf: end of file (end of file)

By contrast, if I change the network package version to 2.6.2.1, the program does not exit with an exception. The previous behavior - and behavior that many programs rely on - is that in the case of a closed connection an empty bytestring is returned. I would consider the older behavior preferable, and the change a bug.

Original issue was opened against stackage for a number of failures: commercialhaskell/stackage#1739

@eborden
Copy link
Collaborator

eborden commented Jul 28, 2016

Agreed on your analysis @snoyberg. We'll need to get a minimal test case in to guard against this type of behavior change in the future.

@eborden
Copy link
Collaborator

eborden commented Jul 28, 2016

Looks to be introduced in #200. I'll dig a bit deeper later and see about producing a fix and minimal test case.

@snoyberg
Copy link
Author

Awesome, thank you for looking into this so quickly! I think the example I gave above should be distillable into a minimal test case. Let me know if you'd like me to do anything to assist on this.

@eborden
Copy link
Collaborator

eborden commented Jul 29, 2016

Well, the culprit is here:
https://github.com/haskell/network/blob/master/Network/Socket.hsc#L739

The recv family of functions in Socket differ from the Socket.ByteString family in throwing an EOF. This difference is certainly annoying, but easy enough to fix.

The path for now will be reintroducing duplication.

@eborden
Copy link
Collaborator

eborden commented Jul 29, 2016

Well the fix is available with a minimally altered version of your case. I haven't had a chance to attempt to boil it down to anything simpler, but the current formulation can suffice.

@snoyberg If you have some time to reduce it this weekend let me know. I'll be preoccupied with moving, so I won't have time. Otherwise I'd say this can drop with a point release.

@kazu-yamamoto Any thoughts?

@trofi
Copy link
Contributor

trofi commented Jul 30, 2016

Here is minimal reproducer for you (extracted from simple-sendfile-0.2.25):

module Main (main) where

import qualified Network.Socket as NS
import qualified Network.Socket.ByteString as NSB

main :: IO ()
main = do
    (s1,s2) <- NS.socketPair NS.AF_UNIX NS.Stream 0
    NS.close s1
    NSB.recv s2 1 >>= print
    NS.close s2

network-2.6.3.0:

$ runhaskell bug.hs 
bug.hs: Network.Socket.recvBuf: end of file (end of file)

network-2.6.2.1:

$ runhaskell bug.hs 
""

trofi added a commit to gentoo-haskell/gentoo-haskell that referenced this issue Jul 30, 2016
network-2.6.3.0 throws exceptions in ByteString
API for 'recv'. That causes a lot of applications
and libraries to break. Some examples:

  dev-haskell/simple-sendfile-0.2.25
  dev-haskell/fluent-logger-0.2.3.1
  dev-haskell/http-client-0.4.30
  dev-haskell/dbus-0.10.12

Bug: haskell/network#215

Package-Manager: portage-2.3.0
RepoMan-Options: --force
@eborden
Copy link
Collaborator

eborden commented Jul 30, 2016

@trofi Perfect!

@eborden
Copy link
Collaborator

eborden commented Jul 30, 2016

#216 Is running through CI with the new minimal case. Thank you @trofi. When everything is green I'll prep this for a point release.

@eborden
Copy link
Collaborator

eborden commented Jul 31, 2016

A point release has been uploaded to hackage. Thank you both for your help.

@eborden eborden closed this as completed Jul 31, 2016
@snoyberg
Copy link
Author

Thank you!

On Sun, Jul 31, 2016, 6:52 AM Evan Borden [email protected] wrote:

Closed #215 #215.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#215 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AADBB_KB3q3A-JhCPkvTf7tI_4S4f9JZks5qbBv0gaJpZM4JXA2h
.

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

No branches or pull requests

3 participants