From a6f7fd9c874ffdab31c3c79c9956857617b723d5 Mon Sep 17 00:00:00 2001
From: Jon Siwek <jsiwek@ncsa.illinois.edu>
Date: Thu, 16 Aug 2012 15:59:26 -0500
Subject: [PATCH] Fix memory leak of serialized IDs when compiled with
 --enable-debug.

When using --enable-debug, values keep track of the last identifier
to which they were bound by storing a ref'd ID pointer.  This could
lead to some circular dependencies in which an ID is never reclaimed
because the Val is bound to the ID and the ID is bound to the Val, with
both holding references to each other.

There might be more cases where this feature of --enable-debug caused
a leak, but it showed up in particular when running the
core.leaks.remote unit test due to the internal
SendID("peer_description") call during the handshake between remote
processes.  Other tests showed the send_id() BIF leaked more generally.

Tracking the ID last bound to a Val through just the identifier string
instead of a ref'd ID pointer fixes the leak.
---
 src/RemoteSerializer.cc |  5 -----
 src/Val.cc              |  2 +-
 src/Val.h               | 16 +++++++++-------
 3 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/src/RemoteSerializer.cc b/src/RemoteSerializer.cc
index cfd20eba39..564ad2be68 100644
--- a/src/RemoteSerializer.cc
+++ b/src/RemoteSerializer.cc
@@ -2897,11 +2897,6 @@ void RemoteSerializer::GotID(ID* id, Val* val)
 					(desc && *desc) ? desc : "not set"),
 			current_peer);
 
-#ifdef USE_PERFTOOLS_DEBUG
-		// May still be cached, but we don't care.
-		heap_checker->IgnoreObject(id);
-#endif
-
 		Unref(id);
 		return;
 		}
diff --git a/src/Val.cc b/src/Val.cc
index 8a8c2b18c0..79fa8a0c69 100644
--- a/src/Val.cc
+++ b/src/Val.cc
@@ -64,7 +64,7 @@ Val::~Val()
 
 	Unref(type);
 #ifdef DEBUG
-	Unref(bound_id);
+	delete [] bound_id;
 #endif
 	}
 
diff --git a/src/Val.h b/src/Val.h
index 2ca18e6131..c3ec5b04fb 100644
--- a/src/Val.h
+++ b/src/Val.h
@@ -347,13 +347,15 @@ public:
 #ifdef DEBUG
 	// For debugging, we keep a reference to the global ID to which a
 	// value has been bound *last*.
-	ID* GetID() const	{ return bound_id; }
+	ID* GetID() const
+		{
+		return bound_id ? global_scope()->Lookup(bound_id) : 0;
+		}
+
 	void SetID(ID* id)
 		{
-		if ( bound_id )
-			::Unref(bound_id);
-		bound_id = id;
-		::Ref(bound_id);
+		delete [] bound_id;
+		bound_id = id ? copy_string(id->Name()) : 0;
 		}
 #endif
 
@@ -401,8 +403,8 @@ protected:
 	RecordVal* attribs;
 
 #ifdef DEBUG
-	// For debugging, we keep the ID to which a Val is bound.
-	ID* bound_id;
+	// For debugging, we keep the name of the ID to which a Val is bound.
+	const char* bound_id;
 #endif
 
 };