From 97969808f6d3c5d97850cea477229fde87aab4ad Mon Sep 17 00:00:00 2001 From: Brian Silverman Date: Tue, 28 Jul 2015 13:43:30 -0700 Subject: [PATCH] Fix a documented memory leak. Despite the old comments, re-initing the msg_t leaks a refcount to metadata in some situations. v1_decoder looks like it isn't tested any more, but it seems like a good idea to fix it because it has the exact same piece of buggy code v2_decoder does. --- src/v1_decoder.cpp | 14 ++++++-------- src/v2_decoder.cpp | 6 ++---- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/v1_decoder.cpp b/src/v1_decoder.cpp index 508f3210..10b8c3f7 100644 --- a/src/v1_decoder.cpp +++ b/src/v1_decoder.cpp @@ -80,10 +80,9 @@ int zmq::v1_decoder_t::one_byte_size_ready (unsigned char const*) return -1; } - // in_progress is initialised at this point so in theory we should - // close it before calling zmq_msg_init_size, however, it's a 0-byte - // message and thus we can treat it as uninitialised... - int rc = in_progress.init_size (*tmpbuf - 1); + int rc = in_progress.close(); + assert(rc == 0); + rc = in_progress.init_size (*tmpbuf - 1); if (rc != 0) { errno_assert (errno == ENOMEM); rc = in_progress.init (); @@ -123,10 +122,9 @@ int zmq::v1_decoder_t::eight_byte_size_ready (unsigned char const*) const size_t msg_size = static_cast (payload_length - 1); - // in_progress is initialised at this point so in theory we should - // close it before calling init_size, however, it's a 0-byte - // message and thus we can treat it as uninitialised... - int rc = in_progress.init_size (msg_size); + int rc = in_progress.close(); + assert(rc == 0); + rc = in_progress.init_size (msg_size); if (rc != 0) { errno_assert (errno == ENOMEM); rc = in_progress.init (); diff --git a/src/v2_decoder.cpp b/src/v2_decoder.cpp index 0cec24e0..43bd9d60 100644 --- a/src/v2_decoder.cpp +++ b/src/v2_decoder.cpp @@ -108,10 +108,8 @@ int zmq::v2_decoder_t::size_ready(uint64_t msg_size, unsigned char const* read_p return -1; } - // in_progress is initialised at this point so in theory we should - // close it before calling init_size, however, it's a 0-byte - // message and thus we can treat it as uninitialised. - int rc = -1; + int rc = in_progress.close(); + assert(rc == 0); // the current message can exceed the current buffer. We have to copy the buffer // data into a new message and complete it in the next receive.