Fix #5807 - Race condition in Invoker

This change addressed a race condition in the ObjectMethodExecutor where
the default argument values array can become visible before it is
initialized. If a second observer accesses the array while it is being
initialized, it can observe a null value for a reference type parameter,
leading to a nullref.

The fix here is to make everything immutable and initialize it all up
front. There's no reason to create an OME without eventually running it,
so there's no downside to doing the initialization up front.

(cherry picked from commit 8f4ca32)

src/Microsoft.AspNetCore.Mvc.Core/Internal/ObjectMethodExecutor.cs

@@ -8,33 +8,47 @@
 using System.Linq.Expressions;
 using System.Reflection;
 using System.Threading.Tasks;
-using Microsoft.AspNetCore.Mvc.Core;
 using Microsoft.Extensions.Internal;
 
 namespace Microsoft.AspNetCore.Mvc.Internal
 {
     public class ObjectMethodExecutor
     {
-        private object[] _parameterDefaultValues;
-        private ActionExecutorAsync _executorAsync;
-        private ActionExecutor _executor;
+        private readonly object[] _parameterDefaultValues;
+        private readonly ActionExecutorAsync _executorAsync;
+        private readonly ActionExecutor _executor;
 
         private static readonly MethodInfo _convertOfTMethod =
             typeof(ObjectMethodExecutor).GetRuntimeMethods().Single(methodInfo => methodInfo.Name == nameof(ObjectMethodExecutor.Convert));
 
         private ObjectMethodExecutor(MethodInfo methodInfo, TypeInfo targetTypeInfo)
-        {            
+        {
             if (methodInfo == null)
             {
                 throw new ArgumentNullException(nameof(methodInfo));
             }
+
             MethodInfo = methodInfo;
             TargetTypeInfo = targetTypeInfo;
             ActionParameters = methodInfo.GetParameters();
             MethodReturnType = methodInfo.ReturnType;
             IsMethodAsync = typeof(Task).IsAssignableFrom(MethodReturnType);
             TaskGenericType = IsMethodAsync ? GetTaskInnerTypeOrNull(MethodReturnType) : null;
             IsTypeAssignableFromIActionResult = typeof(IActionResult).IsAssignableFrom(TaskGenericType ?? MethodReturnType);
+
+            if (IsMethodAsync && TaskGenericType != null)
+            {
+                // For backwards compatibility we're creating a sync-executor for an async method. This was
+                // supported in the past even though MVC wouldn't have called it.
+                _executor = GetExecutor(methodInfo, targetTypeInfo);
+                _executorAsync = GetExecutorAsync(TaskGenericType, methodInfo, targetTypeInfo);
+            }
+            else
+            {
+                _executor = GetExecutor(methodInfo, targetTypeInfo);
+            }
+
+            _parameterDefaultValues = GetParameterDefaultValues(ActionParameters);
         }
 
         private delegate Task<object> ActionExecutorAsync(object target, object[] parameters);
@@ -58,29 +72,15 @@ private ObjectMethodExecutor(MethodInfo methodInfo, TypeInfo targetTypeInfo)
 
         public bool IsTypeAssignableFromIActionResult { get; }
 
-        private ActionExecutorAsync TaskOfTActionExecutorAsync
-        {
-            get
-            {
-                if (_executorAsync == null)
-                {
-                    _executorAsync = GetExecutorAsync(TaskGenericType, MethodInfo, TargetTypeInfo);
-                }
-
-                return _executorAsync;
-            }
-        }
-
         public static ObjectMethodExecutor Create(MethodInfo methodInfo, TypeInfo targetTypeInfo)
         {
             var executor = new ObjectMethodExecutor(methodInfo, targetTypeInfo);
-            executor._executor = GetExecutor(methodInfo, targetTypeInfo);
             return executor;
         }
 
         public Task<object> ExecuteAsync(object target, object[] parameters)
         {
-            return TaskOfTActionExecutorAsync(target, parameters);
+            return _executorAsync(target, parameters);
         }
 
         public object Execute(object target, object[] parameters)
@@ -95,8 +95,6 @@ public object GetDefaultValueForParameter(int index)
                 throw new ArgumentOutOfRangeException(nameof(index));
             }
 
-            EnsureParameterDefaultValues();
-
             return _parameterDefaultValues[index];
         }
 
@@ -216,42 +214,40 @@ private static Task<object> Convert<T>(object taskAsObject)
             return CastToObject<T>(task);
         }
 
-        private void EnsureParameterDefaultValues()
+        private static object[] GetParameterDefaultValues(ParameterInfo[] parameters)
         {
-            if (_parameterDefaultValues == null)
+            var values = new object[parameters.Length];
+
+            for (var i = 0; i < parameters.Length; i++)
             {
-                var count = ActionParameters.Length;
-                _parameterDefaultValues = new object[count];
+                var parameterInfo = parameters[i];
+                object defaultValue;
 
-                for (var i = 0; i < count; i++)
+                if (parameterInfo.HasDefaultValue)
+                {
+                    defaultValue = parameterInfo.DefaultValue;
+                }
+                else
                 {
-                    var parameterInfo = ActionParameters[i];
-                    object defaultValue;
+                    var defaultValueAttribute = parameterInfo
+                        .GetCustomAttribute<DefaultValueAttribute>(inherit: false);
 
-                    if (parameterInfo.HasDefaultValue)
+                    if (defaultValueAttribute?.Value == null)
                     {
-                        defaultValue = parameterInfo.DefaultValue;
+                        defaultValue = parameterInfo.ParameterType.GetTypeInfo().IsValueType
+                            ? Activator.CreateInstance(parameterInfo.ParameterType)
+                            : null;
                     }
                     else
                     {
-                        var defaultValueAttribute = parameterInfo
-                            .GetCustomAttribute<DefaultValueAttribute>(inherit: false);
-
-                        if (defaultValueAttribute?.Value == null)
-                        {
-                            defaultValue = parameterInfo.ParameterType.GetTypeInfo().IsValueType
-                                ? Activator.CreateInstance(parameterInfo.ParameterType)
-                                : null;
-                        }
-                        else
-                        {
-                            defaultValue = defaultValueAttribute.Value;
-                        }
+                        defaultValue = defaultValueAttribute.Value;
                     }
-
-                    _parameterDefaultValues[i] = defaultValue;
                 }
+
+                values[i] = defaultValue;
             }
+
+            return values;
         }
     }
 }