Revert 117772 - Add instrumentation to help track down a use-after-free.

This instrumentation checks to see if a deleted ClientSocketPoolBaseHelper::Group is being re-used. If it catches a use-after-free of this class then it will save the callstack where the deletion occurred into the mini-dump, for manual inspection by bug investigator.

BUG=109876


Review URL: http://codereview.chromium.org/9207014

TBR=eroman@chromium.org
Review URL: https://chromiumcodereview.appspot.com/9226030

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@118511 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 4 insertions, 58 deletions

diff --git a/net/socket/client_socket_pool_base.cc b/net/socket/client_socket_pool_base.cc
index e67b4e7..c66e306 100644
--- a/net/socket/client_socket_pool_base.cc
+++ b/net/socket/client_socket_pool_base.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -6,7 +6,6 @@
 
 #include <math.h>
 #include "base/compiler_specific.h"
-#include "base/debug/alias.h"
 #include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
@@ -1144,22 +1143,15 @@ void ClientSocketPoolBaseHelper::InvokeUserCallback(
 
 ClientSocketPoolBaseHelper::Group::Group()
     : active_socket_count_(0),
-      ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)),
-      liveness_token_(TOKEN_ALIVE) {}
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {}
 
 ClientSocketPoolBaseHelper::Group::~Group() {
   CleanupBackupJob();
-
-  // Mark this object as destroyed.
-  liveness_token_ = TOKEN_DEAD;
-  destruction_callstack_ = base::debug::StackTrace();
 }
 
 void ClientSocketPoolBaseHelper::Group::StartBackupSocketTimer(
     const std::string& group_name,
     ClientSocketPoolBaseHelper* pool) {
-  CheckAlive();
-
   // Only allow one timer pending to create a backup socket.
   if (weak_factory_.HasWeakPtrs())
     return;
@@ -1172,8 +1164,6 @@ void ClientSocketPoolBaseHelper::Group::StartBackupSocketTimer(
 }
 
 bool ClientSocketPoolBaseHelper::Group::TryToUsePreconnectConnectJob() {
-  CheckAlive();
-
   for (std::set<ConnectJob*>::iterator it = jobs_.begin();
        it != jobs_.end(); ++it) {
     ConnectJob* job = *it;
@@ -1185,18 +1175,9 @@ bool ClientSocketPoolBaseHelper::Group::TryToUsePreconnectConnectJob() {
   return false;
 }
 
-void ClientSocketPoolBaseHelper::Group::AddJob(ConnectJob* job) {
-  CheckAlive();
-
-  jobs_.insert(job);
-}
-
-
 void ClientSocketPoolBaseHelper::Group::OnBackupSocketTimerFired(
     std::string group_name,
     ClientSocketPoolBaseHelper* pool) {
-  CheckAlive();
-
   // If there are no more jobs pending, there is no work to do.
   // If we've done our cleanups correctly, this should not happen.
   if (jobs_.empty()) {
@@ -1227,24 +1208,7 @@ void ClientSocketPoolBaseHelper::Group::OnBackupSocketTimerFired(
     pool->OnConnectJobComplete(rv, backup_job);
 }
 
-void ClientSocketPoolBaseHelper::Group::CheckAlive() {
-  if (liveness_token_ == TOKEN_ALIVE)
-    return;
-
-  // Copy all the interesting values onto the stack prior to crashing, in case
-  // the memory for |this| does not make it into the dump.
-  MagicToken token = liveness_token_;
-  base::debug::StackTrace callstack = destruction_callstack_;
-
-  base::debug::Alias(&token);
-  base::debug::Alias(&callstack);
-
-  CHECK_EQ(liveness_token_, TOKEN_ALIVE);
-}
-
 void ClientSocketPoolBaseHelper::Group::RemoveAllJobs() {
-  CheckAlive();
-
   // Delete active jobs.
   STLDeleteElements(&jobs_);
 
diff --git a/net/socket/client_socket_pool_base.h b/net/socket/client_socket_pool_base.h
index 869870d..0cca6f4a 100644
--- a/net/socket/client_socket_pool_base.h
+++ b/net/socket/client_socket_pool_base.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 //
@@ -31,7 +31,6 @@
 #include <vector>
 
 #include "base/basictypes.h"
-#include "base/debug/stack_trace.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
@@ -413,7 +412,7 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper
     // uses it.  Returns true on success.  Otherwise, returns false.
     bool TryToUsePreconnectConnectJob();
 
-    void AddJob(ConnectJob* job);
+    void AddJob(ConnectJob* job) { jobs_.insert(job); }
     void RemoveJob(ConnectJob* job) { jobs_.erase(job); }
     void RemoveAllJobs();
 
@@ -428,34 +427,17 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper
     std::list<IdleSocket>* mutable_idle_sockets() { return &idle_sockets_; }
 
    private:
-    enum MagicToken {
-      TOKEN_ALIVE = 0xCA11AB1E,
-      TOKEN_DEAD = 0xB100D1ED,
-    };
-
     // Called when the backup socket timer fires.
     void OnBackupSocketTimerFired(
         std::string group_name,
         ClientSocketPoolBaseHelper* pool);
 
-    // Crash if |this| is an invalid object (for instance if it was deleted).
-    // This code is being used to help track down http://crbug.com/109876.
-    void CheckAlive();
-
     std::list<IdleSocket> idle_sockets_;
     std::set<ConnectJob*> jobs_;
     RequestQueue pending_requests_;
     int active_socket_count_;  // number of active sockets used by clients
     // A factory to pin the backup_job tasks.
     base::WeakPtrFactory<Group> weak_factory_;
-
-    // Special value used to verify that |this| is not garbage memory.
-    MagicToken liveness_token_;
-
-    // This corresponds with the callstack where |this| was deleted from. If
-    // the object was not deleted, then this corresponds with the allocation
-    // callstack.
-    base::debug::StackTrace destruction_callstack_;
   };
 
   typedef std::map<std::string, Group*> GroupMap;