Revision: 9198
Author: [email protected]
Date: Thu Sep 8 06:51:06 2011
Log: Fix memory leak from d8 shell.
We were not disposing the semaphores and the thread used in SourceGroup.
[email protected]
Signed-off-by: Thiago Farina <[email protected]>
Review URL: http://codereview.chromium.org/7754007/
http://code.google.com/p/v8/source/detail?r=9198
Modified:
/branches/bleeding_edge/src/d8.cc
/branches/bleeding_edge/src/d8.h
/branches/bleeding_edge/src/smart-pointer.h
=======================================
--- /branches/bleeding_edge/src/d8.cc Thu Sep 8 02:49:52 2011
+++ /branches/bleeding_edge/src/d8.cc Thu Sep 8 06:51:06 2011
@@ -1037,7 +1037,7 @@
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 @@
}
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::WaitForThread() {
- if (thread_ == NULL) return;
+ if (thread_.is_empty()) return;
if (Shell::options.last_run) {
thread_->Join();
- thread_ = NULL;
} else {
done_semaphore_->Wait();
}
=======================================
--- /branches/bleeding_edge/src/d8.h Thu Sep 8 01:54:28 2011
+++ /branches/bleeding_edge/src/d8.h Thu Sep 8 06:51:06 2011
@@ -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 @@
#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 @@
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);
=======================================
--- /branches/bleeding_edge/src/smart-pointer.h Tue Dec 7 03:01:02 2010
+++ /branches/bleeding_edge/src/smart-pointer.h Thu Sep 8 06:51:06 2011
@@ -37,35 +37,31 @@
template<typename T>
class SmartPointer {
public:
-
- // 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) {}
-
+ // Default constructor. Constructs an empty scoped pointer.
+ inline SmartPointer() : p_(NULL) {}
+
+ // 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 @@
// 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
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
