From cb3208b03fa1ab54c596ab790bd2bd080d6d7788 Mon Sep 17 00:00:00 2001 From: objmalloc Date: Wed, 3 Oct 2018 16:54:23 +0300 Subject: [PATCH 1/5] Locking into ClrInstanceTypeWrapper.FromCache --- .../Library/ClrWrapper/ClrInstanceTypeWrapper.cs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Jurassic/Library/ClrWrapper/ClrInstanceTypeWrapper.cs b/Jurassic/Library/ClrWrapper/ClrInstanceTypeWrapper.cs index c8262f63..d17efe7c 100644 --- a/Jurassic/Library/ClrWrapper/ClrInstanceTypeWrapper.cs +++ b/Jurassic/Library/ClrWrapper/ClrInstanceTypeWrapper.cs @@ -12,6 +12,7 @@ namespace Jurassic.Library [DebuggerTypeProxy(typeof(ClrInstanceTypeWrapperDebugView))] internal class ClrInstanceTypeWrapper : ObjectInstance { + private static Object lockObject = new Object(); // INITIALIZATION //_________________________________________________________________________________________ @@ -28,11 +29,14 @@ public static ClrInstanceTypeWrapper FromCache(ScriptEngine engine, Type type) throw new JavaScriptException(engine, ErrorType.TypeError, "Unsupported type: CLR types are not supported. Enable CLR types by setting the ScriptEngine's EnableExposedClrTypes property to true."); ClrInstanceTypeWrapper cachedInstance; - if (engine.InstanceTypeWrapperCache.TryGetValue(type, out cachedInstance) == true) - return cachedInstance; - var newInstance = new ClrInstanceTypeWrapper(engine, type); - engine.InstanceTypeWrapperCache.Add(type, newInstance); - return newInstance; + lock (lockObject) + { + if (engine.InstanceTypeWrapperCache.TryGetValue(type, out cachedInstance) == true) + return cachedInstance; + var newInstance = new ClrInstanceTypeWrapper(engine, type); + engine.InstanceTypeWrapperCache.Add(type, newInstance); + return newInstance; + } } /// From 631523b5378037a1c942b85381917a801f407011 Mon Sep 17 00:00:00 2001 From: objmalloc Date: Wed, 10 Oct 2018 11:06:28 +0300 Subject: [PATCH 2/5] Locking on property initializations and function calls --- Jurassic/Compiler/Binders/Binder.cs | 35 +++++++++++++---------- Jurassic/Compiler/Binders/BinderMethod.cs | 29 +++++++++++++------ 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/Jurassic/Compiler/Binders/Binder.cs b/Jurassic/Compiler/Binders/Binder.cs index 094a7f38..2dc0b0db 100644 --- a/Jurassic/Compiler/Binders/Binder.cs +++ b/Jurassic/Compiler/Binders/Binder.cs @@ -22,6 +22,8 @@ internal abstract class Binder private BinderDelegate[] delegateCache; private const int MaximumCachedParameterCount = 8; + [NonSerialized] + private Object lockObject = new Object(); @@ -82,23 +84,26 @@ public object Call(ScriptEngine engine, object thisObject, params object[] argum /// the same parameter count will be markedly quicker. public BinderDelegate CreateDelegate(int argumentCount) { - // If there are too many arguments, don't cache the delegate. - if (argumentCount > MaximumCachedParameterCount) - return CreateDelegateCore(argumentCount); - - // Save the delegate that is created into a cache so it doesn't have to be created again. - if (this.delegateCache == null) - this.delegateCache = new BinderDelegate[MaximumCachedParameterCount + 1]; - var binderDelegate = this.delegateCache[argumentCount]; - if (binderDelegate == null) + lock (lockObject) { - // Create a binding method. - binderDelegate = CreateDelegateCore(argumentCount); - - // Store it in the cache. - this.delegateCache[argumentCount] = binderDelegate; + // If there are too many arguments, don't cache the delegate. + if (argumentCount > MaximumCachedParameterCount) + return CreateDelegateCore(argumentCount); + + // Save the delegate that is created into a cache so it doesn't have to be created again. + if (this.delegateCache == null) + this.delegateCache = new BinderDelegate[MaximumCachedParameterCount + 1]; + var binderDelegate = this.delegateCache[argumentCount]; + if (binderDelegate == null) + { + // Create a binding method. + binderDelegate = CreateDelegateCore(argumentCount); + + // Store it in the cache. + this.delegateCache[argumentCount] = binderDelegate; + } + return binderDelegate; } - return binderDelegate; } /// diff --git a/Jurassic/Compiler/Binders/BinderMethod.cs b/Jurassic/Compiler/Binders/BinderMethod.cs index 8ce0e8f3..28254494 100644 --- a/Jurassic/Compiler/Binders/BinderMethod.cs +++ b/Jurassic/Compiler/Binders/BinderMethod.cs @@ -19,6 +19,7 @@ internal class BinderMethod private int optionalParameterCount; private Type paramArrayElementType; + private Object lockObject = new Object(); // INITIALIZATION @@ -133,8 +134,11 @@ public int RequiredParameterCount { get { - if (this.initialized == false) - Init(); + lock (lockObject) + { + if (this.initialized == false) + Init(); + } return this.requiredParameterCount; } } @@ -146,8 +150,11 @@ public int OptionalParameterCount { get { - if (this.initialized == false) - Init(); + lock (lockObject) + { + if (this.initialized == false) + Init(); + } return this.optionalParameterCount; } } @@ -159,8 +166,11 @@ public bool HasParamArray { get { - if (this.initialized == false) - Init(); + lock (lockObject) + { + if (this.initialized == false) + Init(); + } return this.paramArrayElementType != null; } } @@ -172,8 +182,11 @@ private Type ParamArrayElementType { get { - if (this.initialized == false) - Init(); + lock (lockObject) + { + if (this.initialized == false) + Init(); + } return this.paramArrayElementType; } } From c41edfde2dc39317bedfb684f8ea25d3fe4acc1b Mon Sep 17 00:00:00 2001 From: objmalloc Date: Wed, 16 Jan 2019 12:13:55 +0200 Subject: [PATCH 3/5] Don't allow multiple adding of a ObjectInstance property --- Jurassic/Library/Object/ObjectInstance.cs | 120 +++++++++++++--------- 1 file changed, 71 insertions(+), 49 deletions(-) diff --git a/Jurassic/Library/Object/ObjectInstance.cs b/Jurassic/Library/Object/ObjectInstance.cs index 4cf7bd47..6fc10670 100644 --- a/Jurassic/Library/Object/ObjectInstance.cs +++ b/Jurassic/Library/Object/ObjectInstance.cs @@ -40,6 +40,8 @@ private enum ObjectFlags // Stores flags related to this object. private ObjectFlags flags = ObjectFlags.Extensible; + private object lockDuringSet = new object(); + // INITIALIZATION @@ -497,12 +499,15 @@ public PropertyDescriptor GetOwnPropertyDescriptor(object key) /// be set. This can happen if the property is read-only or if the object is sealed. public virtual void SetPropertyValue(uint index, object value, bool throwOnError) { - string indexStr = index.ToString(); - bool exists = SetPropertyValueIfExists(indexStr, value, throwOnError); - if (exists == false) + lock (lockDuringSet) { - // The property doesn't exist - add it. - AddProperty(indexStr, value, PropertyAttributes.FullAccess, throwOnError); + string indexStr = index.ToString(); + bool exists = SetPropertyValueIfExistsCore(indexStr, value, throwOnError); + if (exists == false) + { + // The property doesn't exist - add it. + AddProperty(indexStr, value, PropertyAttributes.FullAccess, throwOnError); + } } } @@ -527,11 +532,14 @@ public void SetPropertyValue(object key, object value, bool throwOnError) return; } - bool exists = SetPropertyValueIfExists(key, value, throwOnError); - if (exists == false) + lock (lockDuringSet) { - // The property doesn't exist - add it. - AddProperty(key, value, PropertyAttributes.FullAccess, throwOnError); + bool exists = SetPropertyValueIfExistsCore(key, value, throwOnError); + if (exists == false) + { + // The property doesn't exist - add it. + AddProperty(key, value, PropertyAttributes.FullAccess, throwOnError); + } } } @@ -596,6 +604,14 @@ public void SetPropertyValue(PropertyReference propertyReference, object value, /// property needs to be created). /// true if the property value exists; false otherwise. public bool SetPropertyValueIfExists(object key, object value, bool throwOnError) + { + lock (lockDuringSet) + { + return SetPropertyValueIfExistsCore(key, value, throwOnError); + } + } + + private bool SetPropertyValueIfExistsCore(object key, object value, bool throwOnError) { // Do not store nulls - null represents a non-existant value. value = value ?? Undefined.Value; @@ -736,48 +752,51 @@ public bool Delete(object key, bool throwOnError) /// true if the property was successfully modified; false otherwise. public virtual bool DefineProperty(object key, PropertyDescriptor descriptor, bool throwOnError) { - // Retrieve info on the property. - var current = this.schema.GetPropertyIndexAndAttributes(key); - - if (current.Exists == false) + lock (lockDuringSet) { - // Create a new property. - return AddProperty(key, descriptor.Value, descriptor.Attributes, throwOnError); - } + // Retrieve info on the property. + var current = this.schema.GetPropertyIndexAndAttributes(key); - // If the current property is not configurable, then the only change that is allowed is - // a change from one simple value to another (i.e. accessors are not allowed) and only - // if the writable attribute is set. - if (current.IsConfigurable == false) - { - // Get the current value of the property. - object currentValue = this.propertyValues[current.Index]; - object getter = null, setter = null; - if (currentValue is PropertyAccessorValue) + if (current.Exists == false) { - getter = ((PropertyAccessorValue)currentValue).Getter; - setter = ((PropertyAccessorValue)currentValue).Setter; + // Create a new property. + return AddProperty(key, descriptor.Value, descriptor.Attributes, throwOnError); } - // Check if the modification is allowed. - if (descriptor.Attributes != current.Attributes || - (descriptor.IsAccessor == true && (getter != descriptor.Getter || setter != descriptor.Setter)) || - (descriptor.IsAccessor == false && current.IsWritable == false && TypeComparer.SameValue(currentValue, descriptor.Value) == false)) + // If the current property is not configurable, then the only change that is allowed is + // a change from one simple value to another (i.e. accessors are not allowed) and only + // if the writable attribute is set. + if (current.IsConfigurable == false) { - if (throwOnError == true) - throw new JavaScriptException(this.Engine, ErrorType.TypeError, string.Format("The property '{0}' is non-configurable.", key)); - return false; + // Get the current value of the property. + object currentValue = this.propertyValues[current.Index]; + object getter = null, setter = null; + if (currentValue is PropertyAccessorValue) + { + getter = ((PropertyAccessorValue)currentValue).Getter; + setter = ((PropertyAccessorValue)currentValue).Setter; + } + + // Check if the modification is allowed. + if (descriptor.Attributes != current.Attributes || + (descriptor.IsAccessor == true && (getter != descriptor.Getter || setter != descriptor.Setter)) || + (descriptor.IsAccessor == false && current.IsWritable == false && TypeComparer.SameValue(currentValue, descriptor.Value) == false)) + { + if (throwOnError == true) + throw new JavaScriptException(this.Engine, ErrorType.TypeError, string.Format("The property '{0}' is non-configurable.", key)); + return false; + } } - } - // Set the property attributes. - if (descriptor.Attributes != current.Attributes) - this.schema = this.schema.SetPropertyAttributes(key, descriptor.Attributes); + // Set the property attributes. + if (descriptor.Attributes != current.Attributes) + this.schema = this.schema.SetPropertyAttributes(key, descriptor.Attributes); - // Set the property value. - this.propertyValues[current.Index] = descriptor.Value; + // Set the property value. + this.propertyValues[current.Index] = descriptor.Value; - return true; + return true; + } } /// @@ -833,16 +852,19 @@ private bool AddProperty(object key, object value, PropertyAttributes attributes /// Indicates whether to overwrite any existing attributes. internal void FastSetProperty(object key, object value, PropertyAttributes attributes = PropertyAttributes.Sealed, bool overwriteAttributes = false) { - int index = this.schema.GetPropertyIndex(key); - if (index < 0) + lock (lockDuringSet) { - // The property is doesn't exist - add a new property. - AddProperty(key, value, attributes, false); - return; + int index = this.schema.GetPropertyIndex(key); + if (index < 0) + { + // The property is doesn't exist - add a new property. + AddProperty(key, value, attributes, false); + return; + } + if (overwriteAttributes == true) + this.schema = this.schema.SetPropertyAttributes(key, attributes); + this.propertyValues[index] = value; } - if (overwriteAttributes == true) - this.schema = this.schema.SetPropertyAttributes(key, attributes); - this.propertyValues[index] = value; } /// From 6914ec8abb467eae9baec725713a2093b5c63512 Mon Sep 17 00:00:00 2001 From: objmalloc Date: Fri, 25 Jan 2019 16:11:13 +0200 Subject: [PATCH 4/5] Locking when ObjectInstance property is deleted --- Jurassic/Library/Object/ObjectInstance.cs | 50 +++++++++++++---------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/Jurassic/Library/Object/ObjectInstance.cs b/Jurassic/Library/Object/ObjectInstance.cs index 6fc10670..e87e7bef 100644 --- a/Jurassic/Library/Object/ObjectInstance.cs +++ b/Jurassic/Library/Object/ObjectInstance.cs @@ -694,16 +694,19 @@ private bool SetPropertyValueIfExistsCore(object key, object value, bool throwOn /// was false. public virtual bool Delete(uint index, bool throwOnError) { - string indexStr = index.ToString(); + lock (lockDuringSet) + { + string indexStr = index.ToString(); - // Retrieve the attributes for the property. - var propertyInfo = this.schema.GetPropertyIndexAndAttributes(indexStr); - if (propertyInfo.Exists == false) - return true; // Property doesn't exist - delete succeeded! + // Retrieve the attributes for the property. + var propertyInfo = this.schema.GetPropertyIndexAndAttributes(indexStr); + if (propertyInfo.Exists == false) + return true; // Property doesn't exist - delete succeeded! - // Delete the property. - this.schema = this.schema.DeleteProperty(indexStr); - return true; + // Delete the property. + this.schema = this.schema.DeleteProperty(indexStr); + return true; + } } /// @@ -722,22 +725,25 @@ public bool Delete(object key, bool throwOnError) if (arrayIndex != uint.MaxValue) return Delete(arrayIndex, throwOnError); - // Retrieve the attributes for the property. - var propertyInfo = this.schema.GetPropertyIndexAndAttributes(key); - if (propertyInfo.Exists == false) - return true; // Property doesn't exist - delete succeeded! - - // Check if the property can be deleted. - if (propertyInfo.IsConfigurable == false) + lock (lockDuringSet) { - if (throwOnError == true) - throw new JavaScriptException(this.Engine, ErrorType.TypeError, string.Format("The property '{0}' cannot be deleted.", key)); - return false; - } + // Retrieve the attributes for the property. + var propertyInfo = this.schema.GetPropertyIndexAndAttributes(key); + if (propertyInfo.Exists == false) + return true; // Property doesn't exist - delete succeeded! - // Delete the property. - this.schema = this.schema.DeleteProperty(key); - return true; + // Check if the property can be deleted. + if (propertyInfo.IsConfigurable == false) + { + if (throwOnError == true) + throw new JavaScriptException(this.Engine, ErrorType.TypeError, string.Format("The property '{0}' cannot be deleted.", key)); + return false; + } + + // Delete the property. + this.schema = this.schema.DeleteProperty(key); + return true; + } } /// From 8a1a0f95d0187e56b2bd1ed0c3bfcd188d75a62d Mon Sep 17 00:00:00 2001 From: objmalloc Date: Mon, 28 Jan 2019 18:15:59 +0200 Subject: [PATCH 5/5] Lock during schema property add --- Jurassic/Library/Object/HiddenClassSchema.cs | 64 +++++++++++--------- 1 file changed, 34 insertions(+), 30 deletions(-) diff --git a/Jurassic/Library/Object/HiddenClassSchema.cs b/Jurassic/Library/Object/HiddenClassSchema.cs index 9aab1b16..8ddc4d25 100644 --- a/Jurassic/Library/Object/HiddenClassSchema.cs +++ b/Jurassic/Library/Object/HiddenClassSchema.cs @@ -30,6 +30,8 @@ private struct TransitionInfo private HiddenClassSchema parent; private TransitionInfo addPropertyTransitionInfo; + private object lockDuringAdd = new object(); + /// /// Creates a new HiddenClassSchema instance from a modify or delete operation. /// @@ -132,42 +134,44 @@ public SchemaProperty GetPropertyIndexAndAttributes(object key) /// A new schema with the extra property. public HiddenClassSchema AddProperty(object key, PropertyAttributes attributes = PropertyAttributes.FullAccess) { - // Package the name and attributes into a struct. - var transitionInfo = new TransitionInfo() { Key = key, Attributes = attributes }; + lock (this.lockDuringAdd) + { + // Package the name and attributes into a struct. + var transitionInfo = new TransitionInfo() { Key = key, Attributes = attributes }; - // Check if there is a transition to the schema already. - HiddenClassSchema newSchema = null; - if (this.addTransitions != null) - this.addTransitions.TryGetValue(transitionInfo, out newSchema); + // Check if there is a transition to the schema already. + HiddenClassSchema newSchema = null; + if (this.addTransitions != null) + this.addTransitions.TryGetValue(transitionInfo, out newSchema); - if (newSchema == null) - { - if (this.parent == null) + if (newSchema == null) { - // Create a new schema based on this one. A complete copy must be made of the properties hashtable. - var properties = new Dictionary(this.properties); - properties.Add(key, new SchemaProperty(this.NextValueIndex, attributes)); - newSchema = new HiddenClassSchema(properties, this.NextValueIndex + 1, this, transitionInfo); + if (this.parent == null) + { + // Create a new schema based on this one. A complete copy must be made of the properties hashtable. + var properties = new Dictionary(this.properties); + properties.Add(key, new SchemaProperty(this.NextValueIndex, attributes)); + newSchema = new HiddenClassSchema(properties, this.NextValueIndex + 1, this, transitionInfo); + } + else + { + // Create a new schema based on this one. The properties hashtable is "given + // away" so a copy does not have to be made. + if (this.properties == null) + this.properties = CreatePropertiesDictionary(); + this.properties.Add(key, new SchemaProperty(this.NextValueIndex, attributes)); + newSchema = new HiddenClassSchema(this.properties, this.NextValueIndex + 1, this, transitionInfo); + this.properties = null; + } + + // Add a transition to the new schema. + if (this.addTransitions == null) + this.addTransitions = new Dictionary(1); + this.addTransitions.Add(transitionInfo, newSchema); } - else - { - // Create a new schema based on this one. The properties hashtable is "given - // away" so a copy does not have to be made. - if (this.properties == null) - this.properties = CreatePropertiesDictionary(); - this.properties.Add(key, new SchemaProperty(this.NextValueIndex, attributes)); - newSchema = new HiddenClassSchema(this.properties, this.NextValueIndex + 1, this, transitionInfo); - this.properties = null; - } - - // Add a transition to the new schema. - if (this.addTransitions == null) - this.addTransitions = new Dictionary(1); - this.addTransitions.Add(transitionInfo, newSchema); + return newSchema; } - - return newSchema; } ///