Add some instrumentation to catch the source of a potential double-free.

It saves the stack trace where initial deletion happened from. In the case where the deletion was initiated by DeleteSoon(), we will show the stacktrace for the caller of that function (rather than the actual deletion stacktrace from running the task).

BUG=94179

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

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

2 files changed, 81 insertions, 2 deletions

diff --git a/content/plugin/npobject_stub.cc b/content/plugin/npobject_stub.cc
index a191b81..1d51cb2 100644
--- a/content/plugin/npobject_stub.cc
+++ b/content/plugin/npobject_stub.cc
@@ -22,7 +22,9 @@ NPObjectStub::NPObjectStub(
     int route_id,
     gfx::NativeViewId containing_window,
     const GURL& page_url)
-    : npobject_(npobject),
+    : has_deletion_stack_trace_(false),
+      liveness_token_(kTokenAlive),
+      npobject_(npobject),
       channel_(channel),
       route_id_(route_id),
       containing_window_(containing_window),
@@ -35,21 +37,79 @@ NPObjectStub::NPObjectStub(
 }
 
 NPObjectStub::~NPObjectStub() {
+  // Crash if this is a double free!
+  CheckIsAlive();
+
   channel_->RemoveRoute(route_id_);
   CHECK(!npobject_);
+
+  // Mark the object as dead.
+  liveness_token_ = kTokenDead;
+
+  if (!has_deletion_stack_trace_) {
+    // We will probably have already set a more specific stack trace from
+    // DeleteSoonHelper. In case we got deleted from somewhere else, save the
+    // current thread's stack trace.
+    has_deletion_stack_trace_ = true;
+    deletion_stack_trace_ = base::debug::StackTrace();
+  }
+
+  // I doubt this is necessary to prevent optimization, but it can't hurt.
+  base::debug::Alias(&liveness_token_);
+  base::debug::Alias(&has_deletion_stack_trace_);
+  base::debug::Alias(&deletion_stack_trace_);
+}
+
+// static
+void NPObjectStub::DeleteSoonHelper(
+    const base::debug::StackTrace& task_origin_stack_trace,
+    NPObjectStub* stub) {
+  // Make sure the deletion stacktrace is going to be on the stack.
+  base::debug::StackTrace origin = task_origin_stack_trace;
+  base::debug::Alias(&origin);
+
+  stub->CheckIsAlive();
+
+  // Use the task origin's stacktrace as our deletion stacktrace
+  // (rather than the current thread's callstack).
+  stub->has_deletion_stack_trace_ = true;
+  stub->deletion_stack_trace_ = task_origin_stack_trace;
+
+  delete stub;
+}
+
+void NPObjectStub::CheckIsAlive() {
+  // Copy the deletion stacktrace onto stack in case we crash.
+  base::debug::StackTrace deletion_stack_trace = deletion_stack_trace_;
+  base::debug::Alias(&deletion_stack_trace);
+
+  // Copy the token onto stack in case it mismatches so we can explore its
+  // value.
+  int liveness_token = liveness_token_;
+  base::debug::Alias(&liveness_token);
+
+  CHECK_EQ(liveness_token, kTokenAlive);
 }
 
 void NPObjectStub::DeleteSoon(bool release_npobject) {
+  CheckIsAlive();
+
   if (npobject_) {
     channel_->RemoveMappingForNPObjectStub(route_id_, npobject_);
     if (release_npobject)
       WebBindings::releaseObject(npobject_);
     npobject_ = NULL;
-    MessageLoop::current()->DeleteSoon(FROM_HERE, this);
+    MessageLoop::current()->PostTask(
+      FROM_HERE,
+      NewRunnableFunction(
+          &NPObjectStub::DeleteSoonHelper,
+          base::debug::StackTrace(),
+          this));
   }
 }
 
 bool NPObjectStub::Send(IPC::Message* msg) {
+  CheckIsAlive();
   return channel_->Send(msg);
 }
 
diff --git a/content/plugin/npobject_stub.h b/content/plugin/npobject_stub.h
index 3584cfe..225c783 100644
--- a/content/plugin/npobject_stub.h
+++ b/content/plugin/npobject_stub.h
@@ -11,6 +11,7 @@
 
 #include <vector>
 
+#include "base/debug/stack_trace.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "content/plugin/npobject_base.h"
@@ -85,6 +86,24 @@ class NPObjectStub : public IPC::Channel::Listener,
                   IPC::Message* reply_msg);
 
  private:
+  //----------------------------------------------------------------------------
+  // Temporary code for debugging 94179
+  // TODO(eroman): Delete this when done investigating.
+  //----------------------------------------------------------------------------
+  void CheckIsAlive();
+  static void DeleteSoonHelper(
+      const base::debug::StackTrace& task_origin_stack_trace,
+      NPObjectStub* stub);
+
+  bool has_deletion_stack_trace_;
+  base::debug::StackTrace deletion_stack_trace_;
+
+  // Value to indicate whether this instance is alive or dead.
+  static const int kTokenAlive = 0x1Cd9fe38;
+  static const int kTokenDead = 0xDEADBEEF;
+  int liveness_token_;
+  //----------------------------------------------------------------------------
+
   NPObject* npobject_;
   scoped_refptr<PluginChannelBase> channel_;
   int route_id_;