runtime/vm/dart_api_impl.cc - Issue 411633002: Fix for issue 19817

Unified Diff: runtime/vm/dart_api_impl.cc

Issue 411633002: Fix for issue 19817 (Closed) Base URL: http://dart.googlecode.com/svn/branches/bleeding_edge/dart/

Patch Set: Created 6 years, 5 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Index: runtime/vm/dart_api_impl.cc

===================================================================

--- runtime/vm/dart_api_impl.cc (revision 38463)

+++ runtime/vm/dart_api_impl.cc (working copy)

@@ -371,7 +371,7 @@

}

-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();

@@ -1400,7 +1400,8 @@

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;

}

@@ -1424,7 +1425,8 @@

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;

}

@@ -1715,6 +1717,11 @@

*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();

@@ -1724,12 +1731,6 @@

*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,

@@ -3506,6 +3507,11 @@

}

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());

@@ -3522,10 +3528,6 @@

} 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 =

@@ -3753,9 +3755,8 @@

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.

@@ -3841,10 +3842,10 @@

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());

@@ -3870,8 +3871,7 @@

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, hausne

- // 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(

@@ -3910,10 +3910,11 @@

// 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 =

@@ -3994,12 +3995,6 @@

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));

@@ -4007,6 +4002,11 @@

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());

@@ -4065,6 +4065,12 @@

// 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.

@@ -4120,11 +4126,6 @@

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));

@@ -4132,6 +4133,12 @@

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());

@@ -4210,6 +4217,12 @@

// 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 =

@@ -4920,15 +4933,13 @@

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

siva 2014/07/22 19:55:11 True, good catch that was a cut and paste error.

+ }

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()) {

@@ -5055,7 +5066,7 @@

// 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;

}

@@ -5181,7 +5192,7 @@

// 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;

}

« runtime/bin/dartutils.cc ('K') | « runtime/vm/dart_api_impl.h ('k') | runtime/vm/dart_api_impl_test.cc » ('j') | runtime/vm/debugger_api_impl.cc » ('J')