From 96b939a7b319b4e925ed847cb5cbbcc921872f41 Mon Sep 17 00:00:00 2001 From: "eroman@chromium.org" Date: Fri, 20 Jan 2012 21:52:15 +0000 Subject: 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 --- net/socket/client_socket_pool_base.cc | 40 ++--------------------------------- net/socket/client_socket_pool_base.h | 22 ++----------------- 2 files changed, 4 insertions(+), 58 deletions(-) (limited to 'net/socket') 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 #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::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 #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* 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 idle_sockets_; std::set 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 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 GroupMap; -- cgit v1.1