Skip to content

Timeout implementation #120

Closed
wants to merge 4 commits into from

2 participants

@denisenkom

Proposed implementation of get/set timeout on sockets. Issue #8

@tibbe tibbe commented on an outdated diff Jan 10, 2014
Network/Socket.hsc
+ _withTimeVal timeout $ \p_timeval sz -> do
+ throwSocketErrorIfMinus1_ "setSocketSendTimeOut" $
+ c_setsockopt s level opt p_timeval $ fromIntegral sz
+ return ()
+
+
+getSocketSendTimeOut :: Socket -> IO Float
+getSocketSendTimeOut (MkSocket s _ _ _ _) = do
+ (level, opt) <- packSocketOption' "getSocketSendTimeOut" SendTimeOut
+ _withNewTimeVal $ \p_timeval sz ->
+ with ((fromIntegral sz) :: CInt) $ \ptr_sz -> do
+ throwSocketErrorIfMinus1Retry "getSocketSendTimeOut" $
+ c_getsockopt s level opt p_timeval ptr_sz
+ _decodeTimeVal p_timeval
+
+
-- | Get a socket option that gives an Int value.
-- There is currently no API to get e.g. the timeval socket options
@tibbe
Haskell member
tibbe added a note Jan 10, 2014

This comment needs to be updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@tibbe tibbe and 1 other commented on an outdated diff Jan 10, 2014
Network/Socket.hsc
@@ -967,6 +971,69 @@ setSocketOption (MkSocket s _ _ _ _) so v = do
return ()
+_withTimeVal :: Float -> (Ptr a -> Int -> IO b) -> IO b
@tibbe
Haskell member
tibbe added a note Jan 10, 2014

Please don't prefix module-local functions with _.

Please do document every top-level function. It would for example be good to document the unit of the first argument.

@denisenkom
denisenkom added a note Jan 11, 2014

done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@tibbe tibbe and 1 other commented on an outdated diff Jan 10, 2014
Network/Socket.hsc
@@ -967,6 +971,69 @@ setSocketOption (MkSocket s _ _ _ _) so v = do
return ()
+_withTimeVal :: Float -> (Ptr a -> Int -> IO b) -> IO b
+_withTimeVal timeout f = do
+ let sz = (fromIntegral (#const sizeof(struct timeval)))
@tibbe
Haskell member
tibbe added a note Jan 10, 2014

I'd prefer it if you left out the outermost parenthesis. They're not needed.

@denisenkom
denisenkom added a note Jan 11, 2014

done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@tibbe tibbe and 1 other commented on an outdated diff Jan 10, 2014
Network/Socket.hsc
+ sec = (truncate timeout) :: CLong
+ usec = (round ((timeout - (fromIntegral sec)) * 1000000)) :: CLong
+ allocaBytes sz $ \p_timeval -> do
+ (#poke struct timeval, tv_sec) p_timeval sec
+ (#poke struct timeval, tv_usec) p_timeval usec
+ f p_timeval sz
+
+
+_withNewTimeVal :: (Ptr a -> Int -> IO b) -> IO b
+_withNewTimeVal f = do
+ let sz = (fromIntegral (#const sizeof(struct timeval)))
+ allocaBytes sz $ \p_timeval -> do
+ f p_timeval sz
+
+
+_decodeTimeVal :: Ptr a -> IO Float
@tibbe
Haskell member
tibbe added a note Jan 10, 2014

Please call this peekTimeVal to be consistent with the naming of such functions.

@denisenkom
denisenkom added a note Jan 11, 2014

done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@tibbe tibbe and 1 other commented on an outdated diff Jan 10, 2014
Network/Socket.hsc
+
+_withNewTimeVal :: (Ptr a -> Int -> IO b) -> IO b
+_withNewTimeVal f = do
+ let sz = (fromIntegral (#const sizeof(struct timeval)))
+ allocaBytes sz $ \p_timeval -> do
+ f p_timeval sz
+
+
+_decodeTimeVal :: Ptr a -> IO Float
+_decodeTimeVal p_timeval = do
+ sec <- (#peek struct timeval, tv_sec) p_timeval :: IO CLong
+ usec <- (#peek struct timeval, tv_usec) p_timeval :: IO CLong
+ return $ (fromIntegral sec) + (fromIntegral usec) / 1000000.0
+
+
+setSocketRecvTimeOut :: Socket -> Float -> IO ()
@tibbe
Haskell member
tibbe added a note Jan 10, 2014

Double seems like a better option. It would allow us higher precision.

@denisenkom
denisenkom added a note Jan 11, 2014

My rationale for Float was that it is a timeout which isn't precise by definition. This isn't a strong opinion though, I will change it to Double.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@tibbe
Haskell member
tibbe commented Jan 10, 2014

First, thanks for taking the time to look at this. I've added some comments to the commit.

Which platforms do you had time to test on. This needs to work on Linux, OS X, and Windows at least.

@denisenkom

So far tested only on linux, will also test on windows. Can't test on OS X (don't have a mac), can you test on Mac? Or suggest some way to test on it?

@tibbe
Haskell member
tibbe commented Jan 11, 2014

Hi,

Thanks for addressing the comments. If you change to Double I think we're good to go. I will test on OS X if you test Windows & Linux.

Thanks!

@denisenkom

Turned out it actually doesn't work on Linux, working on it, don't merge.

@denisenkom

The problem seems to be in "recv" function, it retries c_recv if it returns EWOULDBLOCK (which is returned when timeout is reached), effectively defeating timeout.
What do you think would be a correct approach here? Remove retries from recv function (and others), which may cause backward compatibility problems?

@tibbe
Haskell member
tibbe commented Jan 13, 2014

I should have suspected as much. The whole retry/would-block part is because we use the GHC I/O manager to manage sockets and we run them all in non-blocking mode. I suspect we cannot get timeouts to work this way and we need to use the generic timeout function instead.

@denisenkom

Yeah, will try "timeout" function. Please decline this pull request. Thanks.

@tibbe tibbe closed this Jan 14, 2014
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.