From 9f9959182d44783ef7f3227328d7ccf3943ea322 Mon Sep 17 00:00:00 2001 From: "jkummerow@chromium.org" Date: Wed, 17 Aug 2011 08:48:54 +0000 Subject: [PATCH] Fix memory leaks in ~Zone and ~Isolate TEST=chromium valgrind bots Review URL: http://codereview.chromium.org/7660016 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@8949 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/isolate.cc | 6 +++++ src/serialize.cc | 58 +++++++----------------------------------------- src/serialize.h | 46 ++++++++++++++++++++++++++++++++++++++ src/zone.cc | 53 +++++++++++++++++++++++++------------------ src/zone.h | 6 ++++- 5 files changed, 96 insertions(+), 73 deletions(-) diff --git a/src/isolate.cc b/src/isolate.cc index 9ca61177ba..eae812bcd9 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -1533,6 +1533,9 @@ void Isolate::SetIsolateThreadLocals(Isolate* isolate, Isolate::~Isolate() { TRACE_ISOLATE(destructor); + // Has to be called while counters_ are still alive. + zone_.DeleteKeptSegment(); + delete unicode_cache_; unicode_cache_ = NULL; @@ -1591,6 +1594,9 @@ Isolate::~Isolate() { delete global_handles_; global_handles_ = NULL; + delete external_reference_table_; + external_reference_table_ = NULL; + #ifdef ENABLE_DEBUGGER_SUPPORT delete debugger_; debugger_ = NULL; diff --git a/src/serialize.cc b/src/serialize.cc index 8cde580fbb..094ad20b22 100644 --- a/src/serialize.cc +++ b/src/serialize.cc @@ -62,57 +62,15 @@ static int* GetInternalPointer(StatsCounter* counter) { } -// ExternalReferenceTable is a helper class that defines the relationship -// between external references and their encodings. It is used to build -// hashmaps in ExternalReferenceEncoder and ExternalReferenceDecoder. -class ExternalReferenceTable { - public: - static ExternalReferenceTable* instance(Isolate* isolate) { - ExternalReferenceTable* external_reference_table = - isolate->external_reference_table(); - if (external_reference_table == NULL) { - external_reference_table = new ExternalReferenceTable(isolate); - isolate->set_external_reference_table(external_reference_table); - } - return external_reference_table; +ExternalReferenceTable* ExternalReferenceTable::instance(Isolate* isolate) { + ExternalReferenceTable* external_reference_table = + isolate->external_reference_table(); + if (external_reference_table == NULL) { + external_reference_table = new ExternalReferenceTable(isolate); + isolate->set_external_reference_table(external_reference_table); } - - int size() const { return refs_.length(); } - - Address address(int i) { return refs_[i].address; } - - uint32_t code(int i) { return refs_[i].code; } - - const char* name(int i) { return refs_[i].name; } - - int max_id(int code) { return max_id_[code]; } - - private: - explicit ExternalReferenceTable(Isolate* isolate) : refs_(64) { - PopulateTable(isolate); - } - ~ExternalReferenceTable() { } - - struct ExternalReferenceEntry { - Address address; - uint32_t code; - const char* name; - }; - - void PopulateTable(Isolate* isolate); - - // For a few types of references, we can get their address from their id. - void AddFromId(TypeCode type, - uint16_t id, - const char* name, - Isolate* isolate); - - // For other types of references, the caller will figure out the address. - void Add(Address address, TypeCode type, uint16_t id, const char* name); - - List refs_; - int max_id_[kTypeCodeCount]; -}; + return external_reference_table; +} void ExternalReferenceTable::AddFromId(TypeCode type, diff --git a/src/serialize.h b/src/serialize.h index f8282f6529..66d6fb5111 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -60,6 +60,52 @@ const int kDebugRegisterBits = 4; const int kDebugIdShift = kDebugRegisterBits; +// ExternalReferenceTable is a helper class that defines the relationship +// between external references and their encodings. It is used to build +// hashmaps in ExternalReferenceEncoder and ExternalReferenceDecoder. +class ExternalReferenceTable { + public: + static ExternalReferenceTable* instance(Isolate* isolate); + + ~ExternalReferenceTable() { } + + int size() const { return refs_.length(); } + + Address address(int i) { return refs_[i].address; } + + uint32_t code(int i) { return refs_[i].code; } + + const char* name(int i) { return refs_[i].name; } + + int max_id(int code) { return max_id_[code]; } + + private: + explicit ExternalReferenceTable(Isolate* isolate) : refs_(64) { + PopulateTable(isolate); + } + + struct ExternalReferenceEntry { + Address address; + uint32_t code; + const char* name; + }; + + void PopulateTable(Isolate* isolate); + + // For a few types of references, we can get their address from their id. + void AddFromId(TypeCode type, + uint16_t id, + const char* name, + Isolate* isolate); + + // For other types of references, the caller will figure out the address. + void Add(Address address, TypeCode type, uint16_t id, const char* name); + + List refs_; + int max_id_[kTypeCodeCount]; +}; + + class ExternalReferenceEncoder { public: ExternalReferenceEncoder(); diff --git a/src/zone.cc b/src/zone.cc index 42ce8c5cb7..7574778f53 100644 --- a/src/zone.cc +++ b/src/zone.cc @@ -34,24 +34,6 @@ namespace v8 { namespace internal { -Zone::Zone() - : zone_excess_limit_(256 * MB), - segment_bytes_allocated_(0), - position_(0), - limit_(0), - scope_nesting_(0), - segment_head_(NULL) { -} -unsigned Zone::allocation_size_ = 0; - - -ZoneScope::~ZoneScope() { - ASSERT_EQ(Isolate::Current(), isolate_); - if (ShouldDeleteOnExit()) isolate_->zone()->DeleteAll(); - isolate_->zone()->scope_nesting_--; -} - - // Segments represent chunks of memory: They have starting address // (encoded in the this pointer) and a size in bytes. Segments are // chained together forming a LIFO structure with the newest segment @@ -60,6 +42,11 @@ ZoneScope::~ZoneScope() { class Segment { public: + void Initialize(Segment* next, int size) { + next_ = next; + size_ = size; + } + Segment* next() const { return next_; } void clear_next() { next_ = NULL; } @@ -77,19 +64,33 @@ class Segment { Segment* next_; int size_; - - friend class Zone; }; +Zone::Zone() + : zone_excess_limit_(256 * MB), + segment_bytes_allocated_(0), + position_(0), + limit_(0), + scope_nesting_(0), + segment_head_(NULL) { +} +unsigned Zone::allocation_size_ = 0; + +ZoneScope::~ZoneScope() { + ASSERT_EQ(Isolate::Current(), isolate_); + if (ShouldDeleteOnExit()) isolate_->zone()->DeleteAll(); + isolate_->zone()->scope_nesting_--; +} + + // Creates a new segment, sets it size, and pushes it to the front // of the segment chain. Returns the new segment. Segment* Zone::NewSegment(int size) { Segment* result = reinterpret_cast(Malloced::New(size)); adjust_segment_bytes_allocated(size); if (result != NULL) { - result->next_ = segment_head_; - result->size_ = size; + result->Initialize(segment_head_, size); segment_head_ = result; } return result; @@ -155,6 +156,14 @@ void Zone::DeleteAll() { } +void Zone::DeleteKeptSegment() { + if (segment_head_ != NULL) { + DeleteSegment(segment_head_, segment_head_->size()); + segment_head_ = NULL; + } +} + + Address Zone::NewExpand(int size) { // Make sure the requested size is already properly aligned and that // there isn't enough room in the Zone to satisfy the request. diff --git a/src/zone.h b/src/zone.h index abb53ad46d..faad3b7ccd 100644 --- a/src/zone.h +++ b/src/zone.h @@ -65,9 +65,13 @@ class Zone { template inline T* NewArray(int length); - // Delete all objects and free all memory allocated in the Zone. + // Deletes all objects and free all memory allocated in the Zone. Keeps one + // small (size <= kMaximumKeptSegmentSize) segment around if it finds one. void DeleteAll(); + // Deletes the last small segment kept around by DeleteAll(). + void DeleteKeptSegment(); + // Returns true if more memory has been allocated in zones than // the limit allows. inline bool excess_allocation();