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

Fix leak from fs_scandir whenever it wasn't fully iterated via fs_scandir_next #601

Merged
merged 1 commit into from
May 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions src/fs.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,22 @@ typedef struct {
#endif

static uv_fs_t* luv_check_fs(lua_State* L, int index) {
if (luaL_testudata(L, index, "uv_fs") != NULL) {
return (uv_fs_t*)lua_touserdata(L, index);
}
uv_fs_t* req = (uv_fs_t*)luaL_checkudata(L, index, "uv_req");
luaL_argcheck(L, req->type == UV_FS && req->data, index, "Expected uv_fs_t");
return req;
}

static int luv_fs_gc(lua_State* L) {
uv_fs_t* req = luv_check_fs(L, 1);
luv_cleanup_req(L, (luv_req_t*)req->data);
req->data = NULL;
uv_fs_req_cleanup(req);
return 0;
}

static void luv_push_timespec_table(lua_State* L, const uv_timespec_t* t) {
lua_createtable(L, 0, 2);
lua_pushinteger(L, t->tv_sec);
Expand Down Expand Up @@ -589,20 +600,15 @@ static int luv_fs_scandir(lua_State* L) {
int flags = 0; // TODO: find out what these flags are.
int ref = luv_check_continuation(L, 2);
uv_fs_t* req = (uv_fs_t*)lua_newuserdata(L, uv_req_size(UV_FS));
req->data = luv_setup_req(L, ctx, ref);
req->data = luv_setup_req_with_mt(L, ctx, ref, "uv_fs");
FS_CALL(scandir, req, path, flags);
}

static int luv_fs_scandir_next(lua_State* L) {
uv_fs_t* req = luv_check_fs(L, 1);
uv_dirent_t ent;
int ret = uv_fs_scandir_next(req, &ent);
if (ret == UV_EOF) {
luv_cleanup_req(L, (luv_req_t*)req->data);
req->data = NULL;
uv_fs_req_cleanup(req);
return 0;
}
if (ret == UV_EOF) return 0;
if (ret < 0) return luv_error(L, ret);
return luv_push_dirent(L, &ent, 0);
}
Expand Down
8 changes: 6 additions & 2 deletions src/lreq.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ static int luv_check_continuation(lua_State* L, int index) {

// Store a lua callback in a luv_req for the continuation.
// The uv_req_t is assumed to be at the top of the stack
static luv_req_t* luv_setup_req(lua_State* L, luv_ctx_t* ctx, int cb_ref) {
static luv_req_t* luv_setup_req_with_mt(lua_State* L, luv_ctx_t* ctx, int cb_ref, const char* mt_name) {
luv_req_t* data;

luaL_checktype(L, -1, LUA_TUSERDATA);

data = (luv_req_t*)malloc(sizeof(*data));
if (!data) luaL_error(L, "Problem allocating luv request");

luaL_getmetatable(L, "uv_req");
luaL_getmetatable(L, mt_name);
lua_setmetatable(L, -2);

lua_pushvalue(L, -1);
Expand All @@ -47,6 +47,10 @@ static luv_req_t* luv_setup_req(lua_State* L, luv_ctx_t* ctx, int cb_ref) {
return data;
}

static luv_req_t* luv_setup_req(lua_State* L, luv_ctx_t* ctx, int cb_ref) {
return luv_setup_req_with_mt(L, ctx, cb_ref, "uv_req");
}


static void luv_fulfill_req(lua_State* L, luv_req_t* data, int nargs) {
if (data->callback_ref == LUA_NOREF) {
Expand Down
11 changes: 11 additions & 0 deletions src/luv.c
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,17 @@ static void luv_req_init(lua_State* L) {
luaL_newlib(L, luv_req_methods);
lua_setfield(L, -2, "__index");
lua_pop(L, 1);

// Only used for things that need to be garbage collected
// (e.g. the req when using uv_fs_scandir)
luaL_newmetatable(L, "uv_fs");
lua_pushcfunction(L, luv_req_tostring);
lua_setfield(L, -2, "__tostring");
luaL_newlib(L, luv_req_methods);
lua_setfield(L, -2, "__index");
lua_pushcfunction(L, luv_fs_gc);
lua_setfield(L, -2, "__gc");
lua_pop(L, 1);
}

// Call lua function, will pop nargs values from top of vm stack and push some
Expand Down
1 change: 1 addition & 0 deletions src/private.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ static int luv_check_continuation(lua_State* L, int index);
top of the stack.
*/
static luv_req_t* luv_setup_req(lua_State* L, luv_ctx_t* ctx, int ref);
static luv_req_t* luv_setup_req_with_mt(lua_State* L, luv_ctx_t* ctx, int ref, const char* mt_name);
static void luv_fulfill_req(lua_State* L, luv_req_t* data, int nargs);
static void luv_cleanup_req(lua_State* L, luv_req_t* data);

Expand Down
5 changes: 4 additions & 1 deletion src/req.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,16 @@
#include "private.h"

static uv_req_t* luv_check_req(lua_State* L, int index) {
if (luaL_testudata(L, index, "uv_fs") != NULL) {
return (uv_req_t*)lua_touserdata(L, index);
}
uv_req_t* req = (uv_req_t*)luaL_checkudata(L, index, "uv_req");
luaL_argcheck(L, req->data, index, "Expected uv_req_t");
return req;
}

static int luv_req_tostring(lua_State* L) {
uv_req_t* req = (uv_req_t*)luaL_checkudata(L, 1, "uv_req");
uv_req_t* req = luv_check_req(L, 1);
switch (req->type) {
#define XX(uc, lc) case UV_##uc: lua_pushfstring(L, "uv_"#lc"_t: %p", req); break;
UV_REQ_TYPE_MAP(XX)
Expand Down
8 changes: 8 additions & 0 deletions tests/test-fs.lua
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,14 @@ return require('lib/tap')(function (test)
end
end)

-- this test does nothing on its own, but when run with a leak checker,
-- it will check that the memory allocated by Libuv for req is cleaned up
-- even if its not iterated fully (or at all)
test("fs.scandir with no iteration", function(print, p, expect, uv)
local req = uv.fs_scandir('.')
assert(req)
end)

test("fs.realpath", function (print, p, expect, uv)
p(assert(uv.fs_realpath('.')))
assert(uv.fs_realpath('.', expect(function (err, path)
Expand Down