mpv icon indicating copy to clipboard operation
mpv copied to clipboard

player/lua: restore original lua allocator before close

Open kasper93 opened this issue 1 year ago • 2 comments

We wrap the default Lua allocator to allow used memory tracking. This works well, except that we never destroy the default allocator or notify it that we are closing. Since we override it for the current Lua state, it is reasonable that the Lua engine does not expect it to be used. We get and pass through a free call, but in the case of LuaJIT, the internal allocator has additional state and is freed differently. So, in fact, it is a LuaJIT leak because once we replace the allocator with our custom one, they should clean its internal state. I guess the assumption is to override allocator only before any allocation happen. To work around this issue, restore the default allocator, the one that we use, before closing the state. This way, everything is cleared as expected.

Note that the current solution of wrapping the default allocator works only because none of the supported Lua engines actually invalidate the allocator on the lua_setallocf() call. However, they could, so keep in mind that we are currently depending on an implementation detail.

Thanks to @Dudemanguy for help with finding the changes that introduced the leak.

Fixes: a67bda28409dd893617ef47f6e089fd753d7de78 Fixes: #14451

Read this before you submit this pull request: https://github.com/mpv-player/mpv/blob/master/DOCS/contribute.md

Reading this link and following the rules will get your pull request reviewed and merged faster. Nobody wants lazy pull requests.

kasper93 avatar Jun 28 '24 00:06 kasper93

Would it be possible for you to build an artifact of libmpv 32 bits with this commit?

wbtcpip2 avatar Jun 30 '24 01:06 wbtcpip2