Add CHECK's to debug QuicStreamSequencerBuffer in weird state. Add a destruction indicator to detec…

net/quic/core/quic_stream_sequencer_buffer.cc

Index: net/quic/core/quic_stream_sequencer_buffer.cc
diff --git a/net/quic/core/quic_stream_sequencer_buffer.cc b/net/quic/core/quic_stream_sequencer_buffer.cc
index e6bd9110390e11ca13743ddfccc53dc0a1eeab07..8a16b4f6efb31be0da077629c177a3ca5a9485ec 100644
--- a/net/quic/core/quic_stream_sequencer_buffer.cc
+++ b/net/quic/core/quic_stream_sequencer_buffer.cc
@@ -4,6 +4,7 @@
 
 #include "net/quic/core/quic_stream_sequencer_buffer.h"
 
+#include "base/debug/stack_trace.h"
 #include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
@@ -50,12 +51,17 @@ QuicStreamSequencerBuffer::QuicStreamSequencerBuffer(size_t max_capacity_bytes)
       blocks_count_(
           ceil(static_cast<double>(max_capacity_bytes) / kBlockSizeBytes)),
       total_bytes_read_(0),
-      blocks_(nullptr) {
+      blocks_(nullptr),
+      destruction_indicator_(123456) {
+  CHECK(blocks_count_ > 1) << "blocks_count_ = " << blocks_count_

[Ryan Hamilton, 2016/11/23 20:42:40]

nit: CHECK_GT()

[danzh1, 2016/11/23 21:39:24]

Done.

+                           << ", max_buffer_capacity_bytes_ = "
+                           << max_buffer_capacity_bytes_;
   Clear();
 }
 
 QuicStreamSequencerBuffer::~QuicStreamSequencerBuffer() {
   Clear();
+  destruction_indicator_ = 654321;
 }
 
 void QuicStreamSequencerBuffer::Clear() {
@@ -92,6 +98,7 @@ QuicErrorCode QuicStreamSequencerBuffer::OnStreamData(
     QuicTime timestamp,
     size_t* const bytes_buffered,
     std::string* error_details) {
+  CHECK_EQ(destruction_indicator_, 123456) << "This object has been destructed";
   *bytes_buffered = 0;
   QuicStreamOffset offset = starting_offset;
   size_t size = data.size();
@@ -273,9 +280,12 @@ QuicErrorCode QuicStreamSequencerBuffer::Readv(const iovec* dest_iov,
                                                size_t dest_count,
                                                size_t* bytes_read,
                                                string* error_details) {
+  CHECK_EQ(destruction_indicator_, 123456) << "This object has been destructed";
+
   *bytes_read = 0;
   for (size_t i = 0; i < dest_count && ReadableBytes() > 0; ++i) {
     char* dest = reinterpret_cast<char*>(dest_iov[i].iov_base);
+    CHECK_NE(dest, nullptr);
     size_t dest_remaining = dest_iov[i].iov_len;
     while (dest_remaining > 0 && ReadableBytes() > 0) {
       size_t block_idx = NextBlockToRead();
@@ -290,9 +300,10 @@ QuicErrorCode QuicStreamSequencerBuffer::Readv(const iovec* dest_iov,
         *error_details = StringPrintf(
             "QuicStreamSequencerBuffer error:"
             " Readv() dest == nullptr: %s"
-            " blocks_[%" PRIuS "] == nullptr: %s",
+            " blocks_[%" PRIuS "] == nullptr: %s, stack trace %s",
             (dest == nullptr ? "true" : "false"), block_idx,
-            (blocks_[block_idx] == nullptr ? "true" : "false"));
+            (blocks_[block_idx] == nullptr ? "true" : "false"),
+            base::debug::StackTrace().ToString().c_str());

[Ryan Hamilton, 2016/11/23 20:42:40]

I'm not sure that sending stack traces to google is something we're allowed to
do from a privacy perspective.

[danzh1, 2016/11/23 21:39:24]

removed as I switch to crashing right above.

         return QUIC_STREAM_SEQUENCER_INVALID_STATE;
       }
       memcpy(dest, blocks_[block_idx]->buffer + start_offset_in_block,
@@ -329,6 +340,8 @@ QuicErrorCode QuicStreamSequencerBuffer::Readv(const iovec* dest_iov,
 
 int QuicStreamSequencerBuffer::GetReadableRegions(struct iovec* iov,
                                                   int iov_count) const {
+  CHECK_EQ(destruction_indicator_, 123456) << "This object has been destructed";
+
   DCHECK(iov != nullptr);
   DCHECK_GT(iov_count, 0);
 
@@ -387,6 +400,8 @@ int QuicStreamSequencerBuffer::GetReadableRegions(struct iovec* iov,
 
 bool QuicStreamSequencerBuffer::GetReadableRegion(iovec* iov,
                                                   QuicTime* timestamp) const {
+  CHECK_EQ(destruction_indicator_, 123456) << "This object has been destructed";
+
   if (ReadableBytes() == 0) {
     iov[0].iov_base = nullptr;
     iov[0].iov_len = 0;
@@ -425,6 +440,8 @@ bool QuicStreamSequencerBuffer::GetReadableRegion(iovec* iov,
 }
 
 bool QuicStreamSequencerBuffer::MarkConsumed(size_t bytes_used) {
+  CHECK_EQ(destruction_indicator_, 123456) << "This object has been destructed";
+
   if (bytes_used > ReadableBytes()) {
     return false;
   }