HttpPost with entity as parameter stuck in an infinite loop? (rc2-final) #4666

HairyPortal · 2016-05-18T10:36:37Z

Consider a call to a simple post action like this

    [HttpPost]
    public async Task<IActionResult> Create(Organization data)
    {
        return NotFound();
    }

The call is unsuccessful and cause memory consumption go up wildly (1 GB every 30 second) and it never stop and consume all my memory.

It looks like the call is stuck in an infinite loop of some sort.

After some experiment, I found out that only classes that I use in DbContext cause this infinite loop. Swapping "Organization" (which is a class I used in DbContext) with other class like the one I used as viewmodel dose not cause this.

I'm not sure what I did wrong because in rc1 everything works just fine.

The text was updated successfully, but these errors were encountered:

xrkolovos · 2016-05-18T11:39:29Z

I am having the same issue. I have a complex ef model, and i get as parameter in the post method one object of this model. It's like the framework trying to discover all the dependancies with this model and iterates through the whole model.

i am running on net46

xrkolovos · 2016-05-18T11:58:49Z

i am testing this bug for like 4 hours. After adding more complexity(i mean more objects with more connections between them) the time that takes to execute the method rises. At a given number of objects (with connections between them), the app breaks and it nevers hit the method inside the mvc controller. After that also we get the memory leak.

The number of the classes in my model is only 24. Houston, We've Got a Problem.

fglebel · 2016-05-18T14:32:21Z

I am also stuck in a loop. Unable to advance. Was working fine in RC1. Unable to hit breakpoint.

I am using EFCore. Did a database first scaffold.

Bartmax · 2016-05-18T15:24:18Z

Just in case, a word from the MS team will be great.

rynowak · 2016-05-18T15:25:56Z

Just in case, a word from the MS team will be great.

Could one of you experiencing the issue grab a call-stack of where MVC is spending most of its time?

xrkolovos · 2016-05-18T16:15:06Z

can you provide some info how to get one? Because i don't know how. After i am skipping the last line of the constuctor the stact trace window is empty.

xrkolovos · 2016-05-18T16:16:38Z

you only need an complex model with about 20-25 classes and relations between them. if you need one, i can created for you.

fglebel · 2016-05-18T16:27:22Z

Since I cannot hit the breakpoint, call stack is empty. I am trying to reproduce from scratch.

xrkolovos · 2016-05-18T16:32:09Z

@fglebel how big is your model? (how many classes do you have related)

xrkolovos · 2016-05-18T17:08:13Z

I created a sample project that demonstrate this bug in OrderNoteController.cs with 2 methods. One with a complex class and one with a Viewmodel Class.

I used the domain model of this project https://github.com/smartstoreag/SmartStoreNET/tree/4db9e2f6f24eea13402fa869a5d88f5b20ee6c52/src/Libraries/SmartStore.Core/Domain

Decisions.WebCoreUi.zip

fglebel · 2016-05-18T17:23:10Z

I was able to reproduce quite easily.

I created a new ASP.NEW Core Web Application (.NET Core) web application with Windows Authentication from VS Studio 2015 Update 2.
Installed EF Core
Did a dotnet ef dbcontext scaffold of my SqlServer database into a Models folder
Added 1 index view, 1 edit view and 1 controller

startup.cs
for services:
added a dbcontext
added an authorize policy

Nothing changed inside app

project.json
added dependencies:
"Microsoft.EntityFrameworkCore.Tools": "1.0.0-preview1-final",
"Microsoft.EntityFrameworkCore.SqlServer": "1.0.0-rc2-final",
"Microsoft.EntityFrameworkCore.SqlServer.Design": "1.0.0-rc2-final"

added tools:
"Microsoft.EntityFrameworkCore.Tools": {
"imports": [ "portable-net451+win8" ],
"version": "1.0.0-preview1-final"
}

Here are all the references:

fglebel · 2016-05-18T17:45:22Z

@fglebel how big is your model? (how many classes do you have related)

@xrkolovos I have about 200

xrkolovos · 2016-05-18T18:05:31Z

Can we hope for a quick fix, in a new AspNetCore.Mvc Package, or a quick solution adding a class that impements a custom binding functionality overwriting the default one?

Any more info or workaround from the team will be helpful because we are stuck, and the next thing to do is migrate back to rc1.

webeg · 2016-05-18T18:55:45Z

Just in case, a word from the MS team will be great.

Could one of you experiencing the issue grab a call-stack of where MVC is spending most of its time?

@rynowak any news on this one?

rynowak · 2016-05-18T19:34:16Z

I'm investigating this currently, will update here when I know more

rynowak · 2016-05-18T21:01:34Z

TLDR

I have a workaround for this, and I'm currently verifying the workaround against our test suite.

For anyone that's interested in trying it out before I've had a change to verify the change in full, here's a preview: https://gist.github.com/rynowak/9488b3b1a6b81876d8b14de72feb0857

To use this, paste that class into your project, and add the following line to your ConfigureServices

services.AddSingleton<IModelBinderFactory, MyModelBinderFactory>();

What went wrong?

We made a change in RC2 to build a 'graph' of model binders up front the first time a parameter is bound. This allows us to do any expensive work necessary to figure out how to bind the first time, and take advantage of caching on each subsequent request.

What's causing you pain is that your model classes are deeply nested, or encounter the same types/properties along different paths through the model graph. The implementation of IModelBinderFactory shipped in 1.0.0-rc2-final doesn't attempt to share any intermediate results.

For a model like this:

public IActionResult CheckOut(Customer customer) { }

class Customer 
{
    int Id { get; set; }
    IList<Product> ProductsInCart { get; set; }
    IList<Product> ProductsViewed { get; set; }
}

class Product
{
  .....
}

We'll create at least 6 IModelBinders before we even look at the properties on Product.

one for the top level parameter (Customer)
one for the Id
one for ProductsInCart
one for ProductsViewed
and two for the class Product

This means that each property on Product now gets two binders created. Obviously this can scale to way more ridiculous levels than the example I gave here.

The fix is to share the intermediate results, so we're using a Dictionary<> rather than a "stack". We're now doing memoization instead of just cycle breaking. We'll consider making more changes in this area to minimize the number of binders created in the future, this is a quick workaround.

What you should know

If you have an action that exposes a large model, in particular if you're exposing your EF entities directly to model binding - you are vulnerable to overposting attacks! See more here if you're not familar with the issues: http://odetocode.com/blogs/scott/archive/2012/03/11/complete-guide-to-mass-assignment-in-asp-net-mvc.aspx

Even if your are not allowing EF to update all of your entities when you save state, by exposing a deeply nested model structure, you allow attackers to force model binding to create those extra objects and run validation on them, which can overload your site in extreme cases.

The safest thing to do is always to use view models / binding models

xrkolovos · 2016-05-19T06:56:47Z

Thanks for you quick workaround. I already tested in our app and is working.
It also nice sharing this information about overposting attacks.

We are generaly using vm but this is not always the case. Writing an app with the mvc framework might be a very rapid soultion, for a small internal app (as in my case now). In that case you might now have the time for optimal usage and also you might not have the threads of the internet. In that case, if the model is a litle bit complicated it might be hard to have an up front 'graph' of model with good performance. It might be a good idea this feature to be optional.

buraktamturk · 2016-05-19T10:00:16Z

Could anyone publish @rynowak's workaround to nuget? I would like to do myself but I just don't know which framework to target (I just target full framework).

xrkolovos · 2016-05-19T11:03:30Z

you dont need a package from nuget, and this solution is a simple workaround.

To use this, paste that class into your project, and add the following line to your ConfigureServices
services.AddSingleton<IModelBinderFactory, MyModelBinderFactory>();

the class -> https://gist.github.com/rynowak/9488b3b1a6b81876d8b14de72feb0857
the "services.AddSingleton.." line in your startup.cs

buraktamturk · 2016-05-19T11:13:19Z

@xrkolovos well, what I am trying to avoid is copying/maintaining the class to my projects individually. People usually maintain more than one project. And this class is irrelevant the purpose of many projects. (I mean just think mvc/web based calculator project and MyModelBinderFactory.cs just why?)

If someone makes this a class library, we could just import it like "AspNetCore.Mvc.Workaround4666": "1.0.0" then call services.AddSingleton just as usual. So we don't have to bloat our projects.

Bartmax · 2016-05-19T11:25:34Z

@buraktamturk you can save the file somewhere on your disk and link it on your projects. or create a class library with that file and reference as a dependency using the projects property on global.json if you prefer to have it as a package.
I don't see any value of having this as an online nuget package.

fglebel · 2016-05-19T12:33:35Z

Also working for me. thank you.

xrkolovos · 2016-05-19T12:36:45Z

Nuget packages mean more permanent solutions, thats not the point of workarounds. But you can always create your personal nuget package and share it accross many projects. But workarounds means that your must change it after.

(MyModelBinderFactory.cs i didn't used this name, i thing you souldn't)

buraktamturk · 2016-05-19T18:38:51Z

Agreed on a few points but I have a few projects hosted in git, and I don't like copying external contents, since they add complexity. Since it is on git, I can't copy the file once to my computer and link it to another projects.

I published "Tamturk.AspNetCore.Mvc.Workarounds.id4666" to nuget it has "1.0.0-rc2-final" version tag as others. It targets both netstandard1.5 and net46, I don't know whatever it's right thing to target both, since I just use full framework on my projects.

You can just add "Tamturk.AspNetCore.Mvc.Workarounds.id4666": "1.0.0-rc2-final" in dependency section on project.json.

And configure it in Startup like this:

services.AddSingleton<Tamturk.AspNetCore.Mvc.Workarounds.id4666.ModelBinderFactory4666>();

Since It's just workaround for only rc2-final, just remove them for next version of the project.

This change to ModelBinderFactory makes the caching much more aggressive, by caching all non-root binders. There's some trickiness here around making sure we have the right behavior when all providers return null. See the tests and comments. I also kept the change I made for a temporary workaround to use a dictionary rather than a "stack" for cycle breaking. This seems like an overall improvement in clarity.

prachya · 2016-06-02T19:08:50Z

I've been fixing this problem too. I did it follow the post of @rynowak and the problem fixed - hope an official fix will be available soon.

This change to ModelBinderFactory makes the caching much more aggressive, by caching all non-root binders. There's some trickiness here around making sure we have the right behavior when all providers return null. See the tests and comments. I also kept the change I made for a temporary workaround to use a dictionary rather than a "stack" for cycle breaking. This seems like an overall improvement in clarity.

rynowak · 2016-06-06T16:35:30Z

Fixed for 1.0.0 78c130d

Hayder90 · 2016-06-21T03:22:00Z

Hello Everyone ,
I had the same issue with the form post , i followed the instruction of the workaround ,
now i have a new issue : No parameterless constructor defined for this object
when i try to post a form i don't know wich object.
i made sure that my model have a parametreless constructure , i ve seen a post talking about the form collection in the IAction result ( if you remove it everything works fine i didn't try this ).
my controller have a constructor with parameters
thank you

dougbu · 2016-06-21T17:19:56Z

@Hayder90 suggest opening a new issue with details about your specific problem. Doesn't sound like anything related to #4666.

In the meantime, debugging your application and breaking on exceptions should get the stack where the "No parameterless constructor" issue is hit. That'll narrow down the types involved.

fhnainia · 2016-06-22T14:47:00Z

Here is the stack from @Hayder90:
System.MissingMethodException: No parameterless constructor defined for this object.
at System.RuntimeTypeHandle.CreateInstance(RuntimeType type, Boolean publicOnly, Boolean noCheck, Boolean& canBeCached, RuntimeMethodHandleInternal& ctor, Boolean& bNeedSecurityCheck)
at System.RuntimeType.CreateInstanceSlow(Boolean publicOnly, Boolean skipCheckThis, Boolean fillCache, StackCrawlMark& stackMark)
at System.Activator.CreateInstance(Type type, Boolean nonPublic)
at System.Activator.CreateInstance(Type type)
at Microsoft.AspNetCore.Mvc.ModelBinding.Binders.ComplexTypeModelBinder.d__3.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.AspNetCore.Mvc.Internal.ControllerArgumentBinder.d__7.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.AspNetCore.Mvc.Internal.ControllerArgumentBinder.d__10.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.AspNetCore.Mvc.Internal.FilterActionInvoker.d__40.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.AspNetCore.Mvc.Internal.FilterActionInvoker.d__39.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at Microsoft.AspNetCore.Mvc.Internal.FilterActionInvoker.d__32.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.AspNetCore.Mvc.Internal.MvcRouteHandler.d__8.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.AspNetCore.Builder.RouterMiddleware.d__4.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.AspNetCore.Session.SessionMiddleware.d__8.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at Microsoft.AspNetCore.Session.SessionMiddleware.d__8.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.<Invoke>d__18.MoveNext() --- End of stack trace from previous location where exception was thrown --- at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.d__18.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.<Invoke>d__18.MoveNext() --- End of stack trace from previous location where exception was thrown --- at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.d__18.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.<Invoke>d__18.MoveNext() --- End of stack trace from previous location where exception was thrown --- at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.d__18.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.<Invoke>d__18.MoveNext() --- End of stack trace from previous location where exception was thrown --- at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.d__18.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.VisualStudio.Web.BrowserLink.Runtime.BrowserLinkMiddleware.d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore.MigrationsEndPointMiddleware.d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore.DatabaseErrorPageMiddleware.d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore.DatabaseErrorPageMiddleware.d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.d__7.MoveNext()

fhnainia · 2016-06-22T15:13:21Z

The "No parameterless constructor" issue is fixed in here: #4895

oyvindvol · 2016-08-01T10:44:06Z

Is this fix already implemented in asp.net core RTM, so that I can delete my MyModelBinder-class from my project now?

dougbu · 2016-08-01T14:58:27Z

@oyvindvol yes, @rynowak's fix was included in the RTM (1.0.0) release.

Eilon assigned rynowak May 18, 2016

Eilon added this to the 1.0.0 milestone May 18, 2016

Eilon added bug 1 - Ready labels May 18, 2016

Bartmax mentioned this issue May 19, 2016

Scaffolding a view shouldn't require a DbContext? aspnet/Tooling#503

Closed

javiercn mentioned this issue May 21, 2016

RC2 Model binding hangs on POCO classes #4704

Closed

rynowak added 2 - Working and removed 1 - Ready labels May 24, 2016

davidfowl mentioned this issue Jun 6, 2016

rc2 postback very slow on first execution #4814

Closed

rynowak closed this as completed Jun 6, 2016

rynowak added 3 - Done and removed 2 - Working labels Jun 6, 2016

dougbu mentioned this issue Jun 20, 2016

POST to an action that accepts View Model with "User" property (regardless of the type) fails #4887

Closed

pranavkm mentioned this issue Sep 20, 2016

Model binding on circular referenced model, cpu usage 100% and memory usage grow until die #5300

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HttpPost with entity as parameter stuck in an infinite loop? (rc2-final) #4666

HttpPost with entity as parameter stuck in an infinite loop? (rc2-final) #4666

HairyPortal commented May 18, 2016

xrkolovos commented May 18, 2016 •

edited

Loading

xrkolovos commented May 18, 2016 •

edited

Loading

fglebel commented May 18, 2016 •

edited

Loading

Bartmax commented May 18, 2016

rynowak commented May 18, 2016

xrkolovos commented May 18, 2016

xrkolovos commented May 18, 2016

fglebel commented May 18, 2016

xrkolovos commented May 18, 2016

xrkolovos commented May 18, 2016 •

edited

Loading

fglebel commented May 18, 2016 •

edited

Loading

fglebel commented May 18, 2016

xrkolovos commented May 18, 2016

webeg commented May 18, 2016

rynowak commented May 18, 2016

rynowak commented May 18, 2016 •

edited

Loading

xrkolovos commented May 19, 2016

buraktamturk commented May 19, 2016

xrkolovos commented May 19, 2016 •

edited

Loading

buraktamturk commented May 19, 2016

Bartmax commented May 19, 2016 •

edited

Loading

fglebel commented May 19, 2016

xrkolovos commented May 19, 2016

buraktamturk commented May 19, 2016

prachya commented Jun 2, 2016

rynowak commented Jun 6, 2016

Hayder90 commented Jun 21, 2016

dougbu commented Jun 21, 2016

fhnainia commented Jun 22, 2016

fhnainia commented Jun 22, 2016

oyvindvol commented Aug 1, 2016

dougbu commented Aug 1, 2016

HttpPost with entity as parameter stuck in an infinite loop? (rc2-final) #4666

HttpPost with entity as parameter stuck in an infinite loop? (rc2-final) #4666

Comments

HairyPortal commented May 18, 2016

xrkolovos commented May 18, 2016 • edited Loading

xrkolovos commented May 18, 2016 • edited Loading

fglebel commented May 18, 2016 • edited Loading

Bartmax commented May 18, 2016

rynowak commented May 18, 2016

xrkolovos commented May 18, 2016

xrkolovos commented May 18, 2016

fglebel commented May 18, 2016

xrkolovos commented May 18, 2016

xrkolovos commented May 18, 2016 • edited Loading

fglebel commented May 18, 2016 • edited Loading

fglebel commented May 18, 2016

xrkolovos commented May 18, 2016

webeg commented May 18, 2016

rynowak commented May 18, 2016

rynowak commented May 18, 2016 • edited Loading

TLDR

What went wrong?

What you should know

xrkolovos commented May 19, 2016

buraktamturk commented May 19, 2016

xrkolovos commented May 19, 2016 • edited Loading

buraktamturk commented May 19, 2016

Bartmax commented May 19, 2016 • edited Loading

fglebel commented May 19, 2016

xrkolovos commented May 19, 2016

buraktamturk commented May 19, 2016

prachya commented Jun 2, 2016

rynowak commented Jun 6, 2016

Hayder90 commented Jun 21, 2016

dougbu commented Jun 21, 2016

fhnainia commented Jun 22, 2016

fhnainia commented Jun 22, 2016

oyvindvol commented Aug 1, 2016

dougbu commented Aug 1, 2016

xrkolovos commented May 18, 2016 •

edited

Loading

xrkolovos commented May 18, 2016 •

edited

Loading

fglebel commented May 18, 2016 •

edited

Loading

xrkolovos commented May 18, 2016 •

edited

Loading

fglebel commented May 18, 2016 •

edited

Loading

rynowak commented May 18, 2016 •

edited

Loading

xrkolovos commented May 19, 2016 •

edited

Loading

Bartmax commented May 19, 2016 •

edited

Loading