Skip to content
This repository

Timeout socket options are unusable #8

Open
bmillwood opened this Issue April 19, 2011 · 11 comments

3 participants

Ben Millwood Johan Tibell joeyadams
Ben Millwood

The setSockOpt API only allows setting options that take a CInt, but the RecvTimeOut option (for example) translates to SO_RCVTIMEO which expects a struct timeval. In my opinion it doesn't make any sense to mirror the underlying setsockopt C function with just one Haskell function; getsockopt is just as bad.

The underlying foreign import has a Ptr CInt argument, but the C function uses void*, so a Ptr a would probably be more appropriate. But this doesn't make a great deal of difference since it's not exported anyway.

While I'm here, does it really make sense to have an API that changes based on which computer it's compiled on? It's not a very helpful error message.

Ben Millwood

Here's my proposed API for this: two functions for every socket option, getSocketDebug, setSocketDebug, etc. These functions take or return Int or Bool or Integer or whatever. Read-only socket options have only the corresponding get* function. The functions would always be defined, but throw an IO exception if they were not available.

This schema would make the SocketOption type redundant, but presumably we'd keep it until the next major version, maybe deprecated.

This involves a lot of functions, so perhaps they should go into a separate module. That would require moving more stuff into internal modules, but there's some argument to support that anyway (e.g. it would avoid the exports of a few things that are only there to support Network.BSD).

Johan Tibell
Owner

I dislike the current design as well and I think having a setter and getter for each option makes more sense. Also, the API should be the same on all platforms and we should raise an exception if the user tries to set an option which is not supported on the platform.

Ben Millwood

Right. Any opinion on the module layout? Should I work on the master branch?

Johan Tibell
Owner

Lets put it in Network.Socket.Option. Working off the master branch should be OK for now.

joeyadams

Take a look at my network-socket-options package. Though there are a few weird things about it:

  • I use a HasSocket type class so the functions can be used on multiple types of things that contain file descriptors.

  • I define Seconds and Microseconds type aliases to clarify units. Microseconds is 64-bit, to avoid truncation issues.

  • At the end, it has helpers for setting socket timeouts, which can be used to work around GHC's lacking IO support on Windows.

These wrinkles may represent features that don't belong in a Network.Socket.Option(s) module. However, we ought to tackle the following issues somehow:

  • Windows and POSIX represent timeout values differently. Both systems treat 0 as "never time out", meaning we have to be careful not to round something to zero.

    Also, we need to be clear about what units are used for time values.

  • The user may need to set options on a raw file descriptor, not just those wrapped in the Socket data constructor.

  • The user may need to set options on a socket wrapped in a Handle (e.g. to time out network IO on Windows). However, doing so is pretty hacky.

Also, I'm not sure I like this idea:

The functions would always be defined, but throw an IO exception if they were not available.

Why not just make the set of functions platform dependent? This way, the user gets a compile-time error instead of a run-time error when a feature is missing.

Ben Millwood

Why not just make the set of functions platform dependent? This way, the user gets a compile-time error instead of a run-time error when a feature is missing.

One, because the error message is a non-obvious one, and two, because a compile-time error cannot be recovered from, whereas such a runtime error potentially could be (say, by not initialising that subsystem).

Johan Tibell
Owner

The user may need to set options on a raw file descriptor, not just those wrapped in the Socket data constructor.

This is not something I want to support in the network package. It belongs in the unix package.

The user may need to set options on a socket wrapped in a Handle (e.g. to time out network IO on Windows). However, doing so is pretty hacky.

I don't want to support this either. If it wouldn't break so much code, I would throw out Handle from the network package. It's a bad abstraction for uniformly treating things as streams. Better would be something like:

class Source where
    read :: Int -> IO ByteString
    close :: IO ()

Handle is a holy mess of buffering, Unicode, line termination handling, and whatnot.

Why not just make the set of functions platform dependent? This way, the user gets a compile-time error instead of a run-time error when a feature is missing.

Even though it's tempting at first, having a package's API vary across platforms is not a good idea (we just got rid of a bunch of such code) as it forces all client code that wants to be cross platform to exactly duplicate all the #ifdefs in the network package, in their own code.

Johan Tibell
Owner

One issue with having each option be represented as a pair of functions is that it's harder to write a function such as isOptionSupported (unless we want on such function per option). Such a function is almost a must if some options aren't supported on some platforms but we still export setters/getters for them (which we want). Perhaps we can defined a SocketOption (or something similar) enum that can be used with a isOptionSupported function.

Ben Millwood

I don't think a function isOptionSupported is necessarily mandatory. I would expect most users would merrily write their code as if it was, and then catch an appropriate exception if it turned out that it wasn't.

Of course, some people are of the opinion that exceptions are the devil, but if we are taking that line there's a lot else we should change too :)

An exceptionless type could be setSocketDebug :: IO (Maybe (Socket -> Bool -> IO ())). Practically speaking, we don't actually need the outer IO, but it will appease purists and probably isn't a big inconvenience in practice (since you can only use the function from IO anyways). Or, of course, something as simple as Socket -> Bool -> IO Bool, or Socket -> Bool -> IO (Maybe IOError), but those do make it easier to ignore the error.

Johan Tibell
Owner

I don't think a function isOptionSupported is necessarily mandatory. I would expect most users would merrily write their code as if it was, and then catch an appropriate exception if it turned out that it wasn't.

It's still nice to be able to enquire about the capabilities of the underlying platform.

Ben Millwood

Right, an IO Maybe type would permit us to do that (or, if you're happy with platform-to-platform inconsistency, just a Maybe type would also work).

Another alternative would be a third value per option, isSocketDebugSupported :: Bool. This prevents users from abstracting over socket options to some extent, but I don't think that's something which is natural to do anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.