0

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; }
Improve this question
9
  • 5
    Why are you dynamically allocating the std::string at all? You don't need to do that in this code, this will suffice: struct PerSocketData { std::string someString; }; Also, there is no need to call clear() or shrink_to_fit() before destroying a std:string. But, where is the PerSocketData allocated and freed? In any case, if you are just looking at memory usage at the OS level, then it's likely that your app is just caching freed memory for later reuse, which is fairly common practice for any non-trivial app that uses a memory manager layer and doesn't allocate OS memory directly. Commented Feb 12 at 20:55
  • I suppose you should delete the UserData (PerSocketData* ud) as well? At least that seems the most obvious to me, since the struct still holds a string within itself. Commented Feb 12 at 20:58
  • 4
    @falero80s You can't use an external Task Manager to diagnose memory leaks, because they don't understand how your app is actually using memory. Only your app's internal runtime memory manager knows what does and does not constitute a real leak. For instance, memory that your code "frees" and the runtime then caches for later reuse is not a leak. Commented Feb 12 at 21:01
  • 3
    When you delete an object all you do is tell the C++ runtime that the memory associated with that object is no longer required. It needn't return the memory to the global 'pool' immediately. As such, something like task manager is really not a useful tool for diagnosing such issues. Commented Feb 12 at 21:03
  • 1
    std::string is a specialized smart pointer for strings. Having a pointer-to-a-string is really odd, since string itself is already a smart pointer. Commented Feb 12 at 23:58

1 Answer 1

Reset to default
5

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.

Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

This is such a gorgeous information! I was in impression that the OS will show the changes immediately, but you made all sense. I fixed it with everything you wrote here. I am aware of the modern style memory allocation but I had started writing a Qt C++ client side GUI application with new/delete everywhere around 6 months ago. Around 11,000 lines of hand written code and there is zero memory leak according to ClangTidy as well as memcheck via valgrind. I think someone needs to slap me hard enough to make me not use new/delete anymore. Will not use for any new project including this one.
@falero80s One of the reasons modern computers are so fast is because they are lazy as <expletive deleted> and generally don't do anything until they are forced. For example, if you new yourself a million bytes, odds are your system will simply say, "Sure. Here you go, pal." and give you a pointer. It won't actually get that million bytes of storage, or even check whether a million bytes is even available, until you start using it, and even then it probably won't get all million at once, just enough to satisfy the immediate need. Makes tracking the actual program memory usage... tricky.
One for thing sir. 'ws->getUserData()' is not the leak at all. We are not supposed to manually delete the whole returned object ever. We can do anything inside the structure. When I delete the 'ud' object (PerSocketData* ud = ws->getUserData();) inside the .close() callback, the program is throwing 'free(): invalid pointer'. It seems like the uWebSocket library is deleting the client specific/associated object on .close() automatically.
@user4581301 :) I know that. When I was learning C++ I was doing all kinds of interesting testings allocating bytes of the size of the system RAM and I wasn't seeing any bytes being allocated according to the task manager, until I actually start filling those boxes with actual bytes. Last time I remember I allocated exactly 32 billion bytes and the program didn't crash (32 GiB physical RAM is installed), but when I added even a single byte more onto the size and tried allocating with new, the program was crashing. That's when I got to know all that.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.