Re: [Haskell-cafe] Space leak with recursion

pollSockets :: State -> IO () pollSockets state = void $ poll (-1) [ Sock (listenSocket state) [In] (Just $ observerHandleEvts state) , Sock (snapSocket state) [In] (Just $ snapshotHandleEvts state) ]
observerHandleEvts :: State -> [Event] -> IO () observerHandleEvts state _ = do void $ receiveMulti $ listenSocket state pollSockets state
snapshotHandleEvts :: State -> [Event] -> IO () snapshotHandleEvts state _ = do void $ receiveMulti $ snapSocket state pollSockets state
What happens here if there is an event waiting on both the listen socket *and* the snap socket? It looks like `observerHandleEvts` will be called and, since it recursively calles `pollSockets`, the `snapshotHandleEvts` handler will not be run, although its continuation will be kept around leaking space.
This could be an issue, but during my testing there were no messages sent to the snapshot socket (I haven't implemented the snapshot socket in the client yet).
It seems unwise to make a recursive call to the event loop inside a handler.
How would I update my state and ensure that the next invocation of a handler gets the updated state? With the forever function my state updates are not propagated. Kind regards, Martijn Rijkeboer

On Fri, Apr 24, 2015 at 10:34:04AM +0200, Martijn Rijkeboer wrote:
pollSockets :: State -> IO () pollSockets state = void $ poll (-1) [ Sock (listenSocket state) [In] (Just $ observerHandleEvts state) , Sock (snapSocket state) [In] (Just $ snapshotHandleEvts state) ]
observerHandleEvts :: State -> [Event] -> IO () observerHandleEvts state _ = do void $ receiveMulti $ listenSocket state pollSockets state
snapshotHandleEvts :: State -> [Event] -> IO () snapshotHandleEvts state _ = do void $ receiveMulti $ snapSocket state pollSockets state
What happens here if there is an event waiting on both the listen socket *and* the snap socket? It looks like `observerHandleEvts` will be called and, since it recursively calles `pollSockets`, the `snapshotHandleEvts` handler will not be run, although its continuation will be kept around leaking space.
This could be an issue, but during my testing there were no messages sent to the snapshot socket (I haven't implemented the snapshot socket in the client yet).
Then I suggest the first thing to try is to run a test without snapshot socket functionality at all, and see if you still get a space leak.
It seems unwise to make a recursive call to the event loop inside a handler.
How would I update my state and ensure that the next invocation of a handler gets the updated state? With the forever function my state updates are not propagated.
I don't see where you are updating any state. Tom

pollSockets :: State -> IO () pollSockets state = void $ poll (-1) [ Sock (listenSocket state) [In] (Just $ observerHandleEvts state) , Sock (snapSocket state) [In] (Just $ snapshotHandleEvts state) ]
observerHandleEvts :: State -> [Event] -> IO () observerHandleEvts state _ = do void $ receiveMulti $ listenSocket state pollSockets state
snapshotHandleEvts :: State -> [Event] -> IO () snapshotHandleEvts state _ = do void $ receiveMulti $ snapSocket state pollSockets state
What happens here if there is an event waiting on both the listen socket *and* the snap socket? It looks like `observerHandleEvts` will be called and, since it recursively calles `pollSockets`, the `snapshotHandleEvts` handler will not be run, although its continuation will be kept around leaking space.
This could be an issue, but during my testing there were no messages sent to the snapshot socket (I haven't implemented the snapshot socket in the client yet).
Then I suggest the first thing to try is to run a test without snapshot socket functionality at all, and see if you still get a space leak.
Will do.
It seems unwise to make a recursive call to the event loop inside a handler.
How would I update my state and ensure that the next invocation of a handler gets the updated state? With the forever function my state updates are not propagated.
I don't see where you are updating any state.
The state contains a sequence number that needs to be incremented, but I left that out for brevity... Kind regards, Martijn Rijkeboer

On Fri, Apr 24, 2015 at 10:41:30AM +0200, Martijn Rijkeboer wrote:
I don't see where you are updating any state.
The state contains a sequence number that needs to be incremented, but I left that out for brevity...
It's important to see that, because that kind of thing is exactly where space leaks can hide. Tom

On Fri, Apr 24, 2015 at 10:41:30AM +0200, Martijn Rijkeboer wrote:
I don't see where you are updating any state.
The state contains a sequence number that needs to be incremented, but I left that out for brevity...
It's important to see that, because that kind of thing is exactly where space leaks can hide.
Sorry for the confusion. The version that has the space leak is the version that I included with my initial mail and doesn't increment the sequence number. Once thee space leak is fixed I will need to add code to increment the sequence number (not yet implemented). I tried to make a minimal version that reproduces the problem, but I need to be able to update the state in the "real" version. Kind regards, Martijn Rijkeboer

On Fri, Apr 24, 2015 at 10:57:41AM +0200, Martijn Rijkeboer wrote:
On Fri, Apr 24, 2015 at 10:41:30AM +0200, Martijn Rijkeboer wrote:
I don't see where you are updating any state.
The state contains a sequence number that needs to be incremented, but I left that out for brevity...
It's important to see that, because that kind of thing is exactly where space leaks can hide.
Sorry for the confusion. The version that has the space leak is the version that I included with my initial mail and doesn't increment the sequence number.
I see.
Once thee space leak is fixed I will need to add code to increment the sequence number (not yet implemented).
In that case the most important thing to do is to try to reproduce the space leak without the snapshot socket.
I tried to make a minimal version that reproduces the problem, but I need to be able to update the state in the "real" version.
Very sensible. I was confused for a moment whether the minimal version did actually exhibit the space leak behaviour. To get the state-updating behaviour you want, I suggest you use `StateT IO` rather than `IO`. With handlers in the `StateT IO` monad the state updates will occur as you expect. Tom

On Fri, Apr 24, 2015 at 10:04:06AM +0100, Tom Ellis wrote:
In that case the most important thing to do is to try to reproduce the space leak without the snapshot socket.
In fact I think we can already deduce the answer from the implementation of `poll`: http://hackage.haskell.org/package/zeromq4-haskell-0.6.3/docs/src/System-ZMQ... The key point is that the [Poll s m] list is `mapM`ed over. Each handler is checked for a match and then fired or not fired before the next handler is checked for a match. This means that if you call `pollSockets` in a handler you are leaking the space associated with the unprocessed entries. Tom

In fact I think we can already deduce the answer from the implementation of `poll`:
http://hackage.haskell.org/package/zeromq4-haskell-0.6.3/docs/src/System-ZMQ...
The key point is that the [Poll s m] list is `mapM`ed over. Each handler is checked for a match and then fired or not fired before the next handler is checked for a match. This means that if you call `pollSockets` in a handler you are leaking the space associated with the unprocessed entries.
I should have checked the code. Unfortunately the idea to use a recursive call to pollSockets comes from the Haskell examples in the ZeroMQ guide. So I will probably not be the only one that has this issue... Kind regards, Martijn Rijkeboer

On Fri, Apr 24, 2015 at 11:39:23AM +0200, Martijn Rijkeboer wrote:
In fact I think we can already deduce the answer from the implementation of `poll`:
http://hackage.haskell.org/package/zeromq4-haskell-0.6.3/docs/src/System-ZMQ...
The key point is that the [Poll s m] list is `mapM`ed over. Each handler is checked for a match and then fired or not fired before the next handler is checked for a match. This means that if you call `pollSockets` in a handler you are leaking the space associated with the unprocessed entries.
I should have checked the code. Unfortunately the idea to use a recursive call to pollSockets comes from the Haskell examples in the ZeroMQ guide. So I will probably not be the only one that has this issue...
Strange. Can you link me to that?

I should have checked the code. Unfortunately the idea to use a recursive call to pollSockets comes from the Haskell examples in the ZeroMQ guide. So I will probably not be the only one that has this issue...
Strange. Can you link me to that?
An example that uses the pollSockets idea (pollServer): - https://github.com/imatix/zguide/blob/master/examples/Haskell/lpclient.hs All Haskell examples from the ZeroMQ guide: - https://github.com/imatix/zguide/tree/master/examples/Haskell Kind regards, Martijn Rijkeboer

On Fri, Apr 24, 2015 at 11:43:44AM +0200, Martijn Rijkeboer wrote:
I should have checked the code. Unfortunately the idea to use a recursive call to pollSockets comes from the Haskell examples in the ZeroMQ guide. So I will probably not be the only one that has this issue...
Strange. Can you link me to that?
An example that uses the pollSockets idea (pollServer): - https://github.com/imatix/zguide/blob/master/examples/Haskell/lpclient.hs
That's quite different. `sendServer` is not being called by `poll`.

On Fri, Apr 24, 2015 at 11:43:44AM +0200, Martijn Rijkeboer wrote:
I should have checked the code. Unfortunately the idea to use a recursive call to pollSockets comes from the Haskell examples in the ZeroMQ guide. So I will probably not be the only one that has this issue...
Strange. Can you link me to that?
An example that uses the pollSockets idea (pollServer): - https://github.com/imatix/zguide/blob/master/examples/Haskell/lpclient.hs
That's quite different. `sendServer` is not being called by `poll`.
Maybe I don't understand, but inside the pollServer function a call to poll is done and on line 47 and 56 of that same function, pollServer is called again (recursive). Kind regards, Martijn Rijkeboer

On Fri, Apr 24, 2015 at 11:53:50AM +0200, Martijn Rijkeboer wrote:
On Fri, Apr 24, 2015 at 11:43:44AM +0200, Martijn Rijkeboer wrote:
I should have checked the code. Unfortunately the idea to use a recursive call to pollSockets comes from the Haskell examples in the ZeroMQ guide. So I will probably not be the only one that has this issue...
Strange. Can you link me to that?
An example that uses the pollSockets idea (pollServer): - https://github.com/imatix/zguide/blob/master/examples/Haskell/lpclient.hs
That's quite different. `sendServer` is not being called by `poll`.
Maybe I don't understand, but inside the pollServer function a call to poll is done and on line 47 and 56 of that same function, pollServer is called again (recursive).
It's fine to call `pollServer` recursively, but it's not fine to call it recursively from a handler, i.e. something that occurs in the second argument to `poll`. Tom

Maybe I don't understand, but inside the pollServer function a call to poll is done and on line 47 and 56 of that same function, pollServer is called again (recursive).
It's fine to call `pollServer` recursively, but it's not fine to call it recursively from a handler, i.e. something that occurs in the second argument to `poll`.
Clear, my bad. I've just implemented a version with StateT (code below) and it doesn't leak space as you already expected. Thank you very much for your help. Kind regards, Martijn Rijkeboer --- code --- module Observable ( run ) where import Control.Monad (forever, void) import Control.Monad.State (StateT, get, liftIO, put, runStateT) import Data.Int (Int64) import System.ZMQ4 data State = State { nextSeqNum :: !Int64 , listenSocket :: !(Socket Pull) } run :: IO () run = do withContext $ \ctx -> withSocket ctx Pull $ \observer -> do setLinger (restrict (0::Int)) observer bind observer "tcp://*:7010" let state = State { nextSeqNum = 0 , listenSocket = observer } void $ runStateT pollSockets state return () pollSockets :: StateT State IO () pollSockets = do state <- get forever $ void $ poll (-1) [Sock (listenSocket state) [In] (Just observerHandleEvts)] observerHandleEvts :: [Event] -> StateT State IO () observerHandleEvts _ = do state <- get liftIO $ void $ receiveMulti $ listenSocket state liftIO $ printSeqNum state put $ incrSeqNum state printSeqNum :: State -> IO () printSeqNum state = putStrLn $ show $ nextSeqNum state incrSeqNum :: State -> State incrSeqNum state = state{nextSeqNum = currSeqNum + 1} where currSeqNum = nextSeqNum state

On Fri, Apr 24, 2015 at 12:16:55PM +0200, Martijn Rijkeboer wrote:
Maybe I don't understand, but inside the pollServer function a call to poll is done and on line 47 and 56 of that same function, pollServer is called again (recursive).
It's fine to call `pollServer` recursively, but it's not fine to call it recursively from a handler, i.e. something that occurs in the second argument to `poll`.
Clear, my bad. I've just implemented a version with StateT (code below) and it doesn't leak space as you already expected. Thank you very much for your help.
Looks good! You're welcome Martijn. Tom

Once thee space leak is fixed I will need to add code to increment the sequence number (not yet implemented).
In that case the most important thing to do is to try to reproduce the space leak without the snapshot socket.
I've just ran the code below (removed the snapshot socket) and the space leak is still there. Since I don't have access to a Windows box at the moment this was tested on the following configuration: - OS: Ubuntu 14.04 (64-bit) - GHC: 7.8.4 (64-bit) - Zeromq4-haskell: 0.6.3 (Stackage LTS 2.4) - ZeroMQ: 4.0.4 (64-bit)
Very sensible. I was confused for a moment whether the minimal version did actually exhibit the space leak behaviour.
To get the state-updating behaviour you want, I suggest you use `StateT IO` rather than `IO`. With handlers in the `StateT IO` monad the state updates will occur as you expect.
Thanks for the suggestion I'll try that, but it will take me some time. Kind regards, Martijn Rijkeboer --- code --- module Observable ( run ) where import Control.Monad (void) import Data.Int (Int64) import System.ZMQ4 data State = State { nextSeqNum :: !Int64 , listenSocket :: !(Socket Pull) } run :: IO () run = do withContext $ \ctx -> withSocket ctx Pull $ \observer -> do setLinger (restrict (0::Int)) observer bind observer "tcp://*:7010" let state = State { nextSeqNum = 0 , listenSocket = observer } pollSockets state pollSockets :: State -> IO () pollSockets state = void $ poll (-1) [ Sock (listenSocket state) [In] (Just $ observerHandleEvts state) ] observerHandleEvts :: State -> [Event] -> IO () observerHandleEvts state _ = do void $ receiveMulti $ listenSocket state -- TODO: update state by incrementing the nextSeqNum pollSockets state

On Fri, Apr 24, 2015 at 11:34:25AM +0200, Martijn Rijkeboer wrote:
Once thee space leak is fixed I will need to add code to increment the sequence number (not yet implemented).
In that case the most important thing to do is to try to reproduce the space leak without the snapshot socket.
I've just ran the code below (removed the snapshot socket) and the space leak is still there. Since I don't have access to a Windows box at the moment this was tested on the following configuration: - OS: Ubuntu 14.04 (64-bit) - GHC: 7.8.4 (64-bit) - Zeromq4-haskell: 0.6.3 (Stackage LTS 2.4) - ZeroMQ: 4.0.4 (64-bit)
This behaviour is consistent with my understanding of how `poll` behaves. `poll` has trivial work to do when the handler returns, and recording this work consumes space that won't be freed. Avoid calling `pollSockets` in your handlers and you'll be fine. Tom
participants (2)
-
Martijn Rijkeboer
-
Tom Ellis