Re: [PATCH] Lua Leak in OS Library

[Peter Cawley <lua@...>]

In my opinion, os.exit() is for exiting quickly and letting the OS
worry about cleaning up. I was under the impression that the OS will
free any resources still held by a terminating application, thus
meaning that nothing is really leaked. If a Lua script wants to exit
cleanly, it should return from its main chunk. If it wants to exit
quickly, it should call os.exit().

My 2c,
Peter

On Sun, Sep 6, 2009 at 10:30 PM, Matthew Paul Del
Buono<delbu9c1@erau.edu> wrote:
> Hi list,
>
> During discussion on IRC today we clarified a (relatively
> benign) memory leak I've been noticing for a while, bug
> ignoring. While the leak is simply due to a direct call to the
> exit() function during os.exit(), I feel that the leak should
> still be handled because it is unclean and is trivial to
> correct.
>
> Is it necessary? It's debatable. The leak is not one that
> would be impacted by prolonged execution. It only shows itself
> because the Lua state is not closed when os.exit() is called.
>
> To reproduce this issue:
>
> $ valgrind lua -e 't = {} for x=1,1000 do t[x] = {} end
> os.exit()'
> ==27928== LEAK SUMMARY:
> ==27928== still reachable: 61,796 bytes in 1,397 blocks.
>
> We can see that this is due to Lua's state and not something
> else like the command line being longer because adding a
> single character produces a dramatically changed result:
>
> $ valgrind lua -e 't = {} for x=1,10000 do t[x] = {} end
> os.exit()'
> ==27937== LEAK SUMMARY:
> ==27937== still reachable: 534,116 bytes in 10,397 blocks.
>
> As discussed earlier, the issue is due to os.exit()
> immediately terminating the program without cleaning up. An
> obvious solution is to allow it to clean up. I developed the
> following patch for this situation:
>
>
> $ diff -u ./lua-5.1.4/src/loslib.c ./lua-5.1.4-
> corrected/src/loslib.c
> --- ./lua-5.1.4/src/loslib.c    2008-01-18 11:38:18.000000000
> -0500
> +++ ./lua-5.1.4-corrected/src/loslib.c  2009-09-06
> 16:55:16.000000000 -0400
> @@ -214,7 +214,9 @@
>
>
>  static int os_exit (lua_State *L) {
> -  exit(luaL_optint(L, 1, EXIT_SUCCESS));
> +  int code = luaL_optint(L, 1, EXIT_SUCCESS);
> +  lua_close(L);
> +  exit(code);
>  }
>
>  static const luaL_Reg syslib[] = {
>
>
> We see that the issue is corrected:
>
> $ valgrind ./lua -e 't = {} for x=1,10000 do t[x] = {} end
> os.exit()'
> ==29175== All heap blocks were freed -- no leaks are possible.
>
> However, there is valid concern regarding this patch. Is it
> safe for Lua to close its own state during a call? Naturally,
> if this function were to exit, the system would become
> unstable as it would begin executing in a closed state, but
> ANSI C defines exit() as never returning, so this should be a
> non-issue.
>
> An alternative approach is to assume that it is not Lua's job
> to do this, but rather the enclosing application's. As such,
> this means it's not Lua's fault for failing to clean up, but
> rather the interpreter's. An alternative solution would be to
> have the interpreter write its own os.exit() to close the
> state and then exit. While this puts the control in the
> application developers' hands, I feel it's a less suitable
> solution because it would mean duplicated code for everyone
> that wishes to handle this situation.
>
> I would like to think that, while this is a benign issue, it's
> one that should be corrected. Does anyone see any problems
> with the patch above?
>
> Regards,
> -- Matthew P. Del Buono
>