dotnet / wpf

WPF is a .NET Core UI framework for building Windows desktop applications.
MIT License
6.98k stars 1.16k forks source link

Memory held by the resources is not released even after disposing all the references in WPF when disabling prefer-32 bit in the application #1082

Closed vsfeedback closed 5 years ago

vsfeedback commented 5 years ago

Hi,

I created a WPF application with byte array, memory stream and BitmapImage when a main page is loaded as in the below snippet.

public partial class MainWindow:Window     
{         
byte[] buffer;         
Stream stream;         
BitmapImage bitmap;         

public MainWindow()         
{             
InitializeComponent();             
this. Loaded+=MainWindow_Loaded;                    
}          
private void MainWindow_Loaded(object sender,RoutedEventArgs e)         
{             
buffer =newbyte[100000000];             
stream =newMemoryStream(buffer);             
bitmap =newBitmapImage();             
bitmap. BeginInit();             
bitmap. UriSource=newUri(@"Image file Path");             
bitmap. EndInit();         
}
}

When the main page is loaded, I noted down the memory (using Visual Studio Diagnostic tool) is raised to 160 MB.

I wrote a button click event and cleared all the resources properly in the event handler as in the below code snippet.

private void button_Click(object sender,RoutedEventArgs e)         
{             
Array.Clear(buffer,0, buffer. Length);             
buffer =null;             
stream. Close();             
stream. Dispose();             
bitmap. UriSource=null;             
bitmap =null;
}

When test the memory in Visual Studio Diagnostic tool, I could see that the memory held by the resources is not released even after cleared and the memory stays at 160 MB.

When tried to call the Garbage collection as in the below code snippet, I could see only 5 MB is reduced and the memory still stays at 155 MB.

GC. Collect();             
GC. WaitForPendingFinalizers();

I suspect memory leak in this. Can you please suggest me how to release the total memory held by the resources?

Note: Disable the Prefer-32 bit in the Application

MSDN forum reference link (for more details): https://social.msdn.microsoft.com/Forums/vstudio/en-US/4b519b48-e9c8-4a66-b051-ce8a0acde2a7/memory-held-by-the-resources-is-not-released-even-after-disposing-all-the-references-in-wpf?forum=wpf

Regards,

Deepak G

This issue has been moved from https://developercommunity.visualstudio.com/content/problem/620424/memory-held-by-the-resources-is-not-released-even.html VSTS ticketId: 937073 These are the original issue comments: (no comments) These are the original issue solutions: (no solutions)

WamWooWam commented 5 years ago

This doesn't seem to be a bug? You're allocating a CLR array that's 100MB in size, it should be freed when the garbage collector decides it's no longer in use. There's no leak here.

Deepak211 commented 5 years ago

it should be freed when the garbage collector decides it's no longer in use. There's no leak here.

I have the below two questions.

  1. Why GC.Collect is not works in 64-bit process application (when disabling prefer-32 bit in the application)?
  2. Why it works when enabling prefer 32-bit?

image

mikedn commented 5 years ago

I don't see any suspicious GC behavior. That stream holds a reference to the array so the array won't ever get GCed. Null out the stream if you want it to work as expected.

WamWooWam commented 5 years ago

GC.Collect won't necessarily free anything, it all depends on more variables than I can count, the .NET Garbage Collector is a complex beast.

Deepak211 commented 5 years ago

Null out the stream if you want it to work as expected.

@mikedn , I have tried null out the stream too. But the behavior varies when enabling and disabling prefer 32-bit. The issue still occurs in disabling prefer-32 bit as I mentioned in my 2nd and 3rd reply in the below forum.

https://social.msdn.microsoft.com/Forums/vstudio/en-US/4b519b48-e9c8-4a66-b051-ce8a0acde2a7/memory-held-by-the-resources-is-not-released-even-after-disposing-all-the-references-in-wpf?forum=wpf

mikedn commented 5 years ago

Back when I played with this I wasn't able to detect any differences between 32 and 64 bit. Besides, even if there are some differences the main issue in your example is that you do not null out the stream and that prevents the array from being collected.

From your forum thread I gather that the issue is actually related to WPF's handling of images, something which your example doesn't illustrate. If you're having problems with handling large images in 32 bit then that might be a WPF real issue. But then large images and 32 bit is probably not a good mix and the normal solution would simply be to use 64 bit.

So I guess the question is what exactly are you doing - how large are those images? how many such images are trying to load? and so on.

stevenbrix commented 5 years ago

@Deepak211 I think your example is a little misleading, generally you need to ensure that 2nd Generation GC has run and actually released all the allocations marked for cleanup. If I change the button click handler to this and click it twice, you see that GC is properly cleaning everything up. In your example, while resources are marked for cleanup, there is nothing in your application causing GC to run again and actually cleanup the resources you've freed.

        private void button_Click(object sender, RoutedEventArgs e)
        {
            if (buffer != null)
            {
                Array.Clear(buffer, 0, buffer.Length);
                buffer = null;
                stream.Close();
                stream.Dispose();
                bitmap.UriSource = null;
                bitmap = null;
                stream = null;
            }

            GC.Collect();
            GC.Collect();
            GC.WaitForPendingFinalizers();
        }

Let me know if you are still seeing your issue if you change to this.

weltkante commented 5 years ago

@stevenbrix you probably meant

GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();

shouldn't need to call GC.Collect twice in a row, without arguments it should run a full GC of all generations (doc) so one call is enough. If anything you need to call it a second time after WaitForPendingFinalizers to collect objects which had a finalizer pending after the first call.

stevenbrix commented 5 years ago

@weltkante good to know! Honestly, I've always taken a sledgehammer approach and just do whatever seems to actually get GC to run 😂

I don't see any different behavior between the two, I still have to click the handler twice. Oddly enough, if I remove one of the GC.Collect() calls, then no matter how many times I click the handler, the memory stays allocated. Do you see something different?

weltkante commented 5 years ago

No I don't see anything obvious, unless you look at external measurements like process memory usage. The .NET runtime won't release GC'ed memory immediately to the OS in case it can reuse it for more allcoations. Eventually the heuristics will hand it back though if its not needed.

The VS diagnostic tool should report the cleanup immediately after the first click I think, unless it is also looking at process memory usage and not managed memory, but then it would be a VS tooling issue and not a .NET Core issue. In either case you can force a memory snapshot after the first click and look who is still keeping references to the stream. If you don't find any reference to your stream in the memory snapshot then its been collected and memory is ready for reuse. I can take a quick look when I'm at my VS 2019 machine tomorrow.

Anyways, this isn't a WPF bug regardless of all this, if I read this right OP is allocating a stream and then not even using it for the image. WPF can't do anything about that, it doesn't even have a reference to the stream to hold on to.

stevenbrix commented 5 years ago

Agreed that this isn't a WPF issue, although now I'm more interested in trying to understand the behavior I'm seeing. So if I take a snapshot of the heap before I click the button, and after I click the button once I see the difference I'd expect, which confirms your hypothesis that VS is showing process memory. I'd be curious to know what causes the CLR to free the heap memory allocated in the process.

image

I'm going to close this since this isn't an actual WPF issue. @Deepak211 let me know if you have any questions/concerns with this resolution. Thanks!

weltkante commented 5 years ago

I'd be curious to know what causes the CLR to free the heap memory allocated in the process.

Given the observations for this scenario I'd guess one trigger is having passed several full GCs without needing the memory. I'm sure there are other conditions and it probably behaves differently in other situations, these are just heuristics trying to optimize for the common scenarios. Note that the OS is very efficient in paging out unused memory on the fly, so its not very important to hand back memory immediately. Thats why there exist dedicated measures for "managed memory usage" and dedicated tools to inspect the managed heap, because the memory management can mask real memory leaks or look like leaks when there aren't any.

Adding some more info for the OP how to differentiate real memory leaks:

One thing which is typical for memory leaks is that the memory usage keeps growing when repeating the operation. This was not tested here, if you'd close and recreate the window repeatedly eventually the memory usage should stop growing. What you are observing for a single instance of the operation is just the .NET runtime keeping some memory reserved anticipating you to create more large objects. Without a continuously growing memory usage you can't speak of a leak.

Sometimes its really hard to detect small leaks this way because the caching behavior of the .NET runtime will mask small leaks (memory can go up slowly for hours and then suddenly drop a bunch so you can't identify trends very well), so an alternative way is to do object counts with memory snapshots. Repeat your operation you think of leaking and take snapshots every 10 or some rounds, if the objects counts of live object only keeps growing then you also have a leak.

Deepak211 commented 4 years ago

Hi @stevenbrix ,

I would like to reopen this issue as it is not resolved with the suggestions you have given to us. Kindly let me know on the below questions.

  1. Calling GC.Collect(); twice is not resolving the issue. Can you please confirm whether you have tested in a 32-bit environment?

  2. You have said that it this not a WPF issue. What is the exact cause for the issue? Is that CLR related issue? and If so, where should we report this issue to get solution for this?

weltkante commented 4 years ago

Calling GC.Collect(); twice is not resolving the issue. Can you please confirm whether you have tested in a 32-bit environment?

You have said that it this not a WPF issue. What is the exact cause for the issue? Is that CLR related issue? and If so, where should we report this issue to get solution for this?

Please re-read the history of the discussion, if I remember right the conclusion was that there wasn't a leak in the repro code given in the issue and you were just observing normal behavior. Also you might have been looking at the wrong numbers, the memory was eligible for reuse so its "free" and not "leaked". It is perfectly normal for a process to reserve some memory in anticipation of future needing that memory, the OS can handle that by paging out such memory when it isn't used, it isn't blocking other processes from using the physical RAM.

If you have an actual leak to be looked at you need to update the repro to actually represent that leak, the given code does not leak anything the last time I looked at it. I have described above (in a previous response) how to test for real leaks, for example memory usage (or object count if you do snapshots) needs to keep growing if you repeat the operation. Otherwise you are just observing a cache doing its work.

Deepak211 commented 4 years ago

@weltkante , The issue that I have reported not about "leak". I am unable to force clearing the used memory using Garbage collection. As per your advice, I consider this as a normal behavior of the process in a 32-bit environment.

It is perfectly normal for a process to reserve some memory in anticipation of future needing that memory, the OS can handle that by paging out such memory when it isn't used, it isn't blocking other processes from using the physical RAM.

CompulsiveCode commented 3 years ago

I also feel something isn't quite working correctly here with GC and BitmapImage, but I'm not sure it's anything to do with 32 bit vs 64 bit. I think the 32 bit memory limit just makes the problem easier to produce.

My attached example simply loads and disposes of images (or tries to) until it crashes. In this case the images are roughly 4000x3000 pixels (a 2.4 MB JPG file).

Running in 'Prefer 32 bit' with no GC, my application crashes due to Out Of Memory after about 30 images.

Running in 'Prefer 32 bit' with GC.Collect in the main loop, my application consumes 30-60 MB of RAM no matter how many images are processed.

Running in 64 bit mode with no GC, my application consumes: 1000 images = 8 gigs of RAM 3000 images = 10 gigs of RAM 5000 images = 11 gigs of RAM 6000 images = 12+ gigs of RAM, and crashes due to Out Of Memory (or other random errors). At the same time, Google Chrome also crashed due to Out Of Memory (while I was writing this).

If this was pure memory leak, I would expect memory consumption at 6000 images to be roughly 6x that of the 1000 image test. But for the most part memory consumption peaks at 11 GB (all of the available RAM) and stays there until it randomly runs out of physical RAM.

Running in 64 bit mode with GC.Collect, my application consumes 30-60 MB of RAM no matter how many images.

I was under the impression that GC.Collect was almost never required, but perhaps opening and closing thousands of large images is an exception to that rule. I also didn't think my lack of GC.Collect could cause other processes like Chrome to run out of memory, but Chrome crashed due to Out Of Memory while I was testing my sample code. It seems odd that fully managed code would required GC.Collect. I've attached a small (50 lines) sample that will crash without GC.Collect wpfTestMem.zip

weltkante commented 3 years ago

You are allocating these images in a tight loop, WPF images are external resources and not collected by the GC alone, it needs cooperation from the native WPF components. If you are in too tight of a loop then you might not give the WPF infrastructure enough time to actually release the image resources.

This is considered acceptable because real world applications that work in tight loops without returning to the dispatcher are actually a very bad thing to do, it causes the UI to become unresponsive, so the framework doesn't go out of its way to support it. WPF is not a general-purpose framework you can use for arbitrary "offline" imaging operations, its first and foremost a UI framework. That means operations need to be short and responsive and return to the dispatcher soon.

That said, responding to a closed issue will only notify people who worked on it previously, so it doesn't guarantee the attention of the team. Generally, if you have a new and different scenario you want to open a new issue so people can look at to determine if you are doing something wrong, or if there is something they can tweak in the WPF implementation to support your scenario better.

In this case I'm pretty sure you are using WPF incorrectly so feel free to continue the discussion here if you have further questions to me specifically.

CompulsiveCode commented 3 years ago

Thank you for the response. I started with this closed thread because it was somewhat recently updated (after being closed), and when I started working on this issue it seemed possibly related.

It's true that my example is a tight loop, but I think it could have just as easily been in a background thread.

Indeed, I can almost guarantee that I am 'using WPF incorrectly'. Admittedly, I rarely use WPF, but couldn't this scenario happen for a UI displaying hundreds of image thumbnails?

Unfortunately, as far as I know, the BitmapImage class is the only .Net imaging class with native CMYK support (which ironically has nothing to do with UI presentation other than displaying PDFs that might use CMYK colors for printing) and proper BitmapEncoder classes for saving TIFs and JPGs in CMYK format. It can handle images that GDI+ cannot.

Ultimately, I can add some GC.Collect lines in my code and get the functionality that I require from .Net.

In fact, just to test an ugly theory, I imported System.Windows.Forms into my WPF app and called DoEvents in the loop, and it also seems to provide the framework with an opportunity to free the memory. This was perhaps a spiteful response to the problem.

In my mind, I assumed that the framework would force a garbage collection sychronously before resorting to an OutOfMemory exception. I never read this anywhere. I just made it up in my mind, in my assumptions about how the garbage collector should work.

Anyway, I appreciate your response. I'm just a developer, trying to use the .Net framework to accomplish a task, even if that task isn't the way it was meant to be used.

weltkante commented 3 years ago

It's true that my example is a tight loop, but I think it could have just as easily been in a background thread.

WPF does not support imaging operations in a background thread (which IMHO is a major problem but thats unrelated to this thread). You can fake it by letting your background thread act as a secondary UI thread, but images loaded there can't easily be transferred to the primary UI thread.

I imported System.Windows.Forms into my WPF app and called DoEvents in the loop, and it also seems to provide the framework with an opportunity to free the memory.

No it doesn't. Or, well it does, in the sense that you give up control of the tight loop and let the dispatcher take over. So basically you "fixed" your tight loop by adding small pauses, but in a way that is more bad programming practice.

That just proves my point that the GC works fine and your blocking of the UI thread is what causes the issue. The GC collects the image and the finalizer tells WPF to clean up the native resources associated with that image, but WPF doesn't get the chance to clean up the native memory.

I think your conceptual problem is that the GC isn't managing the memory. WPF images are tiny wrapper objects around native objects, so the GC can't do much about cleaning them up, except determining they are ready for cleanup. WPF then has to finish the job by calling the native code, but you need to give it a chance to do so.

but couldn't this scenario happen for a UI displaying hundreds of image thumbnails?

WPF imaging is designed for UI displaying images, not for image processing. You'd want to virtualize the images and only load whats on screen into WPF images. WPF isn't great at loading hundreds of images and the performance gets really bad, mostly because everything is forced going through the UI thread. Also for thumbnails you'd not use these image sizes. In fact if you load/display several thousand images of that size you are doing something wrong. These scenarios need specialized coding, which is not a problem specific to WPF, other general-purpose libraries also will struggle with that amount of data if you don't specifically code for keeping the memory footprint down. These are not memory leaks.

All in all I repeat my point from above, WPF is not an imaging library, and if you're using it as such you're quickly running into its limitations. If the dimensions and quantities you specified are your requirements you'll need a dedicated imaging library or very specific coding, naively loading the images and hoping the framework will figure it out won't do. You can still use WPF for displaying the images, but you need to preprocess your thumbnails differently, to reduce the memory footprint.

miloush commented 3 years ago

I do have to side with @CompulsiveCode here. There is nothing wrong with processing images and you do not need a dispatcher at all for this, or any UI to begin with ~(BitmapSources are not even DispatcherObjects)~.

I consider it a bug that the wrappers hold native resources but are not disposable, which would be the easiest way to solve the situation.

The GC collects the image and the finalizer tells WPF to clean up the native resources associated with that image, but WPF doesn't get the chance to clean up the native memory.

This is a bit misleading because it's GC who is in charge of running the finalizers, and it doesn't get a chance to. Good luck GC.Collect is working for @CompulsiveCode, in my experience GC.WaitForPendingFinalizers is needed too.

weltkante commented 3 years ago

There is nothing wrong with processing images

I'm not saying you can't do image processing at all, I'm saying WPF imaging isn't designed for this, so you'll run into all sorts of trouble like not being able to clean up deterministically.

and you do not need a dispatcher at all for this, or any UI to begin with.

You need one and WPF will initialize one if it isn't present, it is a DispatcherObject and will be bound to a Dispatcher on creation. Not respecting the requirement of a dispatcher (by not providing a dispatcher message loop or blocking it for a long time) will lead to problems like reported here.

BitmapSources are not even DispatcherObjects

How did you come to that conclusion? They clearly inherit from DispatcherObject and will fail if you ever try to use them on a different dispatcher than they were created on (e.g. load them on a background thread)

Even if they were not, WPF images are based on COM and need get back to the UI thread for cleanup. Also the WPF engine may attach native resources to it that need to be cleaned up on the UI thread.

This is a bit misleading because it's GC who is in charge of running the finalizers, and it doesn't get a chance to.

It does run the finalizer, but finalizers for COM objects need to call into the apartment they originate from, so the finalizer will wait for the UI thread to become free for processing. If someone runs a loop without taking pauses that blocks the UI thread, which is a bad practice and can lead to starvation like here (and also degraded UI responsiveness if you have UI).

Good luck GC.Collect is working for @CompulsiveCode, in my experience GC.WaitForPendingFinalizers is needed too.

GC.WaitForPendingFinalizers is running a message pump in order to not deadlock, this allows COM finalizers to get back to the UI thread. Its a weaker version of DoEvents.

miloush commented 3 years ago

They clearly inherit from DispatcherObject

I do not have an explanation for that late night note of mine, please accept my apologies for such a demonstrably false claim. Thank you for pointing it out.

I'm saying WPF imaging isn't designed for this.

I would argue that WPF imaging is as designed for image processing as WIC which it is wrapping. WIC also supports MTA and explicitly mentions running on the server, making it the only .NET imaging solution that I see as supported on server-side.

I am not entirely confident that I understand how what you said explains some the behaviour we are seeing, such as the 32/64 bit differences. Otherwise I feel I need to perform some experiments before I can argue or agree further. For one, the WIC sample here does not seem to be bothered about running a message pump.

weltkante commented 3 years ago

WIC doesn't need a message pump if you use it on MTA, but WPF only implements its image objects as DispatcherObject. The fact that above sample created the images on the UI thread is enough to make the WIC objects bound to the STA apartment, requiring the finalizer to go back to that apartment. While you can create WPF images on an MTA thread that will still create a Dispatcher and the images will be unusable on other threads. If you are careful you can make offline image processing work, but like I said above, you need to explicitly code for this (for example you need to be careful not to switch to a different MTA thread if you write async code, something that would not be a problem with WIC). WPF isn't designed for this, it doesn't go out of its way to make it work seamlessly. The fact that you can make something work doesn't mean its designed for this.

As far as WIC is concerned, yes its designed for background processing, but there is no library from Microsoft which maps this 1:1 to the managed world. WPF comes close but constrains itself to UI imaging in its design (due to the forced association with a dispatcher). There are some third party nuget packages based on WIC which make different design choices.

At this point we've strayed for enough off the original issue that I'd say if you want the WPF team to investigate you should make a separate issue. I believe there is no bug or leak as WPF is doing what it is designed for, but that does not mean the design can't be extended to cover off-thread image processing better.

miloush commented 3 years ago

@weltkante Agreed. I was under the belief that making the WPF imaging classes disposable (where disposing it would free the WIC resource) would fix all the related issues we are seeing (or at least make them fixable if you explicitly code for this). You seem to be suggesting that that is not the case and it has been a while since I have been investigating this so I will have to review the options here, especially since .NET Core is now at play. There have been enough issues for this already and I am sure we will be getting more. Thanks for the discussion!

weltkante commented 3 years ago

Making them disposable helps a lot, the problem with the GC is that its running finalizers on a separate thread, so it has to switch back to the UI thread for cleanup (due to COM apartment rules). If you could dispose images on the UI thread explicitly then no finalizer or thread switch is needed (the COM calls can be made directly), you can control the lifetime of the images and avoid the out of memory conditions. Doesn't help with the other design issues around preloading images on a background thread (you still can't switch dispatchers or write async code on the thread pool) but its a step in the right direction if you want to use WPF images for image processing.