Search code examples
c++uwebsockets

The memory doesn't seem to be freed when deleting an object allocated in the heap. What am I missing here?


Whenever a new client connects, a new std::string instance is created in heap inside the .message callback and set with a value sent by the client (the message object).

I can see the memory (RSS tab) keeps increasing with new each client connection (for example, 9000 something bytes, then 9004 something bytes, then 9008 something bytes, and so on for each two or three new clients), but it's not decreasing even when I am deleting the object in the .close callback when each of those clients exits properly. clientSocket.close() is called from the client side (Qt C++ QWebSocket) and the .close callback is fired on the server side.

For testing, I am sending 1024 bytes from each client.

I am using the following command line to watch the memory usage in real time in Linux Debian.

watch -n0.2 'pmap -x process-id | tail -n1'

I must be doing something very wrong. I am a noob in using this library.

This is the user data structure:

struct PerSocketData {
  std::string* someString {nullptr};
};

.message = [&](uWS::WebSocket<false, true, PerSocketData> *ws, std::string_view message, uWS::OpCode opCode) {
    PerSocketData* ud = ws->getUserData();
    ud->someString = new std::string(message);
}

In the .close callback, I am deleting the object as following:

.close = [](uWS::WebSocket<false, true, PerSocketData> *ws, int /*code*/, std::string_view message) {
     PerSocketData* ud = ws->getUserData();
     ud->someString->clear(); //looks dumb but since the delete wasn't working hence I tried this as well
     ud->someString->shrink_to_fit(); //looks dumb but since the delete wasn't working hence I tried this as well
     delete ud->someString;
     ud->someString = nullptr;
 }

Solution

  • A typical C++ runtime environment manages a "heap" of data it gets from the operating system. It calls a system call which hands it one or more contiguous pages, possibly implicitly zeroed.

    System calls are generally slow, so the C++ runtime then subdivides these pages using its own internal algorithms. It records what parts of these pages are currently in use, and which are not. In normal operation it will never send these pages back to the OS for recycling; in some cases, the OS might have a "I am low on memory" request and the runtime tries to hand back (empty) pages that are not in use, but that isn't the normal course of operation.

    This means that a call to new can cause the process to get more memory, but calls to delete rarely return memory from the process to the OS. External tools looking at your process, unless they know the exact C++ runtime libraries you are using, have no way to know what memory is actually committed to storage and which is spare room.

    That being said, if all you are doing is allocating a string then deleting it, the C++ runtime should be reusing the same memory and not constantly requesting new memory from the operating system. So if the process is using more and more and more memory without bounds that is usually a sign of a leak.

    To find leaks, you usually want to instrument your build. This makes the C++ runtime mark up every bit of memory allocated with where it was asked for, so you can tell at shutdown (or whenever) who still has outstanding unrecycled memory.

    Often this ends up being a few spots, where you forget to delete what you allocated.

    In your case, you had a struct you forgot do deallocate.

    Modern C++ programming techniques highly encourage the use of std::unique_ptr to store heap allocated data; that makes it much harder to casually leak memory. This ends up with a program with no calls to new or delete at all, or next to none.

    std::string* someString {nullptr};
    

    this is an example of an anti-pattern. std::string is a resource management class; why are you storing it on the heap?

    std::string someString {""};
    

    you can store strings of arbitrary length within someString and never call new. It internally manages a buffer, which (for short strings) is within the object and for longer strings it does heap allocation.

    It is rarely a good idea to directly heap allocate a class that in turn internally manages a chunk of the heap.

    99.9/100 times a non-C++ programmer types new in a C++ program they are making a mistake.

    One possible leak location would be:

    ws->getUserData()
    

    if it returns a fresh heap-allocted object. Change it from returning a Chicken* to return a std::unique_ptr<Chicken> (no *), then fix up your code to compile (replace new with make_unique and do some std::move) and your leak evaporates by default.

    The errors you get when working with a unique_ptr are telling you "you aren't keeping track of who owns this heap data properly". It moves resource management mistakes from runtime to compile time, and from maybe being caught to being caught every time.

    Some libraries pass around "observer pointers" liberally. These are non-owning pointers to data structures whose lifetime is (in practice) often ill-described, and more often not well understood by the user of the pointer. That could also be what this function is returning.