10

I'm trying to find out how to fix these memory leaks I'm getting while running this program with Valgrind. The leaks occur with the two allocations in nShell_client_main. But I'm not sure how to properly free them.

I've tried freeing them at nShell_Connect, but it's causing libUV to abort the program. I've tried freeing them at the end of nShell_client_main, but then I get read/write errors when closing the loop. Does anyone know how I'm supposed to close these handles? I've read this, which got me started. But, it seams out-dated because uv_ip4_addr has a different prototype in the latest version.

(nShell_main is the "entry" point)

#include "nPort.h"
#include "nShell-main.h"

void nShell_Close(
    uv_handle_t * term_handle
){
}

void nShell_Connect(uv_connect_t * term_handle, int status){
    uv_close((uv_handle_t *) term_handle, 0);
}

nError * nShell_client_main(nShell * n_shell, uv_loop_t * n_shell_loop){

    int uv_error = 0;

    nError * n_error = 0;

    uv_tcp_t * n_shell_socket = 0;
    uv_connect_t * n_shell_connect = 0;

    struct sockaddr_in dest_addr;

    n_shell_socket = malloc(sizeof(uv_tcp_t));

    if (!n_shell_socket){
        // handle error
    }

    uv_error = uv_tcp_init(n_shell_loop, n_shell_socket);

    if (uv_error){
        // handle error
    }

    uv_error = uv_ip4_addr("127.0.0.1", NPORT, &dest_addr);

    if (uv_error){
        // handle error
    }

    n_shell_connect = malloc(sizeof(uv_connect_t));

    if (!n_shell_connect){
        // handle error
    }

    uv_error = uv_tcp_connect(n_shell_connect, n_shell_socket, (struct sockaddr *) &dest_addr, nShell_Connect);

    if (uv_error){
        // handle error
    }

    uv_error = uv_run(n_shell_loop, UV_RUN_DEFAULT);

    if (uv_error){
        // handle error
    }

    return 0;
}

nError * nShell_loop_main(nShell * n_shell){

    int uv_error = 0;

    nError * n_error = 0;

    uv_loop_t * n_shell_loop = 0;

    n_shell_loop = malloc(sizeof(uv_loop_t));

    if (!n_shell_loop){
        // handle error
    }

    uv_error = uv_loop_init(n_shell_loop);

    if (uv_error){
        // handle error
    }

    n_error = nShell_client_main(n_shell, n_shell_loop);

    if (n_error){
        // handle error
    }

    uv_loop_close(n_shell_loop);
    free(n_shell_loop);

    return 0;
}

The assertion is happening at the end of the switch statement in this excerpt of code (taken from Joyent's libUV page on Github):

void uv_close(uv_handle_t* handle, uv_close_cb close_cb) {
  assert(!(handle->flags & (UV_CLOSING | UV_CLOSED)));

  handle->flags |= UV_CLOSING;
  handle->close_cb = close_cb;

  switch (handle->type) {
  case UV_NAMED_PIPE:
    uv__pipe_close((uv_pipe_t*)handle);
    break;

  case UV_TTY:
    uv__stream_close((uv_stream_t*)handle);
    break;

  case UV_TCP:
    uv__tcp_close((uv_tcp_t*)handle);
    break;

  case UV_UDP:
    uv__udp_close((uv_udp_t*)handle);
    break;

  case UV_PREPARE:
    uv__prepare_close((uv_prepare_t*)handle);
    break;

  case UV_CHECK:
    uv__check_close((uv_check_t*)handle);
    break;

  case UV_IDLE:
    uv__idle_close((uv_idle_t*)handle);
    break;

  case UV_ASYNC:
    uv__async_close((uv_async_t*)handle);
    break;

  case UV_TIMER:
    uv__timer_close((uv_timer_t*)handle);
    break;

  case UV_PROCESS:
    uv__process_close((uv_process_t*)handle);
    break;

  case UV_FS_EVENT:
    uv__fs_event_close((uv_fs_event_t*)handle);
    break;

  case UV_POLL:
    uv__poll_close((uv_poll_t*)handle);
    break;

  case UV_FS_POLL:
    uv__fs_poll_close((uv_fs_poll_t*)handle);
    break;

  case UV_SIGNAL:
    uv__signal_close((uv_signal_t*) handle);
    /* Signal handles may not be closed immediately. The signal code will */
    /* itself close uv__make_close_pending whenever appropriate. */
    return;

  default:
    assert(0); // assertion is happening here
  }

  uv__make_close_pending(handle);
}

I could call uv__tcp_close manually, but it's not in the public headers (and probably not the right solution anyway).

tay10r
  • 4,234
  • 2
  • 24
  • 44
  • Remind to avoid code reviewing your code; the layout of your function parameters is unorthodox and really weird (and therefore difficult to read) — and also not entirely consistent. – Jonathan Leffler Sep 02 '14 at 04:14
  • @JonathanLeffler yeah, I started the entire project writing long functions broken up like that. I kind of regret it now, but haven't gotten a chance to rewrite them all. – tay10r Sep 02 '14 at 04:37

4 Answers4

30

libuv is not done with a handle until it's close callback is called. That is the exact moment when you can free the handle.

I see you call uv_loop_close, but you don't check for the return value. If there are still pending handles, it will return UV_EBUSY, so you should check for that.

If you want to close a loop and close all handles, you need to do the following:

  • Use uv_stop to stop the loop
  • Use uv_walk and call uv_close on all handles which are not closing
  • Run the loop again with uv_run so all close callbacks are called and you can free the memory in the callbacks
  • Call uv_loop_close, it should return 0 now
saghul
  • 1,990
  • 1
  • 13
  • 15
12

I finally figured out how to stop a loop and clean up all handles. I created a bunch of handles and SIGINT signal handle:

uv_signal_t *sigint = new uv_signal_t;
uv_signal_init(uv_default_loop(), sigint);
uv_signal_start(sigint, on_sigint_received, SIGINT);

When SIGINT is received (Ctrl+C in console is pressed) the on_sigint_received callback is called. The on_sigint_received looks like:

void on_sigint_received(uv_signal_t *handle, int signum)
{
    int result = uv_loop_close(handle->loop);
    if (result == UV_EBUSY)
    {
        uv_walk(handle->loop, on_uv_walk, NULL);
    }
}

It triggers a call back function on_uv_walk:

void on_uv_walk(uv_handle_t* handle, void* arg)
{
    uv_close(handle, on_uv_close);
}

It tries to close each opened libuv handle. Note: that I do not call uv_stop before uv_walk, as mentioned saghul. After on_sigint_received function is called libuv loop continuous the execution and on the next iteration calls on_uv_close for each opened handle. If you call the uv_stop function, then the on_uv_close callback will not be called.

void on_uv_close(uv_handle_t* handle)
{
    if (handle != NULL)
    {
        delete handle;
    }
}

After that libuv do not have opened handles and finishes the loop (exits from uv_run):

uv_run(uv_default_loop(), UV_RUN_DEFAULT);
int result = uv_loop_close(uv_default_loop());
if (result)
{
    cerr << "failed to close libuv loop: " << uv_err_name(result) << endl;
}
else
{
    cout << "libuv loop is closed successfully!\n";
}
Konstantin Gindemit
  • 382
  • 1
  • 4
  • 10
2

I like the solution given by Konstantin Gindemit

I did run into a couple of problems however. His on_uv_close() function ends with a core dump. Also the uv_signal_t variable was causing valgrind to report a "definitely lost" memory leak.

I am using his code with fixes for these 2 situations.

void on_uv_walk(uv_handle_t* handle, void* arg) {
    uv_close(handle, NULL);
}

void on_sigint_received(uv_signal_t *handle, int signum) {
    int result = uv_loop_close(handle->loop);
    if(result == UV_EBUSY) {
        uv_walk(handle->loop, on_uv_walk, NULL);
    }
}

int main(int argc, char *argv[]) {
    uv_signal_t *sigint = new uv_signal_t;
    uv_signal_init(uv_default_loop(), sigint);
    uv_signal_start(sigint, on_sigint_received, SIGINT);
    uv_loop_t* main_loop = uv_default_loop();
...
    uv_run(main_loop, UV_RUN_DEFAULT));
    uv_loop_close(uv_default_loop());
    delete sigint;
    return 0;
}
Jeff Greer
  • 113
  • 7
0

I think this is basically the same of Jeff Greer's answer but with a little more context.

Related to get valgrind working with the latest version of libuv I found some line's blessed by libuv/test/task.h that make the 2 false positive deallocations go away.

// You have to define this. DON'T define this to assert as it will skip the call when NDEBUG is defined
#define ASSERT(expr) expr

// Secret knowledge hidden within libuv's test folder
static void close_walk_cb(uv_handle_t* handle, void* arg) {
  if (!uv_is_closing(handle))
    uv_close(handle, NULL);
}

static void close_loop(uv_loop_t* loop) {
  uv_walk(loop, close_walk_cb, NULL);
  uv_run(loop, UV_RUN_DEFAULT);
}

/* This macro cleans up the event loop. This is used to avoid valgrind
 * warnings about memory being "leaked" by the event loop.
 */
#define MAKE_VALGRIND_HAPPY(loop)                   \
  do {                                              \
    close_loop(loop);                               \
    ASSERT(0 == uv_loop_close(loop));               \
    uv_library_shutdown();                          \
  } while (0)

You use it like so

// Signal handling stuff for ctrl+C 
// This one is needed if you something set on server->data needs to be freed
void on_close(uv_handle_t* handle) {
    // free(handle->data); <- Uncomment if you know you need to free this
}

// Actual signal handler
void on_signal(uv_signal_t *handle, int signum) {
    uv_tcp_t *server = handle->data;
    uv_close((uv_handle_t*) server, on_close);
    uv_stop(handle->loop);
}

int main() {
    uv_tcp_t server;
    uv_tcp_init(uv_default_loop(), &server);

    struct sockaddr_in addr;
    uv_ip4_addr("0.0.0.0", DEFAULT_PORT, &addr);

    uv_tcp_bind(&server, (const struct sockaddr*)&addr, 0);
    int r = uv_listen((uv_stream_t*) &server, DEFAULT_BACKLOG, on_new_connection);

    // Don't forget this step if you're closing the program with ctrl+C
    uv_signal_t sig;
    uv_signal_init(loop, &sig);
    sig.data = &server;
    uv_signal_start(&sig, on_signal, SIGINT);

    uv_run(loop, UV_RUN_DEFAULT);

    uv_loop_close(loop);

    MAKE_VALGRIND_HAPPY(uv_default_loop());
}

Output of valgrind when just starting and closing the server using using ctrl+C with no requests MAKE_VALGRIND_HAPPY is commented out

make; valgrind --leak-check=full --show-leak-kinds=all --errors-for-leak-kinds=all --error-exitcode=1 --exit-on-first-error=yes ./server
make: `server' is up to date.
==9426== Memcheck, a memory error detector
==9426== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==9426== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==9426== Command: ./server
==9426== 
^Csig.c:410: Closing
==9426== 
==9426== HEAP SUMMARY:
==9426==     in use at exit: 200 bytes in 2 blocks
==9426==   total heap usage: 4 allocs, 2 frees, 1,256 bytes allocated
==9426== 
==9426== 72 bytes in 1 blocks are still reachable in loss record 1 of 2
==9426==    at 0x54AE1F4: calloc (vg_replace_malloc.c:1328)
==9426==    by 0x54EDD6F: uv_loop_init (in /usr/lib64/libuv.so.1.0.0)
==9426==    by 0x54E64A7: uv_default_loop (in /usr/lib64/libuv.so.1.0.0)
==9426==    by 0x401B3F: main (sig.c:445)
==9426== 
==9426== 
==9426== Exit program on first error (--exit-on-first-error=yes)

Output of valgrind when just starting and closing the server using ctrl+C with no requests MAKE_VALGRIND_HAPPY executed at end of main

make; valgrind --leak-check=full --show-leak-kinds=all --errors-for-leak-kinds=all --error-exitcode=1 --exit-on-first-error=yes ./server
clang -o server -Wall -Wsizeof-pointer-div -Wmissing-field-initializers -O0 -g3 sig.c -luv
==9542== Memcheck, a memory error detector
==9542== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==9542== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==9542== Command: ./server
==9542== 
^Csig.c:410: Closing
==9542== 
==9542== HEAP SUMMARY:
==9542==     in use at exit: 0 bytes in 0 blocks
==9542==   total heap usage: 4 allocs, 4 frees, 1,256 bytes allocated
==9542== 
==9542== All heap blocks were freed -- no leaks are possible
==9542== 
==9542== For lists of detected and suppressed errors, rerun with: -s
==9542== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
J. Rehbein
  • 107
  • 1
  • 9