From 080628d32fe62592df0050b259e7b369cb4fdc52 Mon Sep 17 00:00:00 2001
From: "mikhail.naganov@gmail.com"
 <mikhail.naganov@gmail.com@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Thu, 8 Sep 2011 13:51:06 +0000
Subject: [PATCH] Fix memory leak from d8 shell.

We were not disposing the semaphores and the thread used in SourceGroup.

R=mnaganov@chromium.org

Signed-off-by: Thiago Farina <tfarina@chromium.org>

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@9198 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
---
 src/d8.cc           | 11 +++++------
 src/d8.h            | 13 ++++++-------
 src/smart-pointer.h | 43 +++++++++++++++++--------------------------
 3 files changed, 28 insertions(+), 39 deletions(-)

diff --git a/src/d8.cc b/src/d8.cc
index 5a16292..93b383d 100644
--- a/src/d8.cc
+++ b/src/d8.cc
@@ -1037,7 +1037,7 @@ i::Thread::Options SourceGroup::GetThreadOptions() {
 void SourceGroup::ExecuteInThread() {
   Isolate* isolate = Isolate::New();
   do {
-    if (next_semaphore_ != NULL) next_semaphore_->Wait();
+    if (!next_semaphore_.is_empty()) next_semaphore_->Wait();
     {
       Isolate::Scope iscope(isolate);
       Locker lock(isolate);
@@ -1049,15 +1049,15 @@ void SourceGroup::ExecuteInThread() {
       }
       context.Dispose();
     }
-    if (done_semaphore_ != NULL) done_semaphore_->Signal();
+    if (!done_semaphore_.is_empty()) done_semaphore_->Signal();
   } while (!Shell::options.last_run);
   isolate->Dispose();
 }
 
 
 void SourceGroup::StartExecuteInThread() {
-  if (thread_ == NULL) {
-    thread_ = new IsolateThread(this);
+  if (thread_.is_empty()) {
+    thread_ = i::SmartPointer<i::Thread>(new IsolateThread(this));
     thread_->Start();
   }
   next_semaphore_->Signal();
@@ -1065,10 +1065,9 @@ void SourceGroup::StartExecuteInThread() {
 
 
 void SourceGroup::WaitForThread() {
-  if (thread_ == NULL) return;
+  if (thread_.is_empty()) return;
   if (Shell::options.last_run) {
     thread_->Join();
-    thread_ = NULL;
   } else {
     done_semaphore_->Wait();
   }
diff --git a/src/d8.h b/src/d8.h
index d0267a7..3ec0390 100644
--- a/src/d8.h
+++ b/src/d8.h
@@ -28,11 +28,11 @@
 #ifndef V8_D8_H_
 #define V8_D8_H_
 
-
 #ifndef V8_SHARED
-#include "v8.h"
 #include "allocation.h"
 #include "hashmap.h"
+#include "smart-pointer.h"
+#include "v8.h"
 #else
 #include "../include/v8.h"
 #endif  // V8_SHARED
@@ -122,11 +122,10 @@ class SourceGroup {
 #ifndef V8_SHARED
       next_semaphore_(v8::internal::OS::CreateSemaphore(0)),
       done_semaphore_(v8::internal::OS::CreateSemaphore(0)),
-      thread_(NULL),
 #endif  // V8_SHARED
       argv_(NULL),
       begin_offset_(0),
-      end_offset_(0) { }
+      end_offset_(0) {}
 
   void Begin(char** argv, int offset) {
     argv_ = const_cast<const char**>(argv);
@@ -158,9 +157,9 @@ class SourceGroup {
   static i::Thread::Options GetThreadOptions();
   void ExecuteInThread();
 
-  i::Semaphore* next_semaphore_;
-  i::Semaphore* done_semaphore_;
-  i::Thread* thread_;
+  i::SmartPointer<i::Semaphore> next_semaphore_;
+  i::SmartPointer<i::Semaphore> done_semaphore_;
+  i::SmartPointer<i::Thread> thread_;
 #endif  // V8_SHARED
 
   void ExitShell(int exit_code);
diff --git a/src/smart-pointer.h b/src/smart-pointer.h
index 0fa8224..a3e930d 100644
--- a/src/smart-pointer.h
+++ b/src/smart-pointer.h
@@ -37,35 +37,31 @@ namespace internal {
 template<typename T>
 class SmartPointer {
  public:
+  // Default constructor. Constructs an empty scoped pointer.
+  inline SmartPointer() : p_(NULL) {}
 
-  // Default constructor. Construct an empty scoped pointer.
-  inline SmartPointer() : p(NULL) {}
-
-
-  // Construct a scoped pointer from a plain one.
-  explicit inline SmartPointer(T* pointer) : p(pointer) {}
-
+  // Constructs a scoped pointer from a plain one.
+  explicit inline SmartPointer(T* ptr) : p_(ptr) {}
 
   // Copy constructor removes the pointer from the original to avoid double
   // freeing.
-  inline SmartPointer(const SmartPointer<T>& rhs) : p(rhs.p) {
-    const_cast<SmartPointer<T>&>(rhs).p = NULL;
+  inline SmartPointer(const SmartPointer<T>& rhs) : p_(rhs.p_) {
+    const_cast<SmartPointer<T>&>(rhs).p_ = NULL;
   }
 
-
   // When the destructor of the scoped pointer is executed the plain pointer
   // is deleted using DeleteArray.  This implies that you must allocate with
   // NewArray.
-  inline ~SmartPointer() { if (p) DeleteArray(p); }
+  inline ~SmartPointer() { if (p_) DeleteArray(p_); }
 
+  inline T* operator->() const { return p_; }
 
   // You can get the underlying pointer out with the * operator.
-  inline T* operator*() { return p; }
-
+  inline T* operator*() { return p_; }
 
   // You can use [n] to index as if it was a plain pointer
   inline T& operator[](size_t i) {
-    return p[i];
+    return p_[i];
   }
 
   // We don't have implicit conversion to a T* since that hinders migration:
@@ -77,31 +73,26 @@ class SmartPointer {
   // deleted then call Detach().  Afterwards, the smart pointer is empty
   // (NULL).
   inline T* Detach() {
-    T* temp = p;
-    p = NULL;
+    T* temp = p_;
+    p_ = NULL;
     return temp;
   }
 
-
   // Assignment requires an empty (NULL) SmartPointer as the receiver.  Like
   // the copy constructor it removes the pointer in the original to avoid
   // double freeing.
   inline SmartPointer& operator=(const SmartPointer<T>& rhs) {
     ASSERT(is_empty());
-    T* tmp = rhs.p;  // swap to handle self-assignment
-    const_cast<SmartPointer<T>&>(rhs).p = NULL;
-    p = tmp;
+    T* tmp = rhs.p_;  // swap to handle self-assignment
+    const_cast<SmartPointer<T>&>(rhs).p_ = NULL;
+    p_ = tmp;
     return *this;
   }
 
-
-  inline bool is_empty() {
-    return p == NULL;
-  }
-
+  inline bool is_empty() { return p_ == NULL; }
 
  private:
-  T* p;
+  T* p_;
 };
 
 } }  // namespace v8::internal
-- 
2.7.4