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

memory leak in session receive on response? #19

Open
wmoors opened this issue Dec 11, 2019 · 1 comment
Open

memory leak in session receive on response? #19

wmoors opened this issue Dec 11, 2019 · 1 comment

Comments

@wmoors
Copy link

wmoors commented Dec 11, 2019

If I use a closure callback function when creating a request (to keep a reference alive as an upvalue while waiting for the response), I noticed that my program started to leak memory albeit at a very slow pace.

I started to look into this and traced it back to the static int lmpack_session_receive(lua_State *L) function in lmpack.c. It seems that the cookie passed along when creating the request is not being unref'd, so the garbage collector failed to remove the callback function, which in turn kept the upvalue reference alive and so on...

When applying this patch, the leak seems to be fixed for me:

diff --git a/lmpack.c b/lmpack.c
index 4a7e4d1..5ec1a52 100644
--- a/lmpack.c
+++ b/lmpack.c
@@ -904,6 +904,8 @@ static int lmpack_session_receive(lua_State *L)
     case MPACK_RPC_RESPONSE:
       lua_pushstring(L, "response");
       lmpack_geti(L, session->reg, (int)session->unpacked.msg.data.i);
+      lmpack_unref(L, session->reg, (int)session->unpacked.msg.data.i);
+      session->unpacked.msg.data.i = LUA_NOREF;
       break;
     case MPACK_RPC_NOTIFICATION:
       lua_pushstring(L, "notification");

I am not completely convinced this is the right way to fix it, so I didn't submit a patch and post an issue instead.

@Bartel-C8
Copy link

May be closed now, thanks!

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

2 participants