Changed StackZone and ApiZone to be containers for Zone.

runtime/vm/dart_api_impl.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "include/dart_api.h"
 
 #include "vm/bigint_operations.h"
 #include "vm/class_finalizer.h"
 #include "vm/compiler.h"
 #include "vm/dart.h"
@@ skipping 182 matching lines @@
 
 Dart_Handle Api::NewError(const char* format, ...) {
   Isolate* isolate = Isolate::Current();
   DARTSCOPE_NOCHECKS(isolate);
 
   va_list args;
   va_start(args, format);
   intptr_t len = OS::VSNPrint(NULL, 0, format, args);
   va_end(args);
 
-  char* buffer = zone.Alloc<char>(len + 1);
+  char* buffer = isolate->current_zone()->Alloc<char>(len + 1);
   va_list args2;
   va_start(args2, format);
   OS::VSNPrint(buffer, (len + 1), format, args2);
   va_end(args2);
 
   const String& message = String::Handle(isolate, String::New(buffer));
   return Api::NewHandle(isolate, ApiError::New(message));
 }
 
 
@@ skipping 189 matching lines @@
 // TODO(turnidge): Remove all uses and delete.
 DART_EXPORT Dart_Handle Dart_Error(const char* format, ...) {
   Isolate* isolate = Isolate::Current();
   DARTSCOPE(isolate);
 
   va_list args;
   va_start(args, format);
   intptr_t len = OS::VSNPrint(NULL, 0, format, args);
   va_end(args);
 
-  char* buffer = zone.Alloc<char>(len + 1);
+  char* buffer = isolate->current_zone()->Alloc<char>(len + 1);
   va_list args2;
   va_start(args2, format);
   OS::VSNPrint(buffer, (len + 1), format, args2);
   va_end(args2);
 
   const String& message = String::Handle(isolate, String::New(buffer));
   return Api::NewHandle(isolate, ApiError::New(message));
 }
 
 
 // TODO(turnidge): This clones Api::NewError.  I need to use va_copy to
 // fix this but not sure if it available on all of our builds.
 DART_EXPORT Dart_Handle Dart_NewApiError(const char* format, ...) {
   Isolate* isolate = Isolate::Current();
   DARTSCOPE(isolate);
 
   va_list args;
   va_start(args, format);
   intptr_t len = OS::VSNPrint(NULL, 0, format, args);
   va_end(args);
 
-  char* buffer = zone.Alloc<char>(len + 1);
+  char* buffer = isolate->current_zone()->Alloc<char>(len + 1);
   va_list args2;
   va_start(args2, format);
   OS::VSNPrint(buffer, (len + 1), format, args2);
   va_end(args2);
 
   const String& message = String::Handle(isolate, String::New(buffer));
   return Api::NewHandle(isolate, ApiError::New(message));
 }
 
 
 DART_EXPORT Dart_Handle Dart_NewUnhandledExceptionError(Dart_Handle exception) {
   Isolate* isolate = Isolate::Current();
   DARTSCOPE(isolate);
   const Instance& obj = Api::UnwrapInstanceHandle(isolate, exception);
   if (obj.IsNull()) {
     RETURN_TYPE_ERROR(isolate, exception, Instance);
   }
   const Instance& stacktrace = Instance::Handle(isolate);
   return Api::NewHandle(isolate, UnhandledException::New(obj, stacktrace));
 }
 
 
+// Unwind all the API scopes till the exit frame, and reallocate handle.
+DART_EXPORT Dart_Handle UnwindApiScopes(Isolate* isolate, Dart_Handle handle) {

[siva, 2012/10/15 17:50:35]

Why not make this a static method?

[Ivan Posva, 2012/10/15 18:26:01]

There should be no need to return a Dart_Handle here. It leads to the problems
Siva describes.

[Tom Ball, 2012/10/15 19:51:06]

Done.

[Tom Ball, 2012/10/15 19:51:06]

Inlined function.

+  NoGCScope no_gc;
+  ApiState* state = isolate->api_state();
+  ASSERT(state != NULL);
+  RawObject* raw_obj = Api::UnwrapHandle(handle);
+  state->UnwindScopes(isolate->top_exit_frame_info());
+  return Api::NewHandle(isolate, raw_obj);

[siva, 2012/10/15 17:50:35]

These new handles are created in the outer API scope so one could potentially
write a dart loop which calls a native method that throws an exception and ends
up with a bunch of handles in the outer API scope and run out of memory.

[Tom Ball, 2012/10/15 19:51:06]

Removed function, as Ivan described.

+}
+
+
 DART_EXPORT Dart_Handle Dart_PropagateError(Dart_Handle handle) {
   Isolate* isolate = Isolate::Current();
-  CHECK_ISOLATE(isolate);
-  const Object& obj = Object::Handle(isolate, Api::UnwrapHandle(handle));
-  if (!obj.IsError()) {
-    return Api::NewError(
-        "%s expects argument 'handle' to be an error handle.  "
-        "Did you forget to check Dart_IsError first?",
-        CURRENT_FUNC);
+  {
+    const Object& obj = Object::Handle(isolate, Api::UnwrapHandle(handle));
+    if (!obj.IsError()) {
+      return Api::NewError(
+          "%s expects argument 'handle' to be an error handle.  "
+          "Did you forget to check Dart_IsError first?",
+          CURRENT_FUNC);
+    }
   }
   if (isolate->top_exit_frame_info() == 0) {
     // There are no dart frames on the stack so it would be illegal to
     // propagate an error here.
     return Api::NewError("No Dart frames on stack, cannot propagate error.");
   }
 
   // Unwind all the API scopes till the exit frame before propagating.
-  ApiState* state = isolate->api_state();
-  ASSERT(state != NULL);
-  state->UnwindScopes(isolate->top_exit_frame_info());
+  handle = UnwindApiScopes(isolate, handle);

[Ivan Posva, 2012/10/15 18:26:01]

This avoids Siva's concern:

  ASSERT(state != NULL);
  Object* error;
  {
    NoGCScope no_gc;
    RawObject* raw_obj = Api::UnwrapHandle(handle);
    state->UnwindScopes(isolate->top_exit_frame_info());
    error = &Object::Handle(raw_obj);
  }
  Exceptions::PropagateError(Error::Cast(error));

[Tom Ball, 2012/10/15 19:51:06]

Integrated recommended change.

+  const Object& obj = Object::Handle(isolate, Api::UnwrapHandle(handle));
   Exceptions::PropagateError(Error::Cast(obj));
   UNREACHABLE();
-
   return Api::NewError("Cannot reach here.  Internal error.");
 }
 
 
 DART_EXPORT void _Dart_ReportErrorHandle(const char* file,
                                          int line,
                                          const char* handle,
                                          const char* message) {
   fprintf(stderr, "%s:%d: error handle: '%s':\n    '%s'\n",
           file, line, handle, message);
@@ skipping 644 matching lines @@
   CHECK_ISOLATE_SCOPE(isolate);
   ApiState* state = isolate->api_state();
   ApiLocalScope* scope = state->top_scope();
 
   state->set_top_scope(scope->previous());  // Reset top scope to previous.
   delete scope;  // Free up the old scope which we have just exited.
 }
 
 
 DART_EXPORT uint8_t* Dart_ScopeAllocate(intptr_t size) {
-  ApiZone* zone;
+  Zone* zone;
   Isolate* isolate = Isolate::Current();
   if (isolate != NULL) {
     ApiState* state = isolate->api_state();
     if (state == NULL) return NULL;
     ApiLocalScope* scope = state->top_scope();
     zone = scope->zone();
   } else {
     ApiNativeScope* scope = ApiNativeScope::Current();
     if (scope == NULL) return NULL;
     zone = scope->zone();
@@ skipping 2733 matching lines @@
   instance.SetNativeField(index, value);
   return Api::Success(isolate);
 }
 
 
 // --- Exceptions ----
 
 
 DART_EXPORT Dart_Handle Dart_ThrowException(Dart_Handle exception) {
   Isolate* isolate = Isolate::Current();
-  DARTSCOPE(isolate);
-  const Instance& excp = Api::UnwrapInstanceHandle(isolate, exception);
-  if (excp.IsNull()) {
-    RETURN_TYPE_ERROR(isolate, exception, Instance);
+  CHECK_ISOLATE(isolate);
+  {
+    const Instance& excp = Api::UnwrapInstanceHandle(isolate, exception);
+    if (excp.IsNull()) {
+      RETURN_TYPE_ERROR(isolate, exception, Instance);
+    }
   }
   if (isolate->top_exit_frame_info() == 0) {
     // There are no dart frames on the stack so it would be illegal to
     // throw an exception here.
     return Api::NewError("No Dart frames on stack, cannot throw exception");
   }
   // Unwind all the API scopes till the exit frame before throwing an
   // exception.
   ApiState* state = isolate->api_state();
   ASSERT(state != NULL);
-  state->UnwindScopes(isolate->top_exit_frame_info());
+
+  // Unwind all the API scopes till the exit frame before throwing an
+  // exception.
+  exception = UnwindApiScopes(isolate, exception);
+  const Instance& excp = Api::UnwrapInstanceHandle(isolate, exception);
   Exceptions::Throw(excp);
   return Api::NewError("Exception was not thrown, internal error");
 }
 
 
+// Unwind all the API scopes till the exit frame before rethrowing an exception.
+DART_EXPORT void UnwindApiScopesForRethrow(Isolate* isolate,
+                                           Dart_Handle* exception,
+                                           Dart_Handle* stacktrace) {

[siva, 2012/10/15 17:50:35]

Ditto, why not make this a static method.

[Tom Ball, 2012/10/15 19:51:06]

Inlined code and removed function.

+  NoGCScope no_gc;
+  ApiState* state = isolate->api_state();
+  ASSERT(state != NULL);
+  RawObject* raw_excp = Api::UnwrapHandle(*exception);
+  RawObject* raw_stk = Api::UnwrapHandle(*stacktrace);
+  state->UnwindScopes(isolate->top_exit_frame_info());
+  *exception = Api::NewHandle(isolate, raw_excp);

[Ivan Posva, 2012/10/15 18:26:01]

No Dart_Handles as they can leak out.

[Tom Ball, 2012/10/15 19:51:06]

Removed, followed pattern you recommended for PropagateError in
Dart_ReThrowException below.

+  *stacktrace = Api::NewHandle(isolate, raw_stk);
+}
+
+
 DART_EXPORT Dart_Handle Dart_ReThrowException(Dart_Handle exception,
                                               Dart_Handle stacktrace) {
   Isolate* isolate = Isolate::Current();
   CHECK_ISOLATE(isolate);
-  DARTSCOPE(isolate);
-  const Instance& excp = Api::UnwrapInstanceHandle(isolate, exception);
-  if (excp.IsNull()) {
-    RETURN_TYPE_ERROR(isolate, exception, Instance);
-  }
-  const Instance& stk = Api::UnwrapInstanceHandle(isolate, stacktrace);
-  if (stk.IsNull()) {
-    RETURN_TYPE_ERROR(isolate, stacktrace, Instance);
+  {
+    const Instance& excp = Api::UnwrapInstanceHandle(isolate, exception);
+    if (excp.IsNull()) {
+      RETURN_TYPE_ERROR(isolate, exception, Instance);
+    }
+    const Instance& stk = Api::UnwrapInstanceHandle(isolate, stacktrace);
+    if (stk.IsNull()) {
+      RETURN_TYPE_ERROR(isolate, stacktrace, Instance);
+    }
   }
   if (isolate->top_exit_frame_info() == 0) {
     // There are no dart frames on the stack so it would be illegal to
     // throw an exception here.
     return Api::NewError("No Dart frames on stack, cannot throw exception");
   }
   // Unwind all the API scopes till the exit frame before throwing an
   // exception.
   ApiState* state = isolate->api_state();
   ASSERT(state != NULL);
-  state->UnwindScopes(isolate->top_exit_frame_info());
+  UnwindApiScopesForRethrow(isolate, &exception, &stacktrace);
+  const Instance& excp = Api::UnwrapInstanceHandle(isolate, exception);
+  const Instance& stk = Api::UnwrapInstanceHandle(isolate, stacktrace);
   Exceptions::ReThrow(excp, stk);
   return Api::NewError("Exception was not re thrown, internal error");
 }
 
 
 // --- Native functions ---
 
 
 DART_EXPORT Dart_Handle Dart_GetNativeArgument(Dart_NativeArguments args,
                                                int index) {
@@ skipping 463 matching lines @@
 
 DART_EXPORT void Dart_GetPprofSymbolInfo(void** buffer, int* buffer_size) {
   Isolate* isolate = Isolate::Current();
   DebugInfo* pprof_symbol_generator = Dart::pprof_symbol_generator();
   if (pprof_symbol_generator != NULL) {
     DebugInfo::ByteBuffer* debug_region = new DebugInfo::ByteBuffer();
     ASSERT(debug_region != NULL);
     pprof_symbol_generator->WriteToMemory(debug_region);
     *buffer_size = debug_region->size();
     if (*buffer_size != 0) {
-      ApiZone* zone = Api::TopScope(isolate)->zone();
+      Zone* zone = Api::TopScope(isolate)->zone();
       *buffer = reinterpret_cast<void*>(zone->AllocUnsafe(*buffer_size));
       memmove(*buffer, debug_region->data(), *buffer_size);
     } else {
       *buffer = NULL;
     }
     delete debug_region;
   } else {
     *buffer = NULL;
     *buffer_size = 0;
   }
@@ skipping 40 matching lines @@
   }
   {
     NoGCScope no_gc;
     RawObject* raw_obj = obj.raw();
     isolate->heap()->SetPeer(raw_obj, peer);
   }
   return Api::Success(isolate);
 }
 
 }  // namespace dart