Fix handle zapping interaction with NoHandleAllocation.

This makes sure that closed handle scopes are properly zapped even if an
enclosing NoHandleAllocation shrunk the limit. It also unifies the code
that performs scope closing for internal and external handle scopes.

R=svenpanne@chromium.org
TEST=cctest/test-api/NestedLockersNoTryCatch

Review URL: https://codereview.chromium.org/16004006

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@14876 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/api.cc

@@ -679,19 +679,7 @@ HandleScope::~HandleScope() {
 
 
 void HandleScope::Leave() {
-  v8::ImplementationUtilities::HandleScopeData* current =
+  return i::HandleScope::CloseScope(isolate_, prev_next_, prev_limit_);
-      isolate_->handle_scope_data();
-  current->level--;
-  ASSERT(current->level >= 0);
-  current->next = prev_next_;
-  if (current->limit != prev_limit_) {
-    current->limit = prev_limit_;
-    i::HandleScope::DeleteExtensions(isolate_);
-  }
-
-#ifdef ENABLE_EXTRA_CHECKS
-  i::HandleScope::ZapRange(prev_next_, prev_limit_);
-#endif
 }
 
 

src/api.h

@@ -637,7 +637,12 @@ void HandleScopeImplementer::DeleteExtensions(internal::Object** prev_limit) {
     internal::Object** block_limit = block_start + kHandleBlockSize;
 #ifdef DEBUG
     // NoHandleAllocation may make the prev_limit to point inside the block.
-    if (block_start <= prev_limit && prev_limit <= block_limit) break;
+    if (block_start <= prev_limit && prev_limit <= block_limit) {
+#ifdef ENABLE_EXTRA_CHECKS
+      internal::HandleScope::ZapRange(prev_limit, block_limit);
+#endif
+      break;
+    }
 #else
     if (prev_limit == block_limit) break;
 #endif

src/handles-inl.h

@@ -122,31 +122,37 @@ HandleScope::HandleScope(Isolate* isolate) {
 
 
 HandleScope::~HandleScope() {
-  CloseScope();
+  CloseScope(isolate_, prev_next_, prev_limit_);
 }
 
-void HandleScope::CloseScope() {
+
+void HandleScope::CloseScope(Isolate* isolate,
+                             Object** prev_next,
+                             Object** prev_limit) {
   v8::ImplementationUtilities::HandleScopeData* current =
-      isolate_->handle_scope_data();
+      isolate->handle_scope_data();
-  current->next = prev_next_;
+
+  current->next = prev_next;
   current->level--;
-  if (current->limit != prev_limit_) {
+  if (current->limit != prev_limit) {
-    current->limit = prev_limit_;
+    current->limit = prev_limit;
-    DeleteExtensions(isolate_);
+    DeleteExtensions(isolate);
   }
+
 #ifdef ENABLE_EXTRA_CHECKS
-  ZapRange(prev_next_, prev_limit_);
+  ZapRange(prev_next, prev_limit);
 #endif
 }
 
 
 template <typename T>
 Handle<T> HandleScope::CloseAndEscape(Handle<T> handle_value) {
-  T* value = *handle_value;
-  // Throw away all handles in the current scope.
-  CloseScope();
   v8::ImplementationUtilities::HandleScopeData* current =
       isolate_->handle_scope_data();
+
+  T* value = *handle_value;
+  // Throw away all handles in the current scope.
+  CloseScope(isolate_, prev_next_, prev_limit_);
   // Allocate one handle in the parent scope.
   ASSERT(current->level > 0);
   Handle<T> result(CreateHandle<T>(isolate_, value));
@@ -180,15 +186,14 @@ T** HandleScope::CreateHandle(Isolate* isolate, T* value) {
 #ifdef DEBUG
 inline NoHandleAllocation::NoHandleAllocation(Isolate* isolate)
     : isolate_(isolate) {
-  v8::ImplementationUtilities::HandleScopeData* current =
-      isolate_->handle_scope_data();
-
   active_ = !isolate->optimizing_compiler_thread()->IsOptimizerThread();
   if (active_) {
     // Shrink the current handle scope to make it impossible to do
     // handle allocations without an explicit handle scope.
+    v8::ImplementationUtilities::HandleScopeData* current =
+        isolate_->handle_scope_data();
+    limit_ = current->limit;
     current->limit = current->next;
-
     level_ = current->level;
     current->level = 0;
   }
@@ -199,10 +204,12 @@ inline NoHandleAllocation::~NoHandleAllocation() {
   if (active_) {
     // Restore state in current handle scope to re-enable handle
     // allocations.
-    v8::ImplementationUtilities::HandleScopeData* data =
+    v8::ImplementationUtilities::HandleScopeData* current =
         isolate_->handle_scope_data();
-    ASSERT_EQ(0, data->level);
+    ASSERT_EQ(0, current->level);
-    data->level = level_;
+    current->level = level_;
+    ASSERT_EQ(current->next, current->limit);
+    current->limit = limit_;
   }
 }
 

src/handles.h

@@ -155,18 +155,21 @@ class HandleScope {
   void* operator new(size_t size);
   void operator delete(void* size_t);
 
-  inline void CloseScope();
-
   Isolate* isolate_;
   Object** prev_next_;
   Object** prev_limit_;
 
+  // Close the handle scope resetting limits to a previous state.
+  static inline void CloseScope(Isolate* isolate,
+                                Object** prev_next,
+                                Object** prev_limit);
+
   // Extend the handle scope making room for more handles.
   static internal::Object** Extend(Isolate* isolate);
 
 #ifdef ENABLE_EXTRA_CHECKS
   // Zaps the handles in the half-open interval [start, end).
-  static void ZapRange(internal::Object** start, internal::Object** end);
+  static void ZapRange(Object** start, Object** end);
 #endif
 
   friend class v8::HandleScope;
@@ -337,6 +340,7 @@ class NoHandleAllocation BASE_EMBEDDED {
   inline ~NoHandleAllocation();
  private:
   Isolate* isolate_;
+  Object** limit_;
   int level_;
   bool active_;
 #endif