Merge pull request #641 from Unity-Technologies/improve-thread-safety

Improves thread safety of lazy initializations

Mono.Cecil.Cil/MethodBody.cs

@@ -53,23 +53,38 @@ public MetadataToken LocalVarToken {
 		}
 
 		public Collection<Instruction> Instructions {
-			get { return instructions ?? (instructions = new InstructionCollection (method)); }
+			get {
+				if (instructions == null)
+					Interlocked.CompareExchange (ref instructions, new InstructionCollection (method), null);
+
+				return instructions;
+			}
 		}
 
 		public bool HasExceptionHandlers {
 			get { return !exceptions.IsNullOrEmpty (); }
 		}
 
 		public Collection<ExceptionHandler> ExceptionHandlers {
-			get { return exceptions ?? (exceptions = new Collection<ExceptionHandler> ()); }
+			get {
+				if (exceptions == null)
+					Interlocked.CompareExchange (ref exceptions, new Collection<ExceptionHandler> (), null);
+
+				return exceptions;
+			}
 		}
 
 		public bool HasVariables {
 			get { return !variables.IsNullOrEmpty (); }
 		}
 
 		public Collection<VariableDefinition> Variables {
-			get { return variables ?? (variables = new VariableDefinitionCollection ()); }
+			get {
+				if (variables == null)
+					Interlocked.CompareExchange (ref variables, new VariableDefinitionCollection (), null);
+
+				return variables;
+			}
 		}
 
 		public ParameterDefinition ThisParameter {

Mono.Cecil.Cil/Symbols.cs

@@ -12,6 +12,7 @@
 using System.Collections.Generic;
 using System.IO;
 using System.Runtime.InteropServices;
+using System.Threading;
 using SR = System.Reflection;
 
 using Mono.Collections.Generic;
@@ -118,23 +119,38 @@ public bool HasScopes {
 		}
 
 		public Collection<ScopeDebugInformation> Scopes {
-			get { return scopes ?? (scopes = new Collection<ScopeDebugInformation> ()); }
+			get {
+				if (scopes == null)
+					Interlocked.CompareExchange (ref scopes, new Collection<ScopeDebugInformation> (), null);
+
+				return scopes;
+			}
 		}
 
 		public bool HasVariables {
 			get { return !variables.IsNullOrEmpty (); }
 		}
 
 		public Collection<VariableDebugInformation> Variables {
-			get { return variables ?? (variables = new Collection<VariableDebugInformation> ()); }
+			get {
+				if (variables == null)
+					Interlocked.CompareExchange (ref variables, new Collection<VariableDebugInformation> (), null);
+
+				return variables;
+			}
 		}
 
 		public bool HasConstants {
 			get { return !constants.IsNullOrEmpty (); }
 		}
 
 		public Collection<ConstantDebugInformation> Constants {
-			get { return constants ?? (constants = new Collection<ConstantDebugInformation> ()); }
+			get {
+				if (constants == null)
+					Interlocked.CompareExchange (ref constants, new Collection<ConstantDebugInformation> (), null);
+
+				return constants;
+			}
 		}
 
 		internal ScopeDebugInformation ()
@@ -259,7 +275,12 @@ public bool HasCustomDebugInformations {
 		}
 
 		public Collection<CustomDebugInformation> CustomDebugInformations {
-			get { return custom_infos ?? (custom_infos = new Collection<CustomDebugInformation> ()); }
+			get {
+				if (custom_infos == null)
+					Interlocked.CompareExchange (ref custom_infos, new Collection<CustomDebugInformation> (), null);
+
+				return custom_infos;
+			}
 		}
 
 		internal DebugInformation ()
@@ -409,7 +430,13 @@ public bool HasTargets {
 		}
 
 		public Collection<ImportTarget> Targets {
-			get { return targets ?? (targets = new Collection<ImportTarget> ()); }
+			get
+			{
+				if (targets == null)
+					Interlocked.CompareExchange (ref targets, new Collection<ImportTarget> (), null);
+
+				return targets;
+			}
 		}
 
 		public ImportDebugInformation Parent {
@@ -488,11 +515,21 @@ public InstructionOffset CatchHandler {
 		}
 
 		public Collection<InstructionOffset> Yields {
-			get { return yields ?? (yields = new Collection<InstructionOffset> ()); }
+			get {
+				if (yields == null)
+					Interlocked.CompareExchange (ref yields, new Collection<InstructionOffset> (), null);
+
+				return yields;
+			}
 		}
 
 		public Collection<InstructionOffset> Resumes {
-			get { return resumes ?? (resumes = new Collection<InstructionOffset> ()); }
+			get {
+				if (resumes == null)
+					Interlocked.CompareExchange (ref resumes, new Collection<InstructionOffset> (), null);
+
+				return resumes;
+			}
 		}
 
 		public Collection<MethodDefinition> ResumeMethods {
@@ -641,7 +678,12 @@ public bool HasSequencePoints {
 		}
 
 		public Collection<SequencePoint> SequencePoints {
-			get { return sequence_points ?? (sequence_points = new Collection<SequencePoint> ()); }
+			get {
+				if (sequence_points == null)
+					Interlocked.CompareExchange (ref sequence_points, new Collection<SequencePoint> (), null);
+
+				return sequence_points;
+			}
 		}
 
 		public ScopeDebugInformation Scope {

Mono.Cecil/ArrayType.cs

@@ -10,6 +10,7 @@
 
 using System;
 using System.Text;
+using System.Threading;
 using Mono.Collections.Generic;
 using MD = Mono.Cecil.Metadata;
 
@@ -57,8 +58,11 @@ public Collection<ArrayDimension> Dimensions {
 				if (dimensions != null)
 					return dimensions;
 
-				dimensions = new Collection<ArrayDimension> ();
-				dimensions.Add (new ArrayDimension ());
+				var empty_dimensions = new Collection<ArrayDimension> ();
+				empty_dimensions.Add (new ArrayDimension ());
+
+				Interlocked.CompareExchange (ref dimensions, empty_dimensions, null);
+
 				return dimensions;
 			}
 		}

Mono.Cecil/AssemblyDefinition.cs

@@ -10,7 +10,7 @@
 
 using System;
 using System.IO;
-
+using System.Threading;
 using Mono.Collections.Generic;
 
 namespace Mono.Cecil {
@@ -46,7 +46,8 @@ public Collection<ModuleDefinition> Modules {
 				if (main_module.HasImage)
 					return main_module.Read (ref modules, this, (_, reader) => reader.ReadModules ());
 
-				return modules = new Collection<ModuleDefinition> (1) { main_module };
+				Interlocked.CompareExchange (ref modules, new Collection<ModuleDefinition> (1) { main_module }, null);
+				return modules;
 			}
 		}
 

Mono.Cecil/AssemblyNameReference.cs

@@ -12,6 +12,7 @@
 using System.Globalization;
 using System.Security.Cryptography;
 using System.Text;
+using System.Threading;
 
 namespace Mono.Cecil {
 
@@ -84,20 +85,20 @@ public byte [] PublicKey {
 			set {
 				public_key = value;
 				HasPublicKey = !public_key.IsNullOrEmpty ();
-				public_key_token = Empty<byte>.Array;
+				public_key_token = null;
 				full_name = null;
 			}
 		}
 
 		public byte [] PublicKeyToken {
 			get {
-				if (public_key_token.IsNullOrEmpty () && !public_key.IsNullOrEmpty ()) {
+				if (public_key_token == null && !public_key.IsNullOrEmpty ()) {
 					var hash = HashPublicKey ();
 					// we need the last 8 bytes in reverse order
 					var local_public_key_token = new byte [8];
 					Array.Copy (hash, (hash.Length - 8), local_public_key_token, 0, 8);
 					Array.Reverse (local_public_key_token, 0, 8);
-					public_key_token = local_public_key_token; // publish only once finished (required for thread-safety)
+					Interlocked.CompareExchange (ref public_key_token, local_public_key_token, null); // publish only once finished (required for thread-safety)
 				}
 				return public_key_token ?? Empty<byte>.Array;
 			}
@@ -160,7 +161,9 @@ public string FullName {
 					builder.Append ("Retargetable=Yes");
 				}
 
-				return full_name = builder.ToString ();
+				Interlocked.CompareExchange (ref full_name, builder.ToString (), null);
+
+				return full_name;
 			}
 		}
 

Mono.Cecil/CustomAttribute.cs

@@ -10,6 +10,7 @@
 
 using System;
 using System.Diagnostics;
+using System.Threading;
 using Mono.Collections.Generic;
 
 namespace Mono.Cecil {
@@ -105,7 +106,10 @@ public Collection<CustomAttributeArgument> ConstructorArguments {
 			get {
 				Resolve ();
 
-				return arguments ?? (arguments = new Collection<CustomAttributeArgument> ());
+				if (arguments == null)
+					Interlocked.CompareExchange (ref arguments, new Collection<CustomAttributeArgument> (), null);
+
+				return arguments;
 			}
 		}
 
@@ -121,7 +125,10 @@ public Collection<CustomAttributeNamedArgument> Fields {
 			get {
 				Resolve ();
 
-				return fields ?? (fields = new Collection<CustomAttributeNamedArgument> ());
+				if (fields == null)
+					Interlocked.CompareExchange (ref fields, new Collection<CustomAttributeNamedArgument> (), null);
+
+				return fields;
 			}
 		}
 
@@ -137,7 +144,10 @@ public Collection<CustomAttributeNamedArgument> Properties {
 			get {
 				Resolve ();
 
-				return properties ?? (properties = new Collection<CustomAttributeNamedArgument> ());
+				if (properties == null)
+					Interlocked.CompareExchange (ref properties, new Collection<CustomAttributeNamedArgument> (), null);
+
+				return properties;
 			}
 		}
 
@@ -185,21 +195,26 @@ void Resolve ()
 			if (resolved || !HasImage)
 				return;
 
-			Module.Read (this, (attribute, reader) => {
-				try {
-					reader.ReadCustomAttributeSignature (attribute);
-					resolved = true;
-				} catch (ResolutionException) {
-					if (arguments != null)
-						arguments.Clear ();
-					if (fields != null)
-						fields.Clear ();
-					if (properties != null)
-						properties.Clear ();
-
-					resolved = false;
-				}
-			});
+			lock (Module.SyncRoot) {
+				if (resolved)
+					return;
+
+				Module.Read (this, (attribute, reader) => {
+					try {
+						reader.ReadCustomAttributeSignature (attribute);
+						resolved = true;
+					} catch (ResolutionException) {
+						if (arguments != null)
+							arguments.Clear ();
+						if (fields != null)
+							fields.Clear ();
+						if (properties != null)
+							properties.Clear ();
+
+						resolved = false;
+					}
+				});
+			}
 		}
 	}
 }

Mono.Cecil/EventDefinition.cs

@@ -8,6 +8,7 @@
 // Licensed under the MIT/X11 license.
 //
 
+using System.Threading;
 using Mono.Collections.Generic;
 
 namespace Mono.Cecil {
@@ -78,10 +79,10 @@ public Collection<MethodDefinition> OtherMethods {
 
 				InitializeMethods ();
 
-				if (other_methods != null)
-					return other_methods;
+				if (other_methods == null)
+					Interlocked.CompareExchange (ref other_methods, new Collection<MethodDefinition> (), null);
 
-				return other_methods = new Collection<MethodDefinition> ();
+				return other_methods;
 			}
 		}
 

Mono.Cecil/FieldDefinition.cs

@@ -9,6 +9,7 @@
 //
 
 using System;
+using System.Threading;
 using Mono.Collections.Generic;
 
 namespace Mono.Cecil {
@@ -37,7 +38,11 @@ void ResolveLayout ()
 				return;
 			}
 
-			offset = Module.Read (this, (field, reader) => reader.ReadFieldLayout (field));
+			lock (Module.SyncRoot) {
+				if (offset != Mixin.NotResolvedMarker)
+					return;
+				offset = Module.Read (this, (field, reader) => reader.ReadFieldLayout (field));
+			}
 		}
 
 		public bool HasLayoutInfo {
@@ -76,7 +81,11 @@ void ResolveRVA ()
 			if (!HasImage)
 				return;
 
-			rva = Module.Read (this, (field, reader) => reader.ReadFieldRVA (field));
+			lock (Module.SyncRoot) {
+				if (rva != Mixin.NotResolvedMarker)
+					return;
+				rva = Module.Read (this, (field, reader) => reader.ReadFieldRVA (field));
+			}
 		}
 
 		public int RVA {