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

Sent messages cumulate in memory #23

Open
lguminski opened this issue Jan 28, 2017 · 3 comments
Open

Sent messages cumulate in memory #23

lguminski opened this issue Jan 28, 2017 · 3 comments

Comments

@lguminski
Copy link

lguminski commented Jan 28, 2017

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.

@kuon
Copy link

kuon commented May 3, 2017

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

@cgudrian
Copy link

cgudrian commented Nov 5, 2017

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?

@easco
Copy link

easco commented Mar 4, 2018

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.

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

4 participants