AMQP::LibBoostAsioHandler::Watcher "memory leak"

[msukalski]

The Watcher code uses 2 boolean flags, _read_pending and _write_pending, to attempt to prevent more than one read and write ASIO operation from being posted at a time. Unfortunately, the existing implementation is not thread-safe, since changes in the boolean values are not necessarily visible to all threads at the same time. The net result is that the ASIO queues end up with requests that are never processed, and it appears like a slow but always growing memory leak that will eventually consume all available memory.

I've attached my changes to these flags (std::atomic ...) that require C++11 support. compare_exchange_strong() is used to prevent more than one thread from posting a read or write operation at a time. This version of libboostasio.h has also been modified for using asio in standalone mode (and all other boost support has been removed through the use of lambdas everywhere). This code has been tested and verified. There are other ways of addressing this issue, of course, so your mileage may vary...

libboostasio.h.txt

[EHadoux]

Is it related to #237? Because after closing the connection watchers are all that is left.

[msukalski]

No, it doesn't appear to be. You can watch your memory usage grow as "extra" asio read and write operations are posted with a stable connection with a single Watcher. The issue appears because multiple independent threads end up posting asio stream async_read_some() and async_write_calls() -- while only a single operation on an asio strand is dispatched at a time.

[zerodefect]

Can I confirm that once you introduced atomic read/writes on the pending variables that the memory leak(s) (you are reporting) ceased?

[mihacooper]

I have found the similar problem with AMQP::LibBoostAsioHandler. This is my valgrind's massif output

Patch proposed by @msukalski looks like fix the problem.

I have made a real patch from the posted code:
libboostasio.h.patch.txt

But I have nothing to say about its correctness and workability.

[zerodefect]

Ok. Good to know. I'll look at this evening. Thanks for your investigative work.

[mvdwerve]

Let me interject for a second. Nothing in AMQP-CPP is meant to be thread-safe. Generally, you should think in terms of connections, and if you do happen to want to share a connection over threads, or have a general thread-safe handler, you should probably write it yourself.

Mostly, the problem is not processing power but rather waiting for IO, which means that a handler running on a single thread can easily manage multiple connections. That some objects just happen to work well across threads or are thread-safe (like the boost asio service) cannot be assumed about other things in the library. Most likely, if you are using it this way, you'll have more issues than just the libboost handler.

I'll add a general remark towards this end in the readme, about threaded usage overall. Also, the libboost handler is not supported by us, it even says so in the file. Just saying.

[paolopas]

I've written a version of the libboostasio handler that doesn't suffer from the race condition caused by the _read_pending and _write_pending flags (they're no longer there) and fixes several other issues. There's still a lot that can be done to improve it, but if you'd like to try it, you can find it in #558.