Closing libUV Handles Correctly

Question

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).

Awe

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. – Haematite

@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. – Awe

Answer 1

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

Hobnail

Which runmode should we use when calling uv_run? – Satterfield

Use UV_RUN_DEFAULT, since all handles are closed and it might take more than one loop iteration for all of them to close and trigger the close callbacks. – Hobnail

uv_loop won't be closed if you have running uv_req left. – Snap

Answer 2

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";
}

Appellant

Answer 3

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;
}

Elfreda

Answer 4

I'd like to post an answer here because, about a decade later, I've used libuv quite often and enough to know that some of the answers here don't offer the best advice. I'm specifically talking about the practice of closing out all libuvs in a final "walk" call and the end of the program - that's only one step better than just letting the OS clean up the resources for you when your process exits! Just imagine if a long-running web server never closing any of its client sockets until it goes down for maintenance. That's basically a memory leak.

In practice, you will often have to close only a set of handles and not all handles associated on the loop. For example, you may want to close the TCP socket handle for a web client after it disconnects (but not all other clients). A similar problem exists when dealing with temporary timer objects and file IO objects.

A pattern I've fallen into is to simply provide a close method.

class http_server final
{
public:
  /* ... */

  void close()
  {
    uv_close(reinterpret_cast<uv_handle_t*>(&m_handle), nullptr);
  }

private:
  uv_tcp_t m_handle{};
};

When the parent class is allocated on the stack, there's not much you need to do at that point. However, a slightly more complicated case presents itself when the parent class (in this case http_client) exists in a container that has the potential to grow over time. In this case, you'll want to remove it from the container after the close operation completes.

The way I've done this is by invoking a second close callback for the code responsible for the container.

class http_client
{
public:
  void set_close_callback(void* udata, void (*cb)(void*, http_client*))
  {
    m_close_udata = udata;
    m_close_cb = cb;
  }

  /* This may be called by the `http_client` or by `http_server`.*/
  void close()
  {
    uv_close(reinterpret_cast<uv_handle_t*>(&m_handle),
             [](uv_handle_t* h) {
      auto* self = static_cast<http_client*>(uv_handle_get_data(h));
      if (self->m_close_cb) {
        self->m_close_cb(self->m_close_udata, self);
      }
    });
  }
};

class http_server final
{
public:
  void add_client(std::unique_ptr<http_client> c)
  {
    c->set_close_callback(this, &http_server::on_client_close);

    m_clients.emplace_back(std::move(c));
  }
protected:
  static void on_client_close(void* self_ptr, http_client* c)
  {
    auto* self = static_cast<http_server*>(self_ptr);
    /* Note: This is a bit inefficient. You can use `std::set` to sort the
     *       pointers (instead of std::vector) in order to get a faster look
     *       up. */
    for (auto it = self->m_clients.begin(); it != self->m_clients.end(); it++) {
      if (it->get() == c) {
        self->m_clients.erase(it);
        break;
      }
    }
  }

private:
  std::vector<std::unique_ptr<http_client>> m_clients;
};

I used http_client and http_server as examples, but it applies to other libuv objects as well.

My main loop typically gets instantiated like this:

void
main()
{
  uv_loop_t loop{};

  uv_loop_init(&loop);

  /* Instantiate UV objects,
   * ex: http_server server(&loop);
   */

  uv_run(&loop, UV_RUN_DEFAULT);

  /* If this line of code is reached, the loop has been stopped and
     will shutdown. */

  /* Close UV objects.
   * ex: server.close();
   */

  /* Run the loop to allow the close operations to complete. */
  const int close_run_ret = uv_run(&loop, UV_RUN_DEFAULT);
  if (close_run_ret != 0) {
    /* error: Not all objects were successfully closed. */
    return;
  }

  uv_loop_close(&loop);
}

Awe

Answer 5

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)

Wanigan