avm1: Properly and more explicitly work around the double-borrow issue in setters

This commit is contained in:
Nathan Adams 2020-06-28 17:49:32 +02:00
parent a2103d906d
commit ab4304d634
12 changed files with 54 additions and 83 deletions

View File

@ -500,10 +500,8 @@ impl<'gc> TObject<'gc> for FunctionObject<'gc> {
value: Value<'gc>,
activation: &mut StackFrame<'_, 'gc>,
context: &mut UpdateContext<'_, 'gc, '_>,
this: Object<'gc>,
) -> Result<ReturnValue<'gc>, Error<'gc>> {
self.base
.call_setter(name, value, activation, context, this)
) -> Option<Executable<'gc>> {
self.base.call_setter(name, value, activation, context)
}
#[allow(clippy::new_ret_no_self)]

View File

@ -128,7 +128,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
/// Call a setter defined in this object.
///
/// This function returns the `ReturnValue` of the called function; it
/// This function may return a `Executable` of the function to call; it
/// should be resolved and discarded. Attempts to call a non-virtual setter
/// or non-existent setter fail silently.
///
@ -141,8 +141,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
value: Value<'gc>,
activation: &mut StackFrame<'_, 'gc>,
context: &mut UpdateContext<'_, 'gc, '_>,
this: Object<'gc>,
) -> Result<ReturnValue<'gc>, Error<'gc>>;
) -> Option<Executable<'gc>>;
/// Construct a host object of some kind and return it's cell.
///

View File

@ -52,23 +52,16 @@ impl<'gc> Property<'gc> {
/// Set a property slot.
///
/// This function returns the `ReturnValue` of the property's virtual
/// This function may return an `Executable` of the property's virtual
/// function, if any happen to exist. It should be resolved, and it's value
/// discarded.
pub fn set(
&mut self,
activation: &mut StackFrame<'_, 'gc>,
context: &mut UpdateContext<'_, 'gc, '_>,
this: Object<'gc>,
base_proto: Option<Object<'gc>>,
new_value: impl Into<Value<'gc>>,
) -> Result<ReturnValue<'gc>, Error<'gc>> {
pub fn set(&mut self, new_value: impl Into<Value<'gc>>) -> Option<Executable<'gc>> {
match self {
Property::Virtual { set, .. } => {
if let Some(function) = set {
function.exec(activation, context, this, base_proto, &[new_value.into()])
Some(function.to_owned())
} else {
Ok(Value::Undefined.into())
None
}
}
Property::Stored {
@ -78,7 +71,7 @@ impl<'gc> Property<'gc> {
*value = new_value.into();
}
Ok(Value::Undefined.into())
None
}
}
}

View File

@ -1,7 +1,6 @@
use crate::avm1::error::Error;
use crate::avm1::function::{Executable, FunctionObject, NativeFunction};
use crate::avm1::property::{Attribute, Property};
use crate::avm1::return_value::ReturnValue;
use crate::avm1::stack_frame::StackFrame;
use crate::avm1::{Object, ObjectPtr, TObject, UpdateContext, Value};
use crate::property_map::{Entry, PropertyMap};
@ -229,7 +228,7 @@ impl<'gc> ScriptObject<'gc> {
.read()
.values
.contains_key(name, activation.is_case_sensitive());
let mut rval = None;
let mut worked = false;
if is_vacant {
let mut proto: Option<Object<'gc>> = Some((*self).into());
@ -244,44 +243,49 @@ impl<'gc> ScriptObject<'gc> {
}
if let Some(this_proto) = proto {
rval = Some(this_proto.call_setter(
name,
value.clone(),
activation,
context,
(*self).into(),
)?);
if let Some(rval) =
this_proto.call_setter(name, value.clone(), activation, context)
{
let _ = rval
.exec(
activation,
context,
this,
Some(this_proto),
&[value.clone()],
)?
.resolve(activation, context)?;
worked = true;
}
}
}
//No `rval` signals we didn't call a virtual setter above. Normally,
//This signals we didn't call a virtual setter above. Normally,
//we'd resolve and return up there, but we have borrows that need
//to end before we can do so.
if rval.is_none() {
rval = match self
if !worked {
let rval = match self
.0
.write(context.gc_context)
.values
.entry(name.to_owned(), activation.is_case_sensitive())
{
Entry::Occupied(mut entry) => Some(
entry
.get_mut()
.set(activation, context, this, base_proto, value)?,
),
Entry::Occupied(mut entry) => entry.get_mut().set(value.clone()),
Entry::Vacant(entry) => {
entry.insert(Property::Stored {
value,
value: value.clone(),
attributes: Default::default(),
});
None
}
};
}
if let Some(rval) = rval {
rval.resolve(activation, context)?;
if let Some(rval) = rval {
let _ = rval
.exec(activation, context, this, base_proto, &[value])?
.resolve(activation, context)?;
}
}
}
@ -367,18 +371,15 @@ impl<'gc> TObject<'gc> for ScriptObject<'gc> {
value: Value<'gc>,
activation: &mut StackFrame<'_, 'gc>,
context: &mut UpdateContext<'_, 'gc, '_>,
this: Object<'gc>,
) -> Result<ReturnValue<'gc>, Error<'gc>> {
) -> Option<Executable<'gc>> {
match self
.0
.write(context.gc_context)
.values
.get_mut(name, activation.is_case_sensitive())
{
Some(propref) if propref.is_virtual() => {
propref.set(activation, context, this, Some((*self).into()), value)
}
_ => Ok(Value::Undefined.into()),
Some(propref) if propref.is_virtual() => propref.set(value),
_ => None,
}
}
@ -734,6 +735,7 @@ mod tests {
use crate::avm1::activation::Activation;
use crate::avm1::globals::system::SystemProperties;
use crate::avm1::property::Attribute::*;
use crate::avm1::return_value::ReturnValue;
use crate::avm1::Avm1;
use crate::backend::audio::NullAudioBackend;
use crate::backend::input::NullInputBackend;

View File

@ -1,7 +1,6 @@
use crate::avm1::error::Error;
use crate::avm1::function::Executable;
use crate::avm1::property::Attribute;
use crate::avm1::return_value::ReturnValue;
use crate::avm1::sound_object::SoundObject;
use crate::avm1::stack_frame::StackFrame;
use crate::avm1::{Object, ObjectPtr, ScriptObject, TObject, Value};
@ -109,10 +108,8 @@ impl<'gc> TObject<'gc> for SharedObject<'gc> {
value: Value<'gc>,
activation: &mut StackFrame<'_, 'gc>,
context: &mut UpdateContext<'_, 'gc, '_>,
this: Object<'gc>,
) -> Result<ReturnValue<'gc>, Error<'gc>> {
self.base()
.call_setter(name, value, activation, context, this)
) -> Option<Executable<'gc>> {
self.base().call_setter(name, value, activation, context)
}
#[allow(clippy::new_ret_no_self)]

View File

@ -3,7 +3,6 @@
use crate::avm1::error::Error;
use crate::avm1::function::Executable;
use crate::avm1::property::Attribute;
use crate::avm1::return_value::ReturnValue;
use crate::avm1::stack_frame::StackFrame;
use crate::avm1::{Object, ObjectPtr, ScriptObject, TObject, Value};
use crate::backend::audio::{SoundHandle, SoundInstanceHandle};
@ -170,10 +169,8 @@ impl<'gc> TObject<'gc> for SoundObject<'gc> {
value: Value<'gc>,
activation: &mut StackFrame<'_, 'gc>,
context: &mut UpdateContext<'_, 'gc, '_>,
this: Object<'gc>,
) -> Result<ReturnValue<'gc>, Error<'gc>> {
self.base()
.call_setter(name, value, activation, context, this)
) -> Option<Executable<'gc>> {
self.base().call_setter(name, value, activation, context)
}
#[allow(clippy::new_ret_no_self)]

View File

@ -4,7 +4,6 @@ use crate::avm1::error::Error;
use crate::avm1::function::Executable;
use crate::avm1::object::search_prototype;
use crate::avm1::property::Attribute;
use crate::avm1::return_value::ReturnValue;
use crate::avm1::stack_frame::StackFrame;
use crate::avm1::{Object, ObjectPtr, ScriptObject, TDisplayObject, TObject, Value};
use crate::context::UpdateContext;
@ -242,12 +241,11 @@ impl<'gc> TObject<'gc> for StageObject<'gc> {
value: Value<'gc>,
activation: &mut StackFrame<'_, 'gc>,
context: &mut UpdateContext<'_, 'gc, '_>,
this: Object<'gc>,
) -> Result<ReturnValue<'gc>, Error<'gc>> {
) -> Option<Executable<'gc>> {
self.0
.read()
.base
.call_setter(name, value, activation, context, this)
.call_setter(name, value, activation, context)
}
#[allow(clippy::new_ret_no_self)]

View File

@ -4,7 +4,6 @@ use crate::avm1::error::Error;
use crate::avm1::function::Executable;
use crate::avm1::object::search_prototype;
use crate::avm1::property::Attribute;
use crate::avm1::return_value::ReturnValue;
use crate::avm1::script_object::TYPE_OF_OBJECT;
use crate::avm1::stack_frame::StackFrame;
use crate::avm1::{Object, ObjectPtr, ScriptObject, TObject, Value};
@ -150,12 +149,11 @@ impl<'gc> TObject<'gc> for SuperObject<'gc> {
value: Value<'gc>,
activation: &mut StackFrame<'_, 'gc>,
context: &mut UpdateContext<'_, 'gc, '_>,
this: Object<'gc>,
) -> Result<ReturnValue<'gc>, Error<'gc>> {
) -> Option<Executable<'gc>> {
self.0
.read()
.child
.call_setter(name, value, activation, context, this)
.call_setter(name, value, activation, context)
}
#[allow(clippy::new_ret_no_self)]

View File

@ -4,7 +4,6 @@ use crate::avm1::error::Error;
use crate::avm1::function::Executable;
use crate::avm1::object::{ObjectPtr, TObject};
use crate::avm1::property::Attribute;
use crate::avm1::return_value::ReturnValue;
use crate::avm1::stack_frame::StackFrame;
use crate::avm1::{Object, ScriptObject, UpdateContext, Value};
use enumset::EnumSet;
@ -176,12 +175,11 @@ impl<'gc> TObject<'gc> for ValueObject<'gc> {
value: Value<'gc>,
activation: &mut StackFrame<'_, 'gc>,
context: &mut UpdateContext<'_, 'gc, '_>,
this: Object<'gc>,
) -> Result<ReturnValue<'gc>, Error<'gc>> {
) -> Option<Executable<'gc>> {
self.0
.read()
.base
.call_setter(name, value, activation, context, this)
.call_setter(name, value, activation, context)
}
#[allow(clippy::new_ret_no_self)]

View File

@ -4,7 +4,6 @@ use crate::avm1::error::Error;
use crate::avm1::function::Executable;
use crate::avm1::object::{ObjectPtr, TObject};
use crate::avm1::property::Attribute;
use crate::avm1::return_value::ReturnValue;
use crate::avm1::stack_frame::StackFrame;
use crate::avm1::{Object, ScriptObject, UpdateContext, Value};
use crate::xml::{XMLName, XMLNode};
@ -105,10 +104,8 @@ impl<'gc> TObject<'gc> for XMLAttributesObject<'gc> {
value: Value<'gc>,
activation: &mut StackFrame<'_, 'gc>,
context: &mut UpdateContext<'_, 'gc, '_>,
this: Object<'gc>,
) -> Result<ReturnValue<'gc>, Error<'gc>> {
self.base()
.call_setter(name, value, activation, context, this)
) -> Option<Executable<'gc>> {
self.base().call_setter(name, value, activation, context)
}
#[allow(clippy::new_ret_no_self)]

View File

@ -4,7 +4,6 @@ use crate::avm1::error::Error;
use crate::avm1::function::Executable;
use crate::avm1::object::{ObjectPtr, TObject};
use crate::avm1::property::Attribute;
use crate::avm1::return_value::ReturnValue;
use crate::avm1::stack_frame::StackFrame;
use crate::avm1::{Object, ScriptObject, UpdateContext, Value};
use crate::xml::{XMLDocument, XMLNode};
@ -105,10 +104,8 @@ impl<'gc> TObject<'gc> for XMLIDMapObject<'gc> {
value: Value<'gc>,
activation: &mut StackFrame<'_, 'gc>,
context: &mut UpdateContext<'_, 'gc, '_>,
this: Object<'gc>,
) -> Result<ReturnValue<'gc>, Error<'gc>> {
self.base()
.call_setter(name, value, activation, context, this)
) -> Option<Executable<'gc>> {
self.base().call_setter(name, value, activation, context)
}
#[allow(clippy::new_ret_no_self)]

View File

@ -4,7 +4,6 @@ use crate::avm1::error::Error;
use crate::avm1::function::Executable;
use crate::avm1::object::{ObjectPtr, TObject};
use crate::avm1::property::Attribute;
use crate::avm1::return_value::ReturnValue;
use crate::avm1::stack_frame::StackFrame;
use crate::avm1::{Object, ScriptObject, UpdateContext, Value};
use crate::xml::{XMLDocument, XMLNode};
@ -97,10 +96,8 @@ impl<'gc> TObject<'gc> for XMLObject<'gc> {
value: Value<'gc>,
activation: &mut StackFrame<'_, 'gc>,
context: &mut UpdateContext<'_, 'gc, '_>,
this: Object<'gc>,
) -> Result<ReturnValue<'gc>, Error<'gc>> {
self.base()
.call_setter(name, value, activation, context, this)
) -> Option<Executable<'gc>> {
self.base().call_setter(name, value, activation, context)
}
#[allow(clippy::new_ret_no_self)]