[sre] Make creation of dynamic method synchronized

Before, reflection_create_dynamic_method could end up being called by two threads simultaneously. At the end of the method the ilgen field of the MonoReflectionDynamicMethod object is cleared, to enable its collection since it is no longer needed. The problem with this is that it can confuse the other thread compiling the dynamic method, making it see that there is no body.

We could either bother making this clearing work with multiple threads (and I'm not 100% convinced that the rest of the code is thread safe), disable the clearing of ilgen (which would make us use more memory) or go with the simple approach of locking on this DynamicMethod so only one thread is compiling it at one time.

Commit migrated from https://github.com/mono/mono/commit/71630cce754551e2f0414d59afb8824e7e0b0cfc

diff --git a/src/mono/mono/metadata/sre.c b/src/mono/mono/metadata/sre.c
index 4c13e84..4e4a574 100644 (file)
--- a/src/mono/mono/metadata/sre.c
+++ b/src/mono/mono/metadata/sre.c
@@ -4148,9 +4148,6 @@ reflection_create_dynamic_method (MonoReflectionDynamicMethodHandle ref_mb, Mono
        }
        g_slist_free (mb->referenced_by);
 
-       /* ilgen is no longer needed */
-       mb->ilgen = NULL;
-
        domain = mono_domain_get ();
        mono_domain_lock (domain);
        if (!domain->method_to_dyn_method)

diff --git a/src/mono/netcore/CoreFX.issues.rsp b/src/mono/netcore/CoreFX.issues.rsp
index 20a4083..f612264 100644 (file)
--- a/src/mono/netcore/CoreFX.issues.rsp
+++ b/src/mono/netcore/CoreFX.issues.rsp
@@ -160,12 +160,6 @@
 # Expected test Assertion fails https://github.com/mono/mono/issues/14951
 -nomethod System.Linq.Expressions.Tests.CallTests.MethodName_TypeArgsDontMatchConstraints_ThrowsArgumentException
 
-# flaky tests - Invalid IL code in (wrapper dynamic-method)
--nomethod System.Linq.Expressions.Tests.LambdaMultiplyNullableTests.LambdaMultiplyNullableFloatTest
--nomethod System.Linq.Expressions.Tests.LambdaMultiplyTests.LambdaMultiplyFloatTest
--nomethod System.Linq.Expressions.Tests.LambdaDivideTests.LambdaDivideFloatTest
--nomethod System.Linq.Expressions.Tests.OpAssign.AssignmentEquivalentsWithMemberAccess
-
 ####################################################################
 ##  System.Linq.Parallel.Tests
 ####################################################################
@@ -775,23 +769,6 @@
 -nomethod System.Data.SqlClient.Tests.DiagnosticTest.ExecuteScalarAsyncErrorTest
 -nomethod System.Data.SqlClient.Tests.DiagnosticTest.ExecuteScalarAsyncTest
 
-# System.InvalidProgramException : Invalid IL code in (wrapper dynamic-method) System.Text.RegularExpressions.CompiledRegexRunner:Go3 (System.Text.RegularExpressions.RegexRunner): IL_01af: nop
--nomethod System.Data.SqlClient.Tests.SqlErrorCollectionTest.GetEnumerator_Success
--nomethod System.Data.SqlClient.Tests.SqlErrorCollectionTest.CopyTo_NonGeneric_Success
--nomethod System.Data.SqlClient.Tests.SqlErrorCollectionTest.Indexer_Success
--nomethod System.Data.SqlClient.Tests.SqlErrorCollectionTest.Indexer_Throws
--nomethod System.Data.SqlClient.Tests.SqlErrorCollectionTest.CopyTo_Throws
--nomethod System.Data.SqlClient.Tests.SqlErrorCollectionTest.CopyTo_Success
--nomethod System.Data.SqlClient.Tests.SqlErrorCollectionTest.SyncRoot_Success
--nomethod System.Data.SqlClient.Tests.SqlErrorCollectionTest.IsSynchronized_Success
--nomethod System.Data.SqlClient.Tests.SqlErrorCollectionTest.CopyTo_NonGeneric_Throws
--nomethod System.Data.SqlClient.Tests.ExceptionTest.IndependentConnectionExceptionTestOpenConnection
--nomethod System.Data.SqlClient.Tests.ExceptionTest.ExceptionTests
--nomethod System.Data.SqlClient.Tests.ExceptionTest.IndependentConnectionExceptionTestExecuteReader
--nomethod System.Data.SqlClient.Tests.ExceptionTest.NamedPipeInvalidConnStringTest_ManagedSNI
--nomethod System.Data.SqlClient.Tests.SqlConnectionBasicTests.ConnectionTest
--nomethod System.Data.SqlClient.Tests.CloneTests.CloneSqlConnection
-
 ####################################################################
 ##  System.Diagnostics.StackTrace.Tests
 ####################################################################

diff --git a/src/mono/netcore/System.Private.CoreLib/src/System.Reflection.Emit/DynamicMethod.cs b/src/mono/netcore/System.Private.CoreLib/src/System.Reflection.Emit/DynamicMethod.cs
index e351fa6..f8034ec 100644 (file)
--- a/src/mono/netcore/System.Private.CoreLib/src/System.Reflection.Emit/DynamicMethod.cs
+++ b/src/mono/netcore/System.Private.CoreLib/src/System.Reflection.Emit/DynamicMethod.cs
@@ -136,30 +136,33 @@ namespace System.Reflection.Emit {
                private static extern void create_dynamic_method (DynamicMethod m);
 
                private void CreateDynMethod () {
-                       if (mhandle.Value == IntPtr.Zero) {
-                               if (ilgen == null || ilgen.ILOffset == 0)
-                                       throw new InvalidOperationException ("Method '" + name + "' does not have a method body.");
-
-                               ilgen.label_fixup (this);
-
-                               // Have to create all DynamicMethods referenced by this one
-                               try {
-                                       // Used to avoid cycles
-                                       creating = true;
-                                       if (refs != null) {
-                                               for (int i = 0; i < refs.Length; ++i) {
-                                                       if (refs [i] is DynamicMethod) {
-                                                               DynamicMethod m = (DynamicMethod)refs [i];
-                                                               if (!m.creating)
-                                                                       m.CreateDynMethod ();
+                       // Clearing of ilgen in create_dynamic_method is not yet synchronized for multiple threads
+                       lock (this) {
+                               if (mhandle.Value == IntPtr.Zero) {
+                                       if (ilgen == null || ilgen.ILOffset == 0)
+                                               throw new InvalidOperationException ("Method '" + name + "' does not have a method body.");
+
+                                       ilgen.label_fixup (this);
+
+                                       // Have to create all DynamicMethods referenced by this one
+                                       try {
+                                               // Used to avoid cycles
+                                               creating = true;
+                                               if (refs != null) {
+                                                       for (int i = 0; i < refs.Length; ++i) {
+                                                               if (refs [i] is DynamicMethod) {
+                                                                       DynamicMethod m = (DynamicMethod)refs [i];
+                                                                       if (!m.creating)
+                                                                               m.CreateDynMethod ();
+                                                               }
                                                        }
                                                }
+                                       } finally {
+                                               creating = false;
                                        }
-                               } finally {
-                                       creating = false;
+                                       create_dynamic_method (this);
+                                       ilgen = null;
                                }
-
-                               create_dynamic_method (this);
                        }
                }