SPDY: Defer SpdyBufferProducer deletions in SpdyWriteQueue

net/spdy/spdy_write_queue.cc

 // Copyright (c) 2013 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_write_queue.h"
 
 #include <cstddef>
+#include <vector>
 
 #include "base/logging.h"
+#include "base/stl_util.h"
 #include "net/spdy/spdy_buffer.h"
 #include "net/spdy/spdy_buffer_producer.h"
 #include "net/spdy/spdy_stream.h"
 
 namespace net {
 
 SpdyWriteQueue::PendingWrite::PendingWrite() : frame_producer(NULL) {}
 
 SpdyWriteQueue::PendingWrite::PendingWrite(
     SpdyFrameType frame_type,
@@ skipping 66 matching lines @@
   for (int i = MINIMUM_PRIORITY; i <= MAXIMUM_PRIORITY; ++i) {
     if (priority == i)
       continue;
     for (std::deque<PendingWrite>::const_iterator it = queue_[i].begin();
          it != queue_[i].end(); ++it) {
       DCHECK_NE(it->stream.get(), stream.get());
     }
   }
 #endif
 
+  // Defer deletion until queue iteration is complete, as
+  // SpdyBuffer::~SpdyBuffer() can result in callbacks into SpdyWriteQueue.
+  std::vector<SpdyBufferProducer*> erased_buffer_producers;

[willchan no longer on Chromium, 2014/05/06 16:38:14]

You can also use ScopedVector
(https://code.google.com/p/chromium/codesearch#chromium/src/base/memory/scoped...)
if you want.

[Johnny, 2014/05/06 16:54:27]

Leaving as-is I think, to preserve explicit control & documentation on when
callbacks are invoked.

+
   // Do the actual deletion and removal, preserving FIFO-ness.
   std::deque<PendingWrite>* queue = &queue_[priority];
   std::deque<PendingWrite>::iterator out_it = queue->begin();
   for (std::deque<PendingWrite>::const_iterator it = queue->begin();
        it != queue->end(); ++it) {
     if (it->stream.get() == stream.get()) {
-      delete it->frame_producer;
+      erased_buffer_producers.push_back(it->frame_producer);
     } else {
       *out_it = *it;
       ++out_it;
     }
   }
   queue->erase(out_it, queue->end());
   removing_writes_ = false;
+  STLDeleteElements(&erased_buffer_producers);  // Invokes callbacks.
 }
 
 void SpdyWriteQueue::RemovePendingWritesForStreamsAfter(
     SpdyStreamId last_good_stream_id) {
   CHECK(!removing_writes_);
   removing_writes_ = true;
+  std::vector<SpdyBufferProducer*> erased_buffer_producers;
+
   for (int i = MINIMUM_PRIORITY; i <= MAXIMUM_PRIORITY; ++i) {
     // Do the actual deletion and removal, preserving FIFO-ness.
     std::deque<PendingWrite>* queue = &queue_[i];
     std::deque<PendingWrite>::iterator out_it = queue->begin();
     for (std::deque<PendingWrite>::const_iterator it = queue->begin();
          it != queue->end(); ++it) {
       if (it->stream.get() && (it->stream->stream_id() > last_good_stream_id ||
                                it->stream->stream_id() == 0)) {
-        delete it->frame_producer;
+        erased_buffer_producers.push_back(it->frame_producer);
       } else {
         *out_it = *it;
         ++out_it;
       }
     }
     queue->erase(out_it, queue->end());
   }
   removing_writes_ = false;
+  STLDeleteElements(&erased_buffer_producers);  // Invokes callbacks.
 }
 
 void SpdyWriteQueue::Clear() {
   CHECK(!removing_writes_);
   removing_writes_ = true;
+  std::vector<SpdyBufferProducer*> erased_buffer_producers;
+
   for (int i = MINIMUM_PRIORITY; i <= MAXIMUM_PRIORITY; ++i) {
     for (std::deque<PendingWrite>::iterator it = queue_[i].begin();
          it != queue_[i].end(); ++it) {
-      delete it->frame_producer;
+      erased_buffer_producers.push_back(it->frame_producer);
     }
     queue_[i].clear();
   }
   removing_writes_ = false;
+  STLDeleteElements(&erased_buffer_producers);  // Invokes callbacks.
 }
 
 }  // namespace net