You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by ma...@apache.org on 2018/03/12 07:25:08 UTC

[trafficserver] branch quic-latest updated: Fix memory leaks under iocore/net/quic

This is an automated email from the ASF dual-hosted git repository.

maskit pushed a commit to branch quic-latest
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/quic-latest by this push:
     new 3f85b66  Fix memory leaks under iocore/net/quic
3f85b66 is described below

commit 3f85b668c732baf2b354f95351ca5d374df243e7
Author: Masakazu Kitajo <ma...@apache.org>
AuthorDate: Mon Mar 12 16:24:32 2018 +0900

    Fix memory leaks under iocore/net/quic
---
 iocore/net/quic/QUICFrame.cc  |  3 +++
 iocore/net/quic/QUICFrame.h   | 16 ++++++++++++++++
 iocore/net/quic/QUICPacket.cc | 34 +++++++++++++++++-----------------
 iocore/net/quic/QUICPacket.h  | 19 ++++++++++++++-----
 4 files changed, 50 insertions(+), 22 deletions(-)

diff --git a/iocore/net/quic/QUICFrame.cc b/iocore/net/quic/QUICFrame.cc
index f7445ee..92fd961 100644
--- a/iocore/net/quic/QUICFrame.cc
+++ b/iocore/net/quic/QUICFrame.cc
@@ -332,6 +332,9 @@ void
 QUICAckFrame::reset(const uint8_t *buf, size_t len)
 {
   QUICFrame::reset(buf, len);
+  if (this->_ack_block_section) {
+    delete this->_ack_block_section;
+  }
   this->_ack_block_section = new AckBlockSection(buf + this->_get_ack_block_section_offset(), this->ack_block_count());
 }
 
diff --git a/iocore/net/quic/QUICFrame.h b/iocore/net/quic/QUICFrame.h
index 0cc90d5..6d2044e 100644
--- a/iocore/net/quic/QUICFrame.h
+++ b/iocore/net/quic/QUICFrame.h
@@ -587,6 +587,7 @@ public:
   static void
   delete_stream_frame(QUICFrame *frame)
   {
+    frame->~QUICFrame();
     quicStreamFrameAllocator.free(static_cast<QUICStreamFrame *>(frame));
   }
 
@@ -600,90 +601,105 @@ public:
   static void
   delete_padding_frame(QUICFrame *frame)
   {
+    frame->~QUICFrame();
     quicPaddingFrameAllocator.free(static_cast<QUICPaddingFrame *>(frame));
   }
 
   static void
   delete_rst_stream_frame(QUICFrame *frame)
   {
+    frame->~QUICFrame();
     quicRstStreamFrameAllocator.free(static_cast<QUICRstStreamFrame *>(frame));
   }
 
   static void
   delete_connection_close_frame(QUICFrame *frame)
   {
+    frame->~QUICFrame();
     quicConnectionCloseFrameAllocator.free(static_cast<QUICConnectionCloseFrame *>(frame));
   }
 
   static void
   delete_application_close_frame(QUICFrame *frame)
   {
+    frame->~QUICFrame();
     quicApplicationCloseFrameAllocator.free(static_cast<QUICApplicationCloseFrame *>(frame));
   }
 
   static void
   delete_max_data_frame(QUICFrame *frame)
   {
+    frame->~QUICFrame();
     quicMaxDataFrameAllocator.free(static_cast<QUICMaxDataFrame *>(frame));
   }
 
   static void
   delete_max_stream_data_frame(QUICFrame *frame)
   {
+    frame->~QUICFrame();
     quicMaxStreamDataFrameAllocator.free(static_cast<QUICMaxStreamDataFrame *>(frame));
   }
 
   static void
   delete_max_stream_id_frame(QUICFrame *frame)
   {
+    frame->~QUICFrame();
     quicMaxStreamIdFrameAllocator.free(static_cast<QUICMaxStreamIdFrame *>(frame));
   }
 
   static void
   delete_ping_frame(QUICFrame *frame)
   {
+    frame->~QUICFrame();
     quicPingFrameAllocator.free(static_cast<QUICPingFrame *>(frame));
   }
 
   static void
   delete_blocked_frame(QUICFrame *frame)
   {
+    frame->~QUICFrame();
     quicBlockedFrameAllocator.free(static_cast<QUICBlockedFrame *>(frame));
   }
 
   static void
   delete_stream_blocked_frame(QUICFrame *frame)
   {
+    frame->~QUICFrame();
     quicStreamBlockedFrameAllocator.free(static_cast<QUICStreamBlockedFrame *>(frame));
   }
 
   static void
   delete_stream_id_blocked_frame(QUICFrame *frame)
   {
+    frame->~QUICFrame();
     quicStreamIdBlockedFrameAllocator.free(static_cast<QUICStreamIdBlockedFrame *>(frame));
   }
 
   static void
   delete_new_connection_id_frame(QUICFrame *frame)
   {
+    frame->~QUICFrame();
     quicNewConnectionIdFrameAllocator.free(static_cast<QUICNewConnectionIdFrame *>(frame));
   }
 
   static void
   delete_stop_sending_frame(QUICFrame *frame)
   {
+    frame->~QUICFrame();
     quicStopSendingFrameAllocator.free(static_cast<QUICStopSendingFrame *>(frame));
   }
 
   static void
   delete_pong_frame(QUICFrame *frame)
   {
+    frame->~QUICFrame();
     quicPongFrameAllocator.free(static_cast<QUICPongFrame *>(frame));
   }
 
   static void
   delete_retransmission_frame(QUICFrame *frame)
   {
+    frame->~QUICFrame();
     quicRetransmissionFrameAllocator.free(static_cast<QUICRetransmissionFrame *>(frame));
   }
 };
diff --git a/iocore/net/quic/QUICPacket.cc b/iocore/net/quic/QUICPacket.cc
index b52abc9..1fcc8a2 100644
--- a/iocore/net/quic/QUICPacket.cc
+++ b/iocore/net/quic/QUICPacket.cc
@@ -47,7 +47,7 @@ const uint8_t *
 QUICPacketHeader::buf()
 {
   if (this->_buf) {
-    return this->_buf;
+    return this->_buf.get();
   } else {
     // TODO Reuse serialzied data if nothing has changed
     size_t dummy;
@@ -63,16 +63,16 @@ QUICPacketHeader::packet_size() const
 }
 
 QUICPacketHeaderUPtr
-QUICPacketHeader::load(const uint8_t *buf, size_t len, QUICPacketNumber base)
+QUICPacketHeader::load(ats_unique_buf buf, size_t len, QUICPacketNumber base)
 {
   QUICPacketHeaderUPtr header = QUICPacketHeaderUPtr(nullptr, &QUICPacketHeaderDeleter::delete_null_header);
-  if (QUICTypeUtil::has_long_header(buf)) {
+  if (QUICTypeUtil::has_long_header(buf.get())) {
     QUICPacketLongHeader *long_header = quicPacketLongHeaderAllocator.alloc();
-    new (long_header) QUICPacketLongHeader(buf, len, base);
+    new (long_header) QUICPacketLongHeader(std::move(buf), len, base);
     header = QUICPacketHeaderUPtr(long_header, &QUICPacketHeaderDeleter::delete_long_header);
   } else {
     QUICPacketShortHeader *short_header = quicPacketShortHeaderAllocator.alloc();
-    new (short_header) QUICPacketShortHeader(buf, len, base);
+    new (short_header) QUICPacketShortHeader(std::move(buf), len, base);
     header = QUICPacketHeaderUPtr(short_header, &QUICPacketHeaderDeleter::delete_short_header);
   }
   return header;
@@ -133,7 +133,7 @@ QUICPacketType
 QUICPacketLongHeader::type() const
 {
   if (this->_buf) {
-    uint8_t type = this->_buf[0] & 0x7F;
+    uint8_t type = this->_buf.get()[0] & 0x7F;
     if (this->version() == 0x00) {
       return QUICPacketType::VERSION_NEGOTIATION;
     } else {
@@ -149,7 +149,7 @@ QUICConnectionId
 QUICPacketLongHeader::connection_id() const
 {
   if (this->_buf) {
-    return QUICTypeUtil::read_QUICConnectionId(this->_buf + LONG_HDR_OFFSET_CONNECTION_ID, 8);
+    return QUICTypeUtil::read_QUICConnectionId(this->_buf.get() + LONG_HDR_OFFSET_CONNECTION_ID, 8);
   } else {
     return this->_connection_id;
   }
@@ -160,7 +160,7 @@ QUICPacketLongHeader::packet_number() const
 {
   if (this->_buf) {
     const uint8_t packet_number_len = 4;
-    QUICPacketNumber src = QUICTypeUtil::read_QUICPacketNumber(this->_buf + LONG_HDR_OFFSET_PACKET_NUMBER, packet_number_len);
+    QUICPacketNumber src = QUICTypeUtil::read_QUICPacketNumber(this->_buf.get() + LONG_HDR_OFFSET_PACKET_NUMBER, packet_number_len);
     QUICPacketNumber dst = 0;
     QUICPacket::decode_packet_number(dst, src, packet_number_len, this->_base_packet_number);
 
@@ -180,7 +180,7 @@ QUICVersion
 QUICPacketLongHeader::version() const
 {
   if (this->_buf) {
-    return QUICTypeUtil::read_QUICVersion(this->_buf + LONG_HDR_OFFSET_VERSION);
+    return QUICTypeUtil::read_QUICVersion(this->_buf.get() + LONG_HDR_OFFSET_VERSION);
   } else {
     return this->_version;
   }
@@ -196,7 +196,7 @@ const uint8_t *
 QUICPacketLongHeader::payload() const
 {
   if (this->_buf) {
-    return this->_buf + LONG_HDR_OFFSET_PAYLOAD;
+    return this->_buf.get() + LONG_HDR_OFFSET_PAYLOAD;
   } else {
     return this->_payload.get();
   }
@@ -314,7 +314,7 @@ QUICPacketShortHeader::connection_id() const
 {
   if (this->_buf) {
     ink_release_assert(this->has_connection_id());
-    return QUICTypeUtil::read_QUICConnectionId(this->_buf + SHORT_HDR_OFFSET_CONNECTION_ID, 8);
+    return QUICTypeUtil::read_QUICConnectionId(this->_buf.get() + SHORT_HDR_OFFSET_CONNECTION_ID, 8);
   } else {
     return _connection_id;
   }
@@ -330,7 +330,7 @@ QUICPacketShortHeader::packet_number() const
       offset += 8;
     }
 
-    QUICPacketNumber src = QUICTypeUtil::read_QUICPacketNumber(this->_buf + offset, n);
+    QUICPacketNumber src = QUICTypeUtil::read_QUICPacketNumber(this->_buf.get() + offset, n);
     QUICPacketNumber dst = 0;
     QUICPacket::decode_packet_number(dst, src, n, this->_base_packet_number);
 
@@ -357,7 +357,7 @@ QUICPacketShortHeader::_packet_number_len() const
 {
   QUICPacketShortHeaderType type;
   if (this->_buf) {
-    type = static_cast<QUICPacketShortHeaderType>(this->_buf[0] & 0x1F);
+    type = static_cast<QUICPacketShortHeaderType>(this->_buf.get()[0] & 0x1F);
   } else {
     type = this->_packet_number_type;
   }
@@ -393,7 +393,7 @@ bool
 QUICPacketShortHeader::has_connection_id() const
 {
   if (this->_buf) {
-    return QUICTypeUtil::has_connection_id(this->_buf);
+    return QUICTypeUtil::has_connection_id(this->_buf.get());
   } else {
     return this->_has_connection_id;
   }
@@ -403,7 +403,7 @@ const uint8_t *
 QUICPacketShortHeader::payload() const
 {
   if (this->_buf) {
-    return this->_buf + this->size();
+    return this->_buf.get() + this->size();
   } else {
     return this->_payload.get();
   }
@@ -419,7 +419,7 @@ QUICKeyPhase
 QUICPacketShortHeader::key_phase() const
 {
   if (this->_buf) {
-    if (this->_buf[0] & 0x20) {
+    if (this->_buf.get()[0] & 0x20) {
       return QUICKeyPhase::PHASE_1;
     } else {
       return QUICKeyPhase::PHASE_0;
@@ -664,7 +664,7 @@ QUICPacketFactory::create(ats_unique_buf buf, size_t len, QUICPacketNumber base_
   ats_unique_buf plain_txt = ats_unique_malloc(max_plain_txt_len);
   size_t plain_txt_len     = 0;
 
-  QUICPacketHeaderUPtr header = QUICPacketHeader::load(buf.release(), len, base_packet_number);
+  QUICPacketHeaderUPtr header = QUICPacketHeader::load(std::move(buf), len, base_packet_number);
 
   if (header->has_version() && !QUICTypeUtil::is_supported_version(header->version())) {
     // We can't decrypt packets that have unknown versions
diff --git a/iocore/net/quic/QUICPacket.h b/iocore/net/quic/QUICPacket.h
index 6992b79..be45b6e 100644
--- a/iocore/net/quic/QUICPacket.h
+++ b/iocore/net/quic/QUICPacket.h
@@ -52,7 +52,11 @@ using QUICPacketHeaderUPtr        = std::unique_ptr<QUICPacketHeader, QUICPacket
 class QUICPacketHeader
 {
 public:
-  QUICPacketHeader(const uint8_t *buf, size_t len, QUICPacketNumber base) : _buf(buf), _buf_len(len), _base_packet_number(base) {}
+  QUICPacketHeader(ats_unique_buf buf, size_t len, QUICPacketNumber base)
+    : _buf(std::move(buf)), _buf_len(len), _base_packet_number(base)
+  {
+  }
+  ~QUICPacketHeader() {}
   const uint8_t *buf();
   virtual QUICPacketType type() const            = 0;
   virtual QUICConnectionId connection_id() const = 0;
@@ -104,7 +108,7 @@ public:
    *
    * This creates either a QUICPacketShortHeader or a QUICPacketLongHeader.
    */
-  static QUICPacketHeaderUPtr load(const uint8_t *buf, size_t len, QUICPacketNumber base);
+  static QUICPacketHeaderUPtr load(ats_unique_buf buf, size_t len, QUICPacketNumber base);
 
   /*
    * Build a QUICPacketHeader
@@ -135,7 +139,7 @@ protected:
   QUICPacketHeader(){};
 
   // These two are used only if the instance was created with a buffer
-  const uint8_t *_buf = nullptr;
+  ats_unique_buf _buf = {nullptr, [](void *p) { ats_free(p); }};
   size_t _buf_len     = 0;
 
   // These are used only if the instance was created without a buffer
@@ -157,7 +161,8 @@ class QUICPacketLongHeader : public QUICPacketHeader
 {
 public:
   QUICPacketLongHeader() : QUICPacketHeader(){};
-  QUICPacketLongHeader(const uint8_t *buf, size_t len, QUICPacketNumber base) : QUICPacketHeader(buf, len, base) {}
+  ~QUICPacketLongHeader(){};
+  QUICPacketLongHeader(ats_unique_buf buf, size_t len, QUICPacketNumber base) : QUICPacketHeader(std::move(buf), len, base) {}
   QUICPacketLongHeader(QUICPacketType type, QUICConnectionId connection_id, QUICPacketNumber packet_number,
                        QUICPacketNumber base_packet_number, QUICVersion version, ats_unique_buf buf, size_t len);
   QUICPacketType type() const;
@@ -177,7 +182,8 @@ class QUICPacketShortHeader : public QUICPacketHeader
 {
 public:
   QUICPacketShortHeader() : QUICPacketHeader(){};
-  QUICPacketShortHeader(const uint8_t *buf, size_t len, QUICPacketNumber base) : QUICPacketHeader(buf, len, base) {}
+  ~QUICPacketShortHeader(){};
+  QUICPacketShortHeader(ats_unique_buf buf, size_t len, QUICPacketNumber base) : QUICPacketHeader(std::move(buf), len, base) {}
   QUICPacketShortHeader(QUICPacketType type, QUICKeyPhase key_phase, QUICPacketNumber packet_number,
                         QUICPacketNumber base_packet_number, ats_unique_buf buf, size_t len);
   QUICPacketShortHeader(QUICPacketType type, QUICKeyPhase key_phase, QUICConnectionId connection_id, QUICPacketNumber packet_number,
@@ -214,6 +220,7 @@ public:
   {
     QUICPacketLongHeader *long_header = dynamic_cast<QUICPacketLongHeader *>(header);
     ink_assert(long_header != nullptr);
+    long_header->~QUICPacketLongHeader();
     quicPacketLongHeaderAllocator.free(long_header);
   }
 
@@ -222,6 +229,7 @@ public:
   {
     QUICPacketShortHeader *short_header = dynamic_cast<QUICPacketShortHeader *>(header);
     ink_assert(short_header != nullptr);
+    short_header->~QUICPacketShortHeader();
     quicPacketShortHeaderAllocator.free(short_header);
   }
 };
@@ -320,6 +328,7 @@ public:
   static void
   delete_packet(QUICPacket *packet)
   {
+    packet->~QUICPacket();
     quicPacketAllocator.free(packet);
   }
 };

-- 
To stop receiving notification emails like this one, please contact
maskit@apache.org.