[rabbitmq-discuss] rabbitmq-c memory leak?

[Dushin Fred]

Sorry, I should have been more clear.

The original issue I hit was from a build off the 2.3 tag (418f31f).  So the patch I applied was slightly different:

> git diff
diff --git a/src/ChannelImpl.cpp b/src/ChannelImpl.cpp
index 9749556..864502b 100644
--- a/src/ChannelImpl.cpp
+++ b/src/ChannelImpl.cpp
@@ -152,6 +152,7 @@ amqp_channel_t ChannelImpl::GetChannel()
 
 void ChannelImpl::ReturnChannel(amqp_channel_t channel)
 {
+       amqp_maybe_release_buffers_on_channel(m_connection, channel);
     m_channels.at(channel) = CS_Open;
     m_last_used_channel = channel;
 }

I fully respect that this may not be the "right" fix, but in my limited scenario (the sender.cpp program I posted off github), it does the trick.

I have done zero analysis of the right fix, but I'd be happy to crack open the code and look into it, if the root cause of the bug is not immediately evident to you.  Otherwise, if you have any thoughts, feel free to let me know.

And of course I would prefer (as I suspect many would) that the fix go into master, as this bug seems to effectively render the C++ client library unsuitable for production, and I would prefer not to maintain a fix that is not endorsed by the author/maintainer.

-Fred

On Apr 10, 2014, at 1:11 AM, Alan Antonuk <alan.antonuk at gmail.com> wrote:

> What version of SimpleAmqpClient are you using? The lines surrounding your change don't seem to match what is currently in the master branch of SimpleAmqpClient. I believe behavior you're seeing was addressed a while ago (see Issue #56). Please download the master branch and build from that, you will also need rabbitmq-c v0.4.1 or newer for this to work.
> 
> FYI: the patch you are suggesting using will likely cause undefined behavior. Because of the way rabbitmq-c's memory management works, the SimpleAmqpClient cannot call amqp_maybe_release_buffers() until it no longer holds any references to frames returned from amqp_simple_wait_frame(). Once amqp_maybe_release_buffers() (or amqp_maybe_release_buffers_on_channel()) any outstanding frames are considered freed and their use will lead to undefined behavior. SimpleAmqpClient maintains a list of frames that it has seen from the broker, in ChannelImpl::MaybeReleaseBuffersOnChannel it checks to see if there are any frames that it still has a reference to before calling amqp_maybe_release_buffers()
> 
> -Alan
> 
> 
> On Wed, Apr 9, 2014 at 3:17 PM, Dushin Fred <fred at dushin.net> wrote:
> That works for me, too.  Thanks, Abhay.
> 
> Alan, would you like a pull request?  Is there a timeline for folding this an the other recent fixes into master?  Anything I can do to help expedite?
> 
> -Fred
> 
> On Apr 9, 2014, at 4:54 AM, Abhay Rajure <arajure at gmail.com> wrote:
> 
>> Try this --
>> 
>> In File SimpleAmqpClient/src/ChannelImpl.cpp,
>> call amqp_maybe_release_buffers_on_channel() in ReturnChannel().
>> 
>> void ChannelImpl::ReturnChannel(amqp_chanel_t channel)
>> {
>>   +amqp_maybe_release_buffers_on_channel(m_connection, channel);
>>    m_free_channels.push(channel);
>> }
> 
> 
> _______________________________________________
> rabbitmq-discuss mailing list
> rabbitmq-discuss at lists.rabbitmq.com
> https://lists.rabbitmq.com/cgi-bin/mailman/listinfo/rabbitmq-discuss
> 
> 
> _______________________________________________
> rabbitmq-discuss mailing list
> rabbitmq-discuss at lists.rabbitmq.com
> https://lists.rabbitmq.com/cgi-bin/mailman/listinfo/rabbitmq-discuss

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rabbitmq.com/pipermail/rabbitmq-discuss/attachments/20140410/797985e0/attachment.html>