Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Commit

Permalink
Fix for #2799 - OOM during TryUpdateModelAsync
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rynowak committed Jul 22, 2015
1 parent 0f20eb9 commit bae442c
Showing 1 changed file with 12 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ private bool ValidateChildNodes(
ValidationContext validationContext)
{
var isValid = true;
ExpandValidationNode(validationContext, modelExplorer);
var childNodes = GetChildNodes(validationContext, modelExplorer);

IList<IModelValidator> validators = null;
var elementMetadata = modelExplorer.Metadata.ElementMetadata;
Expand All @@ -175,7 +175,7 @@ private bool ValidateChildNodes(
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 ?
_modelMetadataProvider.GetModelExplorerForType(childNode.ModelMetadata.ModelType, childNode.Model) :
Expand Down Expand Up @@ -271,16 +271,20 @@ private bool IsTypeExcludedFromValidation(IList<IExcludeTypeValidationFilter> fi
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;

// 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 ||
!validationNode.ValidateAllProperties ||
validationNode.Model == null)
{
return;
return validationNode.ChildNodes;
}

var childNodes = new List<ModelValidationNode>(validationNode.ChildNodes);
var elementMetadata = modelExplorer.Metadata.ElementMetadata;
if (elementMetadata == null)
{
Expand All @@ -293,7 +297,7 @@ private void ExpandValidationNode(ValidationContext context, ModelExplorer model
{
ValidateAllProperties = true
};
validationNode.ChildNodes.Add(childNode);
childNodes.Add(childNode);
}
}
else
Expand All @@ -312,10 +316,12 @@ private void ExpandValidationNode(ValidationContext context, ModelExplorer model
ValidateAllProperties = true
};

validationNode.ChildNodes.Add(childNode);
childNodes.Add(childNode);
index++;
}
}

return childNodes;
}

private class ValidationContext
Expand Down

0 comments on commit bae442c

Please sign in to comment.