Fix for issue 19817

runtime/vm/dart_api_impl.cc

 // Copyright (c) 2013, 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 "include/dart_mirrors_api.h"
 #include "include/dart_native_api.h"
 
 #include "platform/assert.h"
 #include "vm/bigint_operations.h"
@@ skipping 353 matching lines @@
     const ReusableObjectHandleScope& reuse, Dart_Handle dart_handle) {
   Object& ref = reuse.Handle();
   ref = Api::UnwrapHandle(dart_handle);
   if (ref.IsInstance()) {
     return Instance::Cast(ref);
   }
   return Object::null_instance();
 }
 
 
-Dart_Handle Api::CheckIsolateState(Isolate* isolate) {
+Dart_Handle Api::CheckAndFinalizePendingClasses(Isolate* isolate) {
   if (!isolate->AllowClassFinalization()) {
     // Class finalization is blocked for the isolate. Do nothing.
     return Api::Success();
   }
   if (ClassFinalizer::ProcessPendingClasses()) {
     return Api::Success();
   }
   ASSERT(isolate->object_store()->sticky_error() != Object::null());
   return Api::NewHandle(isolate, isolate->object_store()->sticky_error());
 }
@@ skipping 1008 matching lines @@
                                             intptr_t* size) {
   Isolate* isolate = Isolate::Current();
   DARTSCOPE(isolate);
   TIMERSCOPE(isolate, time_creating_snapshot);
   if (buffer == NULL) {
     RETURN_NULL_ERROR(buffer);
   }
   if (size == NULL) {
     RETURN_NULL_ERROR(size);
   }
-  Dart_Handle state = Api::CheckIsolateState(isolate);
+  // Finalize all classes if needed.
+  Dart_Handle state = Api::CheckAndFinalizePendingClasses(isolate);
   if (::Dart_IsError(state)) {
     return state;
   }
   // Since this is only a snapshot the root library should not be set.
   isolate->object_store()->set_root_library(Library::Handle(isolate));
   FullSnapshotWriter writer(buffer, ApiReallocate);
   writer.WriteFullSnapshot();
   *size = writer.BytesWritten();
   return Api::Success();
 }
 
 
 DART_EXPORT Dart_Handle Dart_CreateScriptSnapshot(uint8_t** buffer,
                                                   intptr_t* size) {
   Isolate* isolate = Isolate::Current();
   DARTSCOPE(isolate);
   TIMERSCOPE(isolate, time_creating_snapshot);
   if (buffer == NULL) {
     RETURN_NULL_ERROR(buffer);
   }
   if (size == NULL) {
     RETURN_NULL_ERROR(size);
   }
-  Dart_Handle state = Api::CheckIsolateState(isolate);
+  // Finalize all classes if needed.
+  Dart_Handle state = Api::CheckAndFinalizePendingClasses(isolate);
   if (::Dart_IsError(state)) {
     return state;
   }
   Library& library =
       Library::Handle(isolate, isolate->object_store()->root_library());
   if (library.IsNull()) {
     return
         Api::NewError("%s expects the isolate to have a script loaded in it.",
                       CURRENT_FUNC);
   }
@@ skipping 270 matching lines @@
                                           Dart_Handle type,
                                           bool* value) {
   Isolate* isolate = Isolate::Current();
   DARTSCOPE(isolate);
 
   const Type& type_obj = Api::UnwrapTypeHandle(isolate, type);
   if (type_obj.IsNull()) {
     *value = false;
     RETURN_TYPE_ERROR(isolate, type, Type);
   }
+  if (!type_obj.IsFinalized()) {
+    return Api::NewError(
+        "%s expects argument 'type' to be a fully resolved type.",
+        CURRENT_FUNC);
+  }
   if (object == Api::Null()) {
     *value = false;
     return Api::Success();
   }
   const Instance& instance = Api::UnwrapInstanceHandle(isolate, object);
   if (instance.IsNull()) {
     *value = false;
     RETURN_TYPE_ERROR(isolate, object, Instance);
   }
-  // Finalize all classes.
-  Dart_Handle state = Api::CheckIsolateState(isolate);
-  if (::Dart_IsError(state)) {
-    *value = false;
-    return state;
-  }
   CHECK_CALLBACK_STATE(isolate);
   Error& malformed_type_error = Error::Handle(isolate);
   *value = instance.IsInstanceOf(type_obj,
                                  Object::null_type_arguments(),
                                  &malformed_type_error);
   ASSERT(malformed_type_error.IsNull());  // Type was created from a class.
   return Api::Success();
 }
 
 
@@ skipping 1756 matching lines @@
         CURRENT_FUNC);
   }
 
   // Get the class to instantiate.
   Object& unchecked_type = Object::Handle(Api::UnwrapHandle(type));
   if (unchecked_type.IsNull() || !unchecked_type.IsType()) {
     RETURN_TYPE_ERROR(isolate, type, Type);
   }
   Type& type_obj = Type::Handle();
   type_obj ^= unchecked_type.raw();
+  if (!type_obj.IsFinalized()) {
+    return Api::NewError(
+        "%s expects argument 'type' to be a fully resolved type.",
+        CURRENT_FUNC);
+  }
   Class& cls = Class::Handle(isolate, type_obj.type_class());
   TypeArguments& type_arguments =
       TypeArguments::Handle(isolate, type_obj.arguments());
 
   const String& base_constructor_name = String::Handle(isolate, cls.Name());
 
   // And get the name of the constructor to invoke.
   String& dot_name = String::Handle(isolate);
   result = Api::UnwrapHandle(constructor_name);
   if (result.IsNull()) {
     dot_name = Symbols::Dot().raw();
   } else if (result.IsString()) {
     dot_name = String::Concat(Symbols::Dot(), String::Cast(result));
   } else {
     RETURN_TYPE_ERROR(isolate, constructor_name, String);
   }
-  Dart_Handle state = Api::CheckIsolateState(isolate);
-  if (::Dart_IsError(state)) {
-    return state;
-  }
 
   // Resolve the constructor.
   String& constr_name =
       String::Handle(String::Concat(base_constructor_name, dot_name));
   result = ResolveConstructor("Dart_New",
                               cls,
                               base_constructor_name,
                               constr_name,
                               number_of_arguments);
   if (result.IsError()) {
@@ skipping 207 matching lines @@
   }
   const String& constructor_name = Api::UnwrapStringHandle(isolate, name);
   if (constructor_name.IsNull()) {
     RETURN_TYPE_ERROR(isolate, name, String);
   }
   const Instance& instance = Api::UnwrapInstanceHandle(isolate, object);
   if (instance.IsNull()) {
     RETURN_TYPE_ERROR(isolate, object, Instance);
   }
 
-  // Since we have allocated an object it would mean that all classes
-  // are finalized and hence it is not necessary to call
-  // Api::CheckIsolateState.
+  // Since we have allocated an object it would mean that the type
+  // is finalized.
   // TODO(asiva): How do we ensure that a constructor is not called more than
   // once for the same object.
 
   // Construct name of the constructor to invoke.
   const Type& type_obj = Type::Handle(isolate, instance.GetType());
   const Class& cls = Class::Handle(isolate, type_obj.type_class());
   const String& class_name = String::Handle(isolate, cls.Name());
   const Array& strings = Array::Handle(Array::New(3));
   strings.SetAt(0, class_name);
   strings.SetAt(1, Symbols::Dot());
@@ skipping 65 matching lines @@
         "%s expects argument 'number_of_arguments' to be non-negative.",
         CURRENT_FUNC);
   }
   const Object& obj = Object::Handle(isolate, Api::UnwrapHandle(target));
   if (obj.IsError()) {
     return target;
   }
   Dart_Handle result;
   Array& args = Array::Handle(isolate);
   if (obj.IsType()) {
-    // Finalize all classes.
-    Dart_Handle state = Api::CheckIsolateState(isolate);
-    if (::Dart_IsError(state)) {
-      return state;
+    if (!Type::Cast(obj).IsFinalized()) {
+      return Api::NewError(
+          "%s expects argument 'target' to be a fully resolved type.",
+          CURRENT_FUNC);
     }
 
     const Class& cls = Class::Handle(isolate, Type::Cast(obj).type_class());
     const Function& function = Function::Handle(
         isolate,
         Resolver::ResolveStatic(cls,
                                 function_name,
                                 number_of_arguments,
                                 Object::empty_array()));
     if (function.IsNull()) {
       const String& cls_name = String::Handle(isolate, cls.Name());
       return Api::NewError("%s: did not find static method '%s.%s'.",
                            CURRENT_FUNC,
                            cls_name.ToCString(),
                            function_name.ToCString());
     }
     // Setup args and check for malformed arguments in the arguments list.
     result = SetupArguments(isolate, number_of_arguments, arguments, 0, &args);
     if (!::Dart_IsError(result)) {
       result = Api::NewHandle(isolate,
                               DartEntry::InvokeFunction(function, args));
     }
     return result;
   } else if (obj.IsNull() || obj.IsInstance()) {
     // Since we have allocated an object it would mean that all classes

[hausner, 2014/07/22 18:23:32]

all classes, or the type of the receiver?

[siva, 2014/07/22 19:55:11]

Fixed the comment.

On 2014/07/22 18:23:32, hausner wrote:

-    // are finalized and hence it is not necessary to call
-    // Api::CheckIsolateState.
+    // are finalized and hence it is not necessary to check here.
     Instance& instance = Instance::Handle(isolate);
     instance ^= obj.raw();
     ArgumentsDescriptor args_desc(
         Array::Handle(ArgumentsDescriptor::New(number_of_arguments + 1)));
     const Function& function = Function::Handle(
         isolate,
         Resolver::ResolveDynamic(instance, function_name, args_desc));
     if (function.IsNull()) {
       // Setup args and check for malformed arguments in the arguments list.
       result = SetupArguments(isolate,
@@ skipping 18 matching lines @@
     if (!::Dart_IsError(result)) {
       args.SetAt(0, instance);
       result = Api::NewHandle(isolate,
                               DartEntry::InvokeFunction(function, args));
     }
     return result;
   } else if (obj.IsLibrary()) {
     // Check whether class finalization is needed.
     const Library& lib = Library::Cast(obj);
 
-    // Finalize all classes if needed.
-    Dart_Handle state = Api::CheckIsolateState(isolate);
-    if (::Dart_IsError(state)) {
-      return state;
+    // Check that the library is loaded.
+    if (!lib.Loaded()) {
+      return Api::NewError(
+          "%s expects library argument 'target' to be loaded.",
+          CURRENT_FUNC);
     }
 
     const Function& function =
         Function::Handle(isolate,
                          lib.LookupFunctionAllowPrivate(function_name));
     if (function.IsNull()) {
       return Api::NewError("%s: did not find top-level function '%s'.",
                            CURRENT_FUNC,
                            function_name.ToCString());
     }
@@ skipping 60 matching lines @@
 DART_EXPORT Dart_Handle Dart_GetField(Dart_Handle container, Dart_Handle name) {
   Isolate* isolate = Isolate::Current();
   DARTSCOPE(isolate);
   CHECK_CALLBACK_STATE(isolate);
 
   const String& field_name = Api::UnwrapStringHandle(isolate, name);
   if (field_name.IsNull()) {
     RETURN_TYPE_ERROR(isolate, name, String);
   }
 
-  // Finalize all classes.
-  Dart_Handle state = Api::CheckIsolateState(isolate);
-  if (::Dart_IsError(state)) {
-    return state;
-  }
-
   Field& field = Field::Handle(isolate);
   Function& getter = Function::Handle(isolate);
   const Object& obj = Object::Handle(isolate, Api::UnwrapHandle(container));
   if (obj.IsNull()) {
     return Api::NewError("%s expects argument 'container' to be non-null.",
                          CURRENT_FUNC);
   } else if (obj.IsType()) {
+    if (!Type::Cast(obj).IsFinalized()) {
+      return Api::NewError(
+          "%s expects argument 'container' to be a fully resolved type.",
+          CURRENT_FUNC);
+    }
     // To access a static field we may need to use the Field or the
     // getter Function.
     Class& cls = Class::Handle(isolate, Type::Cast(obj).type_class());
 
     field = cls.LookupStaticField(field_name);
     if (field.IsNull() || field.IsUninitialized()) {
       const String& getter_name =
           String::Handle(isolate, Field::GetterName(field_name));
       getter = cls.LookupStaticFunctionAllowPrivate(getter_name);
     }
@@ skipping 38 matching lines @@
                                                           args,
                                                           args_descriptor));
     }
     return Api::NewHandle(isolate, DartEntry::InvokeFunction(getter, args));
 
   } else if (obj.IsLibrary()) {
     // To access a top-level we may need to use the Field or the
     // getter Function.  The getter function may either be in the
     // library or in the field's owner class, depending.
     const Library& lib = Library::Cast(obj);
+    // Check that the library is loaded.
+    if (!lib.Loaded()) {
+      return Api::NewError(
+          "%s expects library argument 'container' to be loaded.",
+          CURRENT_FUNC);
+    }
     field = lib.LookupFieldAllowPrivate(field_name);
     if (field.IsNull()) {
       // No field found and no ambiguity error.  Check for a getter in the lib.
       const String& getter_name =
           String::Handle(isolate, Field::GetterName(field_name));
       getter = lib.LookupFunctionAllowPrivate(getter_name);
     } else if (!field.IsNull() && field.IsUninitialized()) {
       // A field was found.  Check for a getter in the field's owner classs.
       const Class& cls = Class::Handle(isolate, field.owner());
       const String& getter_name =
@@ skipping 35 matching lines @@
   }
 
   // Since null is allowed for value, we don't use UnwrapInstanceHandle.
   const Object& value_obj = Object::Handle(isolate, Api::UnwrapHandle(value));
   if (!value_obj.IsNull() && !value_obj.IsInstance()) {
     RETURN_TYPE_ERROR(isolate, value, Instance);
   }
   Instance& value_instance = Instance::Handle(isolate);
   value_instance ^= value_obj.raw();
 
-  // Finalize all classes.
-  Dart_Handle state = Api::CheckIsolateState(isolate);
-  if (::Dart_IsError(state)) {
-    return state;
-  }
   Field& field = Field::Handle(isolate);
   Function& setter = Function::Handle(isolate);
   const Object& obj = Object::Handle(isolate, Api::UnwrapHandle(container));
   if (obj.IsNull()) {
     return Api::NewError("%s expects argument 'container' to be non-null.",
                          CURRENT_FUNC);
   } else if (obj.IsType()) {
+    if (!Type::Cast(obj).IsFinalized()) {
+      return Api::NewError(
+          "%s expects argument 'container' to be a fully resolved type.",
+          CURRENT_FUNC);
+    }
+
     // To access a static field we may need to use the Field or the
     // setter Function.
     Class& cls = Class::Handle(isolate, Type::Cast(obj).type_class());
 
     field = cls.LookupStaticField(field_name);
     if (field.IsNull()) {
       String& setter_name =
           String::Handle(isolate, Field::SetterName(field_name));
       setter = cls.LookupStaticFunctionAllowPrivate(setter_name);
     }
@@ skipping 58 matching lines @@
                                                           args,
                                                           args_descriptor));
     }
     return Api::NewHandle(isolate, DartEntry::InvokeFunction(setter, args));
 
   } else if (obj.IsLibrary()) {
     // To access a top-level we may need to use the Field or the
     // setter Function.  The setter function may either be in the
     // library or in the field's owner class, depending.
     const Library& lib = Library::Cast(obj);
+    // Check that the library is loaded.
+    if (!lib.Loaded()) {
+      return Api::NewError(
+          "%s expects library argument 'container' to be loaded.",
+          CURRENT_FUNC);
+    }
     field = lib.LookupFieldAllowPrivate(field_name);
     if (field.IsNull()) {
       const String& setter_name =
           String::Handle(isolate, Field::SetterName(field_name));
       setter ^= lib.LookupFunctionAllowPrivate(setter_name);
     }
 
     if (!setter.IsNull()) {
       // Invoke the setter and return the result.
       const int kNumArgs = 1;
@@ skipping 690 matching lines @@
                                      intptr_t number_of_type_arguments,
                                      Dart_Handle* type_arguments) {
   Isolate* isolate = Isolate::Current();
   DARTSCOPE(isolate);
 
   // Validate the input arguments.
   const Library& lib = Api::UnwrapLibraryHandle(isolate, library);
   if (lib.IsNull()) {
     RETURN_TYPE_ERROR(isolate, library, Library);
   }
+  if (!lib.Loaded()) {
+    RETURN_TYPE_ERROR(isolate, library, Library);

[hausner, 2014/07/22 18:23:32]

Shouldn't this be the same "library must be loaded" error message you have in
other places above?

[siva, 2014/07/22 19:55:11]

True, good catch that was a cut and paste error.

On 2014/07/22 18:23:32, hausner wrote:

+  }
   const String& name_str = Api::UnwrapStringHandle(isolate, class_name);
   if (name_str.IsNull()) {
     RETURN_TYPE_ERROR(isolate, class_name, String);
   }
-  // Ensure all classes are finalized.
-  Dart_Handle state = Api::CheckIsolateState(isolate);
-  if (::Dart_IsError(state)) {
-    return state;
-  }
   const Class& cls =
       Class::Handle(isolate, lib.LookupClassAllowPrivate(name_str));
   if (cls.IsNull()) {
     const String& lib_name = String::Handle(isolate, lib.name());
     return Api::NewError("Type '%s' not found in library '%s'.",
                          name_str.ToCString(), lib_name.ToCString());
   }
   if (cls.NumTypeArguments() == 0) {
     if (number_of_type_arguments != 0) {
       return Api::NewError("Invalid number of type arguments specified, "
@@ skipping 106 matching lines @@
   Dart_Handle result;
   CompileSource(isolate, library, script, &result);
   // Propagate the error out right now.
   if (::Dart_IsError(result)) {
     return result;
   }
 
   // If this is the dart:_builtin library, register it with the VM.
   if (url_str.Equals("dart:_builtin")) {
     isolate->object_store()->set_builtin_library(library);
-    Dart_Handle state = Api::CheckIsolateState(isolate);
+    Dart_Handle state = Api::CheckAndFinalizePendingClasses(isolate);
     if (::Dart_IsError(state)) {
       return state;
     }
   }
   return result;
 }
 
 
 DART_EXPORT Dart_Handle Dart_LibraryImportLibrary(Dart_Handle library,
                                                   Dart_Handle import,
@@ skipping 105 matching lines @@
   Isolate* isolate = Isolate::Current();
   DARTSCOPE(isolate);
   CHECK_CALLBACK_STATE(isolate);
 
   isolate->DoneLoading();
 
   // TODO(hausner): move the remaining code below (finalization and
   // invoing of _completeDeferredLoads) into Isolate::DoneLoading().
 
   // Finalize all classes if needed.
-  Dart_Handle state = Api::CheckIsolateState(isolate);
+  Dart_Handle state = Api::CheckAndFinalizePendingClasses(isolate);
   if (::Dart_IsError(state)) {
     return state;
   }
 
   if (complete_futures) {
     const Library& corelib = Library::Handle(isolate, Library::CoreLibrary());
     const String& function_name =
         String::Handle(isolate, String::New("_completeDeferredLoads"));
     const Function& function =
         Function::Handle(isolate,
@@ skipping 93 matching lines @@
 
 
 DART_EXPORT void Dart_RegisterRootServiceRequestCallback(
     const char* name,
     Dart_ServiceRequestCallback callback,
     void* user_data) {
   Service::RegisterRootEmbedderCallback(name, callback, user_data);
 }
 
 }  // namespace dart