Release SpdySession socket read buffer

net/spdy/spdy_session.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/spdy/spdy_session.h"
 
 #include <algorithm>
 #include <limits>
 #include <map>
 #include <utility>
@@ skipping 719 matching lines @@
                          const SettingsMap& initial_settings,
                          TimeFunc time_func,
                          ServerPushDelegate* push_delegate,
                          ProxyDelegate* proxy_delegate,
                          NetLog* net_log)
     : in_io_loop_(false),
       spdy_session_key_(spdy_session_key),
       pool_(NULL),
       http_server_properties_(http_server_properties),
       transport_security_state_(transport_security_state),
-      read_buffer_(new IOBuffer(kReadBufferSize)),
+      // read_buffer_(new IOBuffer(kReadBufferSize)),
       stream_hi_water_mark_(kFirstStreamId),
       last_accepted_push_stream_id_(0),
       unclaimed_pushed_streams_(this),
       push_delegate_(push_delegate),
       num_pushed_streams_(0u),
       num_active_pushed_streams_(0u),
       bytes_pushed_count_(0u),
       bytes_pushed_and_unclaimed_count_(0u),
       in_flight_write_frame_type_(DATA),
       in_flight_write_frame_size_(0),
@@ skipping 611 matching lines @@
 }
 
 size_t SpdySession::DumpMemoryStats(StreamSocket::SocketMemoryStats* stats,
                                     bool* is_session_active) const {
   // TODO(xunjieli): Include |pending_create_stream_queues_| when WeakPtr is
   // supported in memory_usage_estimator.h.
   *is_session_active = is_active();
   connection_->DumpMemoryStats(stats);
 
   // |connection_| is estimated in stats->total_size. |read_buffer_| is
-  // estimated in kReadBufferSize. TODO(xunjieli): Make them use EMU().
-  return stats->total_size + kReadBufferSize +
+  // estimated in |read_buffer_size|. TODO(xunjieli): Make them use EMU().
+  size_t read_buffer_size = read_buffer_ ? kReadBufferSize : 0;
+  return stats->total_size + read_buffer_size +
          SpdyEstimateMemoryUsage(spdy_session_key_) +
          SpdyEstimateMemoryUsage(pooled_aliases_) +
          SpdyEstimateMemoryUsage(active_streams_) +
          SpdyEstimateMemoryUsage(unclaimed_pushed_streams_) +
          SpdyEstimateMemoryUsage(created_streams_) +
          SpdyEstimateMemoryUsage(write_queue_) +
          SpdyEstimateMemoryUsage(in_flight_write_) +
          SpdyEstimateMemoryUsage(buffered_spdy_framer_) +
          SpdyEstimateMemoryUsage(initial_settings_) +
          SpdyEstimateMemoryUsage(stream_send_unstall_queue_) +
@@ skipping 469 matching lines @@
     }
   }
 
   CHECK(in_io_loop_);
   in_io_loop_ = false;
 
   return result;
 }
 
 int SpdySession::DoRead() {
+  DCHECK(!read_buffer_);
   CHECK(in_io_loop_);
 
   CHECK(connection_);
   CHECK(connection_->socket());
   read_state_ = READ_STATE_DO_READ_COMPLETE;
   int rv = ERR_READ_IF_READY_NOT_IMPLEMENTED;
+  read_buffer_ = new IOBuffer(kReadBufferSize);
   if (base::FeatureList::IsEnabled(Socket::kReadIfReadyExperiment)) {
     rv = connection_->socket()->ReadIfReady(
         read_buffer_.get(), kReadBufferSize,
         base::Bind(&SpdySession::PumpReadLoop, weak_factory_.GetWeakPtr(),
                    READ_STATE_DO_READ));
-    // TODO(xunjieli): Follow-up CL to release |read_buffer_|.
-    // https://crbug.com/690915.
+    if (rv == ERR_IO_PENDING)

[DmitrySkiba, 2017/03/06 21:42:37]

Are we killing buffer with read operation in progress? I thought read_buffer_
should be alive until DoReadComplete() is called.

[xunjieli, 2017/03/06 21:58:59]

Great question. The new ReadIfReady() method that I implemented allows us to
release |read_buffer_| if a read is pending.

The CL is at https://codereview.chromium.org/2593063003/, which is expected to
land soon.

[xunjieli, 2017/03/06 22:00:51]

Btw, if you apply the CL linked above and start Chrome with
"--enable-features=SocketReadIfReady", all BIOAdapter buffers will be gone. 

+      read_buffer_ = nullptr;
   }
   if (rv == ERR_READ_IF_READY_NOT_IMPLEMENTED) {
     // Fallback to regular Read().
     return connection_->socket()->Read(
         read_buffer_.get(), kReadBufferSize,
         base::Bind(&SpdySession::PumpReadLoop, weak_factory_.GetWeakPtr(),
                    READ_STATE_DO_READ_COMPLETE));
   }
-  if (rv == ERR_IO_PENDING)
+  if (rv == ERR_IO_PENDING) {
     read_state_ = READ_STATE_DO_READ;
+    return ERR_IO_PENDING;
+  }
   return rv;
 }
 
 int SpdySession::DoReadComplete(int result) {
+  DCHECK(read_buffer_);
   CHECK(in_io_loop_);
 
   // Parse a frame.  For now this code requires that the frame fit into our
   // buffer (kReadBufferSize).
   // TODO(mbelshe): support arbitrarily large frames!
 
   if (result == 0) {
     DoDrainSession(ERR_CONNECTION_CLOSED, "Connection closed");
     return ERR_CONNECTION_CLOSED;
   }
@@ skipping 17 matching lines @@
     data += bytes_processed;
 
     if (availability_state_ == STATE_DRAINING) {
       return ERR_CONNECTION_CLOSED;
     }
 
     DCHECK_EQ(buffered_spdy_framer_->spdy_framer_error(),
               SpdyFramer::SPDY_NO_ERROR);
   }
 
+  read_buffer_ = nullptr;
   read_state_ = READ_STATE_DO_READ;
   return OK;
 }
 
 void SpdySession::PumpWriteLoop(WriteState expected_write_state, int result) {
   CHECK(!in_io_loop_);
   DCHECK_EQ(write_state_, expected_write_state);
 
   DoWriteLoop(expected_write_state, result);
 
@@ skipping 1238 matching lines @@
     if (!queue->empty()) {
       SpdyStreamId stream_id = queue->front();
       queue->pop_front();
       return stream_id;
     }
   }
   return 0;
 }
 
 }  // namespace net