Skip to content

Commit

Permalink
Revert of Add some instrumentation to investigate a possible use afte…
Browse files Browse the repository at this point in the history
…r free. (patchset chromium#1 id:1 of https://codereview.chromium.org/1254033003/ )

Reason for revert:
This debugging code didn't find any crashes and the original crash reported in BUG 468529 hasn't happened in two months.

Original issue's description:
> Add some instrumentation to investigate a possible use after free.
>
> BUG=468529
> R=rch@chromium.org
>
> Committed: https://crrev.com/cccb61583618f1159fd3f55eeb1ae845078dc077
> Cr-Commit-Position: refs/heads/master@{#341786}

R=rch@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=468529

Review URL: https://codereview.chromium.org/1351673005

Cr-Commit-Position: refs/heads/master@{#349622}
  • Loading branch information
rtenneti authored and Commit bot committed Sep 18, 2015
1 parent cbaefe5 commit 6b7227c
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 62 deletions.
33 changes: 0 additions & 33 deletions net/quic/quic_http_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@
#include "net/quic/quic_http_stream.h"

#include "base/callback_helpers.h"
#ifdef TEMP_INSTRUMENTATION_468529
#include "base/debug/alias.h"
#endif
#include "base/metrics/histogram_macros.h"
#include "base/strings/stringprintf.h"
#include "net/base/io_buffer.h"
Expand Down Expand Up @@ -49,16 +46,6 @@ QuicHttpStream::QuicHttpStream(
}

QuicHttpStream::~QuicHttpStream() {
#ifdef TEMP_INSTRUMENTATION_468529
liveness_ = DEAD;
stack_trace_ = base::debug::StackTrace();

// Probably not necessary, but just in case compiler tries to optimize out the
// writes to liveness_ and stack_trace_.
base::debug::Alias(&liveness_);
base::debug::Alias(&stack_trace_);
#endif

Close(false);
if (session_)
session_->RemoveObserver(this);
Expand Down Expand Up @@ -380,8 +367,6 @@ void QuicHttpStream::DoCallback(int rv) {

int QuicHttpStream::DoLoop(int rv) {
do {
CrashIfInvalid();

State state = next_state_;
next_state_ = STATE_NONE;
switch (state) {
Expand Down Expand Up @@ -564,22 +549,4 @@ void QuicHttpStream::ResetStream() {
request_body_stream_->Reset();
}

void QuicHttpStream::CrashIfInvalid() const {
#ifdef TEMP_INSTRUMENTATION_468529
Liveness liveness = liveness_;

if (liveness == ALIVE)
return;

// Copy relevant variables onto the stack to guarantee they will be available
// in minidumps, and then crash.
base::debug::StackTrace stack_trace = stack_trace_;

base::debug::Alias(&liveness);
base::debug::Alias(&stack_trace);

CHECK_EQ(ALIVE, liveness);
#endif
}

} // namespace net
29 changes: 0 additions & 29 deletions net/quic/quic_http_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,6 @@

#include <list>

#include "build/build_config.h"

// TODO(rtenneti): Temporary while investigating crbug.com/468529.
// Note base::Debug::StackTrace() is not supported in NACL
// builds so conditionally disabled it there.
#ifndef OS_NACL
#define TEMP_INSTRUMENTATION_468529
#endif

#ifdef TEMP_INSTRUMENTATION_468529
#include "base/debug/stack_trace.h"
#endif
#include "base/memory/weak_ptr.h"
#include "net/base/io_buffer.h"
#include "net/http/http_stream.h"
Expand Down Expand Up @@ -87,14 +75,6 @@ class NET_EXPORT_PRIVATE QuicHttpStream
private:
friend class test::QuicHttpStreamPeer;

#ifdef TEMP_INSTRUMENTATION_468529
// TODO(rtenneti): Temporary while investigating crbug.com/468529
enum Liveness {
ALIVE = 0xCA11AB13,
DEAD = 0xDEADBEEF,
};
#endif

enum State {
STATE_NONE,
STATE_SEND_HEADERS,
Expand Down Expand Up @@ -128,9 +108,6 @@ class NET_EXPORT_PRIVATE QuicHttpStream

void ResetStream();

// TODO(rtenneti): Temporary while investigating crbug.com/468529
void CrashIfInvalid() const;

State next_state_;

base::WeakPtr<QuicChromiumClientSession> session_;
Expand Down Expand Up @@ -186,12 +163,6 @@ class NET_EXPORT_PRIVATE QuicHttpStream

BoundNetLog stream_net_log_;

#ifdef TEMP_INSTRUMENTATION_468529
// TODO(rtenneti): Temporary while investigating crbug.com/468529
Liveness liveness_ = ALIVE;
base::debug::StackTrace stack_trace_;
#endif

base::WeakPtrFactory<QuicHttpStream> weak_factory_;

DISALLOW_COPY_AND_ASSIGN(QuicHttpStream);
Expand Down

0 comments on commit 6b7227c

Please sign in to comment.