Fix for #2799 - OOM during TryUpdateModelAsync

The change here is that when we create the ModelValidationNodes to just
return them rather than adding them to the tree. For a very large model,
having these extra nodes in the tree would eventually cause an OOM.

We're going to be taking a more thorough look at this code separately,
hence the quick fix.
This commit is contained in:
Ryan Nowak 2015-07-21 14:50:03 -07:00
parent 0f20eb9112
commit bae442cf48
1 changed files with 12 additions and 6 deletions

View File

@ -166,7 +166,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation
ValidationContext validationContext) ValidationContext validationContext)
{ {
var isValid = true; var isValid = true;
ExpandValidationNode(validationContext, modelExplorer); var childNodes = GetChildNodes(validationContext, modelExplorer);
IList<IModelValidator> validators = null; IList<IModelValidator> validators = null;
var elementMetadata = modelExplorer.Metadata.ElementMetadata; var elementMetadata = modelExplorer.Metadata.ElementMetadata;
@ -175,7 +175,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation
validators = GetValidators(validationContext.ModelValidationContext.ValidatorProvider, elementMetadata); validators = GetValidators(validationContext.ModelValidationContext.ValidatorProvider, elementMetadata);
} }
foreach (var childNode in validationContext.ValidationNode.ChildNodes) foreach (var childNode in childNodes)
{ {
var childModelExplorer = childNode.ModelMetadata.MetadataKind == Metadata.ModelMetadataKind.Type ? var childModelExplorer = childNode.ModelMetadata.MetadataKind == Metadata.ModelMetadataKind.Type ?
_modelMetadataProvider.GetModelExplorerForType(childNode.ModelMetadata.ModelType, childNode.Model) : _modelMetadataProvider.GetModelExplorerForType(childNode.ModelMetadata.ModelType, childNode.Model) :
@ -271,16 +271,20 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation
return filters.Any(filter => filter.IsTypeExcluded(type)); return filters.Any(filter => filter.IsTypeExcluded(type));
} }
private void ExpandValidationNode(ValidationContext context, ModelExplorer modelExplorer) private IList<ModelValidationNode> GetChildNodes(ValidationContext context, ModelExplorer modelExplorer)
{ {
var validationNode = context.ValidationNode; var validationNode = context.ValidationNode;
// This is the trivial case where the node-tree that was built-up during binding already has
// all of the nodes we need.
if (validationNode.ChildNodes.Count != 0 || if (validationNode.ChildNodes.Count != 0 ||
!validationNode.ValidateAllProperties || !validationNode.ValidateAllProperties ||
validationNode.Model == null) validationNode.Model == null)
{ {
return; return validationNode.ChildNodes;
} }
var childNodes = new List<ModelValidationNode>(validationNode.ChildNodes);
var elementMetadata = modelExplorer.Metadata.ElementMetadata; var elementMetadata = modelExplorer.Metadata.ElementMetadata;
if (elementMetadata == null) if (elementMetadata == null)
{ {
@ -293,7 +297,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation
{ {
ValidateAllProperties = true ValidateAllProperties = true
}; };
validationNode.ChildNodes.Add(childNode); childNodes.Add(childNode);
} }
} }
else else
@ -312,10 +316,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation
ValidateAllProperties = true ValidateAllProperties = true
}; };
validationNode.ChildNodes.Add(childNode); childNodes.Add(childNode);
index++; index++;
} }
} }
return childNodes;
} }
private class ValidationContext private class ValidationContext