commit 50d4c71c942c1c4b9c54412507301a5f6630d74b
author wub <wub@google.com> Tue May 19 15:48:28 2020 -0700
committer Copybara-Service <copybara-worker@google.com> Tue May 19 15:49:01 2020 -0700
tree 0bf3a48c89d03f7d21c16ea10ca64f80e720b3e2
parent 403342681c37abe75ccb866418dd6cd23eac35d8

Change quicpacketwriter::getnextwritelocation to return a release function used to free a writer-allocated buffer. use this release function to avoid buffer leak if a writer buffer is allocated but writer->writepacket is not called. protected by --gfe2_reloadable_flag_quic_avoid_leak_writer_buffer.

This is needed to fix a buffer leak when the new PigeonWriter is used.

PiperOrigin-RevId: 312369772
Change-Id: I0d12327b58989ec69e401b03f7b9ebb4d8ce22fd

quic/core/quic_packet_creator_test.cc

diff --git a/quic/core/quic_packet_creator_test.cc b/quic/core/quic_packet_creator_test.cc
index 19267c9..ca59097 100644
--- a/quic/core/quic_packet_creator_test.cc
+++ b/quic/core/quic_packet_creator_test.cc
@@ -162,7 +162,8 @@
                        connection_id_.length()),
         data_("foo"),
         creator_(connection_id_, &client_framer_, &delegate_, &producer_) {
-    EXPECT_CALL(delegate_, GetPacketBuffer()).WillRepeatedly(Return(nullptr));
+    EXPECT_CALL(delegate_, GetPacketBuffer())
+        .WillRepeatedly(Return(QuicPacketBuffer()));
     creator_.SetEncrypter(ENCRYPTION_INITIAL, std::make_unique<NullEncrypter>(
                                                   Perspective::IS_CLIENT));
     creator_.SetEncrypter(ENCRYPTION_HANDSHAKE, std::make_unique<NullEncrypter>(
@@ -1725,9 +1726,10 @@
 
 TEST_P(QuicPacketCreatorTest, FlushWithExternalBuffer) {
   creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE);
-  char external_buffer[kMaxOutgoingPacketSize];
-  char* expected_buffer = external_buffer;
-  EXPECT_CALL(delegate_, GetPacketBuffer()).WillOnce(Return(expected_buffer));
+  char* buffer = new char[kMaxOutgoingPacketSize];
+  QuicPacketBuffer external_buffer = {buffer,
+                                      [](const char* p) { delete[] p; }};
+  EXPECT_CALL(delegate_, GetPacketBuffer()).WillOnce(Return(external_buffer));
 
   QuicFrame frame;
   MakeIOVector("test", &iov_);
@@ -1738,10 +1740,13 @@
       /*needs_full_padding=*/true, NOT_RETRANSMISSION, &frame));
 
   EXPECT_CALL(delegate_, OnSerializedPacket(_))
-      .WillOnce(Invoke([expected_buffer](SerializedPacket serialized_packet) {
-        EXPECT_EQ(expected_buffer, serialized_packet.encrypted_buffer);
+      .WillOnce(Invoke([&external_buffer](SerializedPacket serialized_packet) {
+        EXPECT_EQ(external_buffer.buffer, serialized_packet.encrypted_buffer);
       }));
   creator_.FlushCurrentPacket();
+  if (!GetQuicReloadableFlag(quic_avoid_leak_writer_buffer)) {
+    delete[] buffer;
+  }
 }
 
 // Test for error found in
@@ -2311,7 +2316,7 @@
               MaybeBundleAckOpportunistically,
               (),
               (override));
-  MOCK_METHOD(char*, GetPacketBuffer, (), (override));
+  MOCK_METHOD(QuicPacketBuffer, GetPacketBuffer, (), (override));
   MOCK_METHOD(void, OnSerializedPacket, (SerializedPacket), (override));
   MOCK_METHOD(void,
               OnUnrecoverableError,
@@ -2466,7 +2471,8 @@
                  &delegate_,
                  &producer_),
         ack_frame_(InitAckFrame(1)) {
-    EXPECT_CALL(delegate_, GetPacketBuffer()).WillRepeatedly(Return(nullptr));
+    EXPECT_CALL(delegate_, GetPacketBuffer())
+        .WillRepeatedly(Return(QuicPacketBuffer()));
     creator_.SetEncrypter(
         ENCRYPTION_FORWARD_SECURE,
         std::make_unique<NullEncrypter>(Perspective::IS_CLIENT));