Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nancy 1.4.1 memory leak #2113

Closed
cusnirati opened this Issue Nov 15, 2015 · 15 comments

Comments

Projects
None yet
5 participants
@cusnirati
Copy link

cusnirati commented Nov 15, 2015

While testing for high-load, I discovered a memory leak issue directly linked to either Nancy or Nancy.Hosting.Self .
Downgrading to the 1.2.0 version solves this.
I uploaded a small demo for this : https://github.com/cusnirati/Nancy1.4.1MemoryLeakDemo

I tested this using JMeter, the project for repeating my steps is inside the solution linked above.

screenshot 2015-11-15 22 28 26
screenshot 2015-11-15 22 29 04
screenshot 2015-11-15 22 29 41
screenshot 2015-11-15 22 30 13
screenshot 2015-11-15 22 30 52

@khellang

This comment has been minimized.

Copy link
Member

khellang commented Nov 15, 2015

It would be nice to narrow down the version even more. Are you seeing the same on 1.3?

@cusnirati

This comment has been minimized.

Copy link
Author

cusnirati commented Nov 15, 2015

For 1.3.0 everything looks good, no leaks.
Let me check for the 1.4.1.
I just mentioned the 1.2.0 because that was my previous working version.

@khellang

This comment has been minimized.

Copy link
Member

khellang commented Nov 15, 2015

OK, I've identified the regressing commit, 4aeaf31, pushed in 1.4.0. It introduced the CancellationTokenSource.CreateLinkedTokenSource call which allocates per request, without disposing it afterwards. This means that the linked CTS will be kept around forever.

Here's a minute of profiling with 400 connections on 12 threads:

image

image

I'll send a PR for it. Thanks for filing this! 👍

@cusnirati

This comment has been minimized.

Copy link
Author

cusnirati commented Nov 15, 2015

No problem :)

@khellang

This comment has been minimized.

Copy link
Member

khellang commented Nov 23, 2015

@cusnirati We've now shipped 1.4.2. Would you mind updating and letting us know if things look better?

@cusnirati

This comment has been minimized.

Copy link
Author

cusnirati commented Nov 23, 2015

I confirm, the issue is fixed.

@khellang

This comment has been minimized.

Copy link
Member

khellang commented Nov 23, 2015

Thanks for following up! 👍

@fastatech

This comment has been minimized.

Copy link

fastatech commented Jan 12, 2016

Hi,
I am not sure that the issue is fixed. I have installed version 1.4.3 from Nuget (but somehow the actual version referenced in project is 1.4.2 so maybe here is the problem) but the leak is still there. My process with Nancy grew up over the day up to 1GB of memory. Only thing I do is calling one GET(passing some parameters) route which returns string "success" from around 80 hosts. That is my real and only traffic on my server (I am just gathering data from the hosts).
Downgrading to 1.3.0 fixed the problem and the memory is around 115MB now. Please can you verify that you pushed correct package version to Nuget and the leak is really fixed. It seems strange to me. I can profile it for you if you want but I am bit new into this. I am running it on Ubuntu server with Mono, only in console. So if you tell me how to profile it I can do it.
Thanks,
Jan

@khellang

This comment has been minimized.

Copy link
Member

khellang commented Jan 12, 2016

@fastatech We've had several people confirm that the issue is fixed, including @cusnirati, in this thread. It could be something else, though. I'd like you to make sure you're actually running with 1.4.3 first...

@fastatech

This comment has been minimized.

Copy link

fastatech commented Jan 12, 2016

@khellang I have upgraded to the latest version (1.4.3) using Nuget manager but the version of the referenced Nancy.dll still says 1.4.2. Is this expected?
Please see this screenshot: http://snag.gy/CyLHL.jpg

@khellang

This comment has been minimized.

Copy link
Member

khellang commented Jan 12, 2016

@fastatech Hmm. Yes, looks like we forgot to bump the AssemblyVersion for the 1.4.3 release. Sorry about that 😞

If you check the properties for the assembly, you should see the "product version" is correct:

image

@fastatech

This comment has been minimized.

Copy link

fastatech commented Jan 12, 2016

@khellang No worries. So you are saying if I reference product version 1.4.3 (file version 1.4.2.0) the leak is no longer there and I should look for something else? I will try it again and investigate it. I just dont know how to profile Nancyfx on mono in console mode :/

@khellang

This comment has been minimized.

Copy link
Member

khellang commented Jan 12, 2016

Yep. The leak (in this issue) should be fixed in that version. If you still see a memory leak, it must be something else... 😞

@yzhou88

This comment has been minimized.

Copy link

yzhou88 commented Oct 30, 2016

I afraid the leak is still serious!
I am using Nancy 1.4.3.
Not sure if Nancy 2.00 can fix this issue.

@thecodejunkie

This comment has been minimized.

Copy link
Member

thecodejunkie commented Oct 30, 2016

@yzhou88 the leak, that was discussed in here, has been fixed, profiles and verified to have been fixed. If you think you have a memory leak, please file a new issue and provide as much detail as you possibly can and preferably a sample app that reproduces the bug. Make sure you are not causing the leak yourself by allocating unmanaged resources etc. but failing to properly release them. You can use the 5 day trial of DotMemory to profile your application and provide some better insight in your bug report.

Thanks

@khellang khellang referenced this issue Dec 30, 2016

Open

Fix a memory leak #2605

4 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.