[Core] Use lock in MSBuildPropertyGroupEvaluated

Using a ConcurrentDictionary broke the tests that rely on the items
being ordered in the Dictionary. The item definition test
ItemDefinitionGroup_AddFilesWithoutMetadata_MetadataUsesEmptyElements
failed due to the item definition properties being re-ordered. For
now using a lock around the Dictionary instead of a
ConcurrentDictionary.

Author: mrward

main/src/core/MonoDevelop.Core/MonoDevelop.Projects.MSBuild/MSBuildPropertyGroupEvaluated.cs

@@ -24,16 +24,15 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 // THE SOFTWARE.
 using System;
-using System.Collections.Concurrent;
 using System.Collections.Generic;
-using System.Xml.Linq;
+using System.Linq;
 using MonoDevelop.Core;
 
 namespace MonoDevelop.Projects.MSBuild
 {
 	class MSBuildPropertyGroupEvaluated: MSBuildNode, IMSBuildPropertyGroupEvaluated, IMSBuildProjectObject
 	{
-		protected ConcurrentDictionary<string, IMSBuildPropertyEvaluated> properties = new ConcurrentDictionary<string, IMSBuildPropertyEvaluated> (StringComparer.OrdinalIgnoreCase);
+		protected Dictionary<string, IMSBuildPropertyEvaluated> properties = new Dictionary<string, IMSBuildPropertyEvaluated> (StringComparer.OrdinalIgnoreCase);
 		MSBuildEngine engine;
 
 		internal MSBuildPropertyGroupEvaluated (MSBuildProject parent)
@@ -43,45 +42,60 @@ internal MSBuildPropertyGroupEvaluated (MSBuildProject parent)
 
 		internal void Sync (MSBuildEngine engine, object item, bool clearProperties = true)
 		{
-			if (clearProperties)
-				properties.Clear ();
+			if (clearProperties) {
+				lock (properties) {
+					properties.Clear ();
+				}
+			}
 			this.engine = engine;
 			foreach (var propName in engine.GetItemMetadataNames (item)) {
 				var prop = new MSBuildPropertyEvaluated (ParentProject, propName, engine.GetItemMetadata (item, propName), engine.GetEvaluatedItemMetadata (item, propName));
-				properties [propName] = prop;
+				lock (properties) {
+					properties [propName] = prop;
+				}
 			}
 		}
 
 		public bool HasProperty (string name)
 		{
-			return properties.ContainsKey (name);
+			lock (properties) {
+				return properties.ContainsKey (name);
+			}
 		}
 
 		public IMSBuildPropertyEvaluated GetProperty (string name)
 		{
 			IMSBuildPropertyEvaluated prop;
-			properties.TryGetValue (name, out prop);
+			lock (properties) {
+				properties.TryGetValue (name, out prop);
+			}
 			return prop;
 		}
 
 		internal void SetProperty (string key, IMSBuildPropertyEvaluated value)
 		{
-			properties [key] = value;
+			lock (properties) {
+				properties [key] = value;
+			}
 		}
 
 		internal void SetProperties (Dictionary<string,IMSBuildPropertyEvaluated> properties)
 		{
-			this.properties = new ConcurrentDictionary<string, IMSBuildPropertyEvaluated> (properties, StringComparer.OrdinalIgnoreCase);
+			this.properties = properties;
 		}
 
 		public IEnumerable<IMSBuildPropertyEvaluated> GetProperties ()
 		{
-			return properties.Values;
+			lock (properties) {
+				return properties.Values.ToArray ();
+			}
 		}
 
 		internal bool RemoveProperty (string name)
 		{
-			return properties.TryRemove (name, out _);
+			lock (properties) {
+				return properties.Remove (name);
+			}
 		}
 
 		public string GetValue (string name, string defaultValue = null)
@@ -153,11 +167,15 @@ public MSBuildEvaluatedPropertyCollection (MSBuildProject parent): base (parent)
 
 		internal void SyncCollection (MSBuildEngine e, object project)
 		{
-			properties.Clear ();
+			lock (properties) {
+				properties.Clear ();
+			}
 			foreach (var p in e.GetEvaluatedProperties (project)) {
 				string name, value, finalValue; bool definedMultipleTimes;
 				e.GetPropertyInfo (p, out name, out value, out finalValue, out definedMultipleTimes);
-				properties [name] = new MSBuildPropertyEvaluated (ParentProject, name, value, finalValue, definedMultipleTimes);
+				lock (properties) {
+					properties [name] = new MSBuildPropertyEvaluated (ParentProject, name, value, finalValue, definedMultipleTimes);
+				}
 			}
 		}
 
@@ -244,7 +262,9 @@ MSBuildPropertyEvaluated AddProperty (string name)
 		{
 			var p = new MSBuildPropertyEvaluated (ParentProject, name, null, null);
 			p.IsNew = true;
-			properties [name] = p;
+			lock (properties) {
+				properties [name] = p;
+			}
 			return p;
 		}
 
@@ -281,13 +301,19 @@ void IPropertyGroupListener.PropertyRemoved (MSBuildProperty prop)
 				//    that property group property.
 				if (ep.IsNew || !prop.IsNew) {
 					ep.IsNew = false;
-					properties.TryRemove (ep.Name, out _);
+					lock (properties) {
+						properties.Remove (ep.Name);
+					}
 				}
 			}
 		}
 
 		public IEnumerable<IMSBuildPropertyEvaluated> Properties {
-			get { return properties.Values; }
+			get {
+				lock (properties) {
+					return properties.Values.ToArray ();
+				}
+			}
 		}
 	}