elm-phoenix-socket icon indicating copy to clipboard operation
elm-phoenix-socket copied to clipboard

Sent messages cumulate in memory

Open lguminski opened this issue 8 years ago • 3 comments

Thank you for this library. It is a very useful piece of code.

It works perfectly, nevertheless I spotted something strange in version 2.0.1. It looks like the Phoenix.Socket model leaks memory. When heartbeat is enabled (introduced through #7 ), each heartbeat messages adds up to socket.pushes. After some time the structure looks like this:

pushes = Dict.fromList [
    (2,{ event = \"heartbeat\", channel = \"phoenix\", payload = {}, onOk = Nothing, onError = Nothing }),
    (3,{ event = \"heartbeat\", channel = \"phoenix\", payload = {}, onOk = Nothing, onError = Nothing }),
    (4,{ event = \"heartbeat\", channel = \"phoenix\", payload = {}, onOk = Nothing, onError = Nothing }),
    (5,{ event = \"heartbeat\", channel = \"phoenix\", payload = {}, onOk = Nothing, onError = Nothing }),
    (6,{ event = \"heartbeat\", channel = \"phoenix\", payload = {}, onOk = Nothing, onError = Nothing }),
    (7,{ event = \"heartbeat\", channel = \"phoenix\", payload = {}, onOk = Nothing, onError = Nothing }),
    (8,{ event = \"heartbeat\", channel = \"phoenix\", payload = {}, onOk = Nothing, onError = Nothing })
]

After digging deeper I noticed that server responds properly to each heartbeat with replies like these:

Phoenix message: { event = "phx_reply", topic = "phoenix", payload = { status = "ok", response = {} }, ref = Just 2 }
main.js:907
Phoenix message: { event = "phx_reply", topic = "phoenix", payload = { status = "ok", response = {} }, ref = Just 3 }
main.js:907
Phoenix message: { event = "phx_reply", topic = "phoenix", payload = { status = "ok", response = {} }, ref = Just 4 }
main.js:907
Phoenix message: { event = "phx_reply", topic = "phoenix", payload = { status = "ok", response = {} }, ref = Just 5 }

(note that each reply has correct ref set)

I would expect that the library after seeing phx_reply with a proper reference ref would remove original heartbeat messages from socket.pushes. But it does not happen.

I started thinking that I do not use the library properly. So I checked its code to find out where are the places which remove something from socket.pushes. And the only places I found in Socket.elm (v2.0.1) are

 84         ChannelClosed channelName ->
 91                         pushes =
 92                             Dict.remove channel.joinRef socket.pushes

102         ChannelJoined channelName ->
109                         pushes =
110                             Dict.remove channel.joinRef socket.pushes

This means that I cannot find a code, which would remove references from socket.pushes upon receiving phx_reply from the server.

I am still not 100% sure if this is a bug. It could be that I am not using the library properly. If so, please advise me how to eliminate the leak.

PS. I would assume that all sent messages cumulate in memory like this. Not only heartbeats.

lguminski avatar Jan 28 '17 23:01 lguminski

Yeah it looks like pushes are cumulating in memory. Is it expected behavior?

kuon avatar May 03 '17 03:05 kuon

The sensible thing would be to remove the messages from the pushes list once the reply has been received. But what about heartbeats that got no reply? Should they remain in the list forever?

cgudrian avatar Nov 05 '17 19:11 cgudrian

In point of fact, it's possible for any message that gets pushed to receive a reply. On the Elixir side, in the handle_in callback for a message can return a tuple of the form:

{:reply, {<status>, <reply value>}}, socket}

And doing so causes a phx_reply message to be sent on the channel. At the moment, any message that receives such reply (other than the channel join or channel leave messages whose refs are specifically tracked) is ignored. The reply is not sent on to the channel's "on" handler.

easco avatar Mar 04 '18 22:03 easco