Re: Lua memory allocator
> On 13 Apr 2017, at 17:12, Willy Tarreau wrote: > > On Thu, Apr 13, 2017 at 05:02:54PM +0200, Willy Tarreau wrote: >> On Thu, Apr 13, 2017 at 12:37:19PM +0200, Thierry Fournier wrote: >>> Good catch. I read the code of the Lua function luaL_newstate, and I >>> approve your change because this function dos exactly the same job, but >>> with a libc memory allocator. Note that a few lines after after your patch >>> (about 15 lines), I call the function "lua_setallocf()" which set again >>> the memory allocator. I think that this line becomes useless and it will >>> be better to remove it. >> >> Yes it's after I noticed it that I started to understand the problem. But >> it's unsure to me whether we can get rid of it, as I understood that the >> first one stated what allocator to use during that call. You may be right, >> it probably doesn't make much sense. I can test however, it's easy. > > OK I could confirm that you're right so I removed this call and merged > the change now. Ok, thanks for tests. > thanks! > Willy >
Re: Lua memory allocator
On Thu, Apr 13, 2017 at 05:02:54PM +0200, Willy Tarreau wrote: > On Thu, Apr 13, 2017 at 12:37:19PM +0200, Thierry Fournier wrote: > > Good catch. I read the code of the Lua function luaL_newstate, and I > > approve your change because this function dos exactly the same job, but > > with a libc memory allocator. Note that a few lines after after your patch > > (about 15 lines), I call the function "lua_setallocf()" which set again > > the memory allocator. I think that this line becomes useless and it will > > be better to remove it. > > Yes it's after I noticed it that I started to understand the problem. But > it's unsure to me whether we can get rid of it, as I understood that the > first one stated what allocator to use during that call. You may be right, > it probably doesn't make much sense. I can test however, it's easy. OK I could confirm that you're right so I removed this call and merged the change now. thanks! Willy
Re: Lua memory allocator
On Thu, Apr 13, 2017 at 12:37:19PM +0200, Thierry Fournier wrote: > Good catch. I read the code of the Lua function luaL_newstate, and I > approve your change because this function dos exactly the same job, but > with a libc memory allocator. Note that a few lines after after your patch > (about 15 lines), I call the function "lua_setallocf()" which set again > the memory allocator. I think that this line becomes useless and it will > be better to remove it. Yes it's after I noticed it that I started to understand the problem. But it's unsure to me whether we can get rid of it, as I understood that the first one stated what allocator to use during that call. You may be right, it probably doesn't make much sense. I can test however, it's easy. Thanks! Willy
Re: Lua memory allocator
> On 12 Apr 2017, at 23:30, Willy Tarreau wrote: > > Thierry, > > while instrumenting my malloc/free functions to debug a problem, I was > hit by a malloc/realloc inconsistency in the Lua allocator. The problem > is that luaL_newstate() uses malloc() to create its first objects and > only after this one we change the allocator to use ours. Thus on the > first realloc() call it fails because it tries to reallocate using one > function something allocated differently earlier. > > I found some info regarding the fact that instead it was possible to > use lua_newstate() and pass it the allocator to use. So that's what I > did and it solve the problem for me. > > I think that it results in more accurately accounting the memory in use > by the Lua stack since even the very first malloc is counted now. But > could you take a quick look to ensure I didn't overlook anything ? I'm > attaching the patch, if you're fine with it I can merge it. Hi, Good catch. I read the code of the Lua function luaL_newstate, and I approve your change because this function dos exactly the same job, but with a libc memory allocator. Note that a few lines after after your patch (about 15 lines), I call the function “lua_setallocf()” which set again the memory allocator. I think that this line becomes useless and it will be better to remove it. Thierry > Thanks, > Willy > <0001-MINOR-lua-ensure-the-memory-allocator-is-used-all-th.patch>
Lua memory allocator
Thierry, while instrumenting my malloc/free functions to debug a problem, I was hit by a malloc/realloc inconsistency in the Lua allocator. The problem is that luaL_newstate() uses malloc() to create its first objects and only after this one we change the allocator to use ours. Thus on the first realloc() call it fails because it tries to reallocate using one function something allocated differently earlier. I found some info regarding the fact that instead it was possible to use lua_newstate() and pass it the allocator to use. So that's what I did and it solve the problem for me. I think that it results in more accurately accounting the memory in use by the Lua stack since even the very first malloc is counted now. But could you take a quick look to ensure I didn't overlook anything ? I'm attaching the patch, if you're fine with it I can merge it. Thanks, Willy >From a1ee4ff02e0755d9c8237615fe5ca7124c70c1ad Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Wed, 12 Apr 2017 21:40:29 +0200 Subject: MINOR: lua: ensure the memory allocator is used all the time luaL_setstate() uses malloc() to initialize the first objects, and only after this we replace the allocator. This creates trouble when replacing the standard memory allocators during debugging sessions since the new allocator is used to realloc() an area previously allocated using the default malloc(). Lua provides lua_newstate() in addition to luaL_newstate(), which takes an allocator for the initial malloc. This is exactly what we need, and this patch does this and fixes the problem. This has no impact outside of debugging sessions and there's no need to backport this. --- src/hlua.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hlua.c b/src/hlua.c index 5383fe9..9b59194 100644 --- a/src/hlua.c +++ b/src/hlua.c @@ -7182,7 +7182,7 @@ void hlua_init(void) gL.Mref = LUA_REFNIL; gL.flags = 0; LIST_INIT(&gL.com); - gL.T = luaL_newstate(); + gL.T = lua_newstate(hlua_alloc, &hlua_global_allocator); hlua_sethlua(&gL); gL.Tref = LUA_REFNIL; gL.task = NULL; -- 1.7.12.1
