Is it valid to call GUI::SetApplicationResources with a NULL pointer?
I believe it is not a good idea to call SetApplicationResources with a null pointer, but otherwise Noesis::Shutdown reports a memory leak, so either way I need to investigate.
I noticed that some controls log an error the moment SetApplicationResources(nullptr) is called:
I can only guess, but it looks like the ancestor is no longer there but the binding still tries to convert (now empty) tag to a number.
So my question is: are things supposed to fall apart if we pull the rug out like this? Or is this some kind of bug?
I noticed that some controls log an error the moment SetApplicationResources(nullptr) is called:
For example it happens for TreeView with default theme and is caused by a binding to Tag:[NOESIS/E] 'Converter<BaseComponent>' binding converter failed to convert value 'null' (type 'BaseComponent')
Code: Select all
<Decorator x:Name="CurrentIndent" Grid.Column="0" Width="{Binding Tag, RelativeSource={RelativeSource AncestorType={x:Type ItemsPresenter}, AncestorLevel=1}}"/>
So my question is: are things supposed to fall apart if we pull the rug out like this? Or is this some kind of bug?
-
-
sfernandez
Site Admin
- Posts: 2865
- Joined:
Re: Is it valid to call GUI::SetApplicationResources with a NULL pointer?
Hi Ankor, are you planning on calling SetApplicationResource(nullptr) in the middle of your application, or are you talking about the Noesis Shutdown, where we set it to null to release all the memory?
Re: Is it valid to call GUI::SetApplicationResources with a NULL pointer?
In the middle.
Basically the main UI is suspended (update is not called), while a loading screen is displayed and that loading screen may have its own set of resources.
So it goes like this:
Main UI start: SetApplicationResources(main resources);
Main UI suspend: SetApplicationResources(NULL);
Loader start: SetApplicationResources(its own resources);
Loader completed: SetApplicationResources(NULL);
Main UI resume: SetApplicationResources(main resources);
I have doubts whether it is valid to do things like this. I'm not even sure it has any benefits, so if you say it doesn't, I'll be happy to look into refactoring this part :)
Now as I understand resources have to be set to NULL before Shutdown, and that's why I was seeing a memory leak when I removed this call entirely.
Basically the main UI is suspended (update is not called), while a loading screen is displayed and that loading screen may have its own set of resources.
So it goes like this:
Main UI start: SetApplicationResources(main resources);
Main UI suspend: SetApplicationResources(NULL);
Loader start: SetApplicationResources(its own resources);
Loader completed: SetApplicationResources(NULL);
Main UI resume: SetApplicationResources(main resources);
I have doubts whether it is valid to do things like this. I'm not even sure it has any benefits, so if you say it doesn't, I'll be happy to look into refactoring this part :)
Now as I understand resources have to be set to NULL before Shutdown, and that's why I was seeing a memory leak when I removed this call entirely.
-
-
sfernandez
Site Admin
- Posts: 2865
- Joined:
Re: Is it valid to call GUI::SetApplicationResources with a NULL pointer?
Could you just call SetApplicationResources with the new set of resources?
Calling SetApplicationResources with null has extra meaning for us, because as I mentioned we release a few internal structures in preparation for shutdown. Anyway, there is no real problem in calling it in the middle of your application, but in your case I don't think it is necessary, you can just swap the resources when needed.
Code: Select all
Noesis::GUI::SetApplicationResources(main_resources);
...
Noesis::GUI::SetApplicationResources(loader_resources);
...
Noesis::GUI::SetApplicationResources(main_resources);
Re: Is it valid to call GUI::SetApplicationResources with a NULL pointer?
Ok, understood.
I'll change our code to set resources to null only after closing everything before shutdown.
Thank you!
I'll change our code to set resources to null only after closing everything before shutdown.
Thank you!
Re: Is it valid to call GUI::SetApplicationResources with a NULL pointer?
Let me clarify that this is not necessary because it is done automatically at shutdown time. If you are getting leaks because you are not clearing the global dictionaries, that's a bug.I'll change our code to set resources to null only after closing everything before shutdown.
Re: Is it valid to call GUI::SetApplicationResources with a NULL pointer?
Hmm... our bug or yours? :)Let me clarify that this is not necessary because it is done automatically at shutdown time. If you are getting leaks because you are not clearing the global dictionaries, that's a bug.
Without clearing the resources before Shutdown I see this:
The number of bytes may be larger if I do more actions in the application.[NOESIS/I] Noesis Shutdown
[NOESIS/W] Memory leaks detected: 1024 bytes
Also, looks like null was irrelevant to my issue anyway.
Setting non-null resources (even created from the same xaml) also seems to cause binding errors in the process.
For now I've just silenced them for this specific case and in the future I hope we'll get rid of this resource swapping anyway.
Re: Is it valid to call GUI::SetApplicationResources with a NULL pointer?
We clear resources at shutdown time. Are you able to reproduce this in one of our examples? Something unexpected (like a circular reference) is happening here.Hmm... our bug or yours? :)
Without clearing the resources before Shutdown I see this:The number of bytes may be larger if I do more actions in the application.[NOESIS/I] Noesis Shutdown
[NOESIS/W] Memory leaks detected: 1024 bytes
Re: Is it valid to call GUI::SetApplicationResources with a NULL pointer?
I see. I will review our code and try with your examples when I have time.
Thanks!
Thanks!
Re: Is it valid to call GUI::SetApplicationResources with a NULL pointer?
So, I figured out the memory leak issue. It is indeed a bug in our code, but a very odd one, so I suggest you to take a look into it on your side as well or at least document it.
First, while picking apart literally everything inside our resources, I noticed the memory leak only happens if there are TextBlock or TextBox objects in the resources (e.g. in some control templates). The more instances of TextBlock/Box there are the larger the leak.
Then, while trying to remove as much code as possible between load and Shutdown, I noticed we have following lines that are absent from your examples:
This code wasn't written by me, but I never paid attention, because it looked like a reasonable thing to do. Turns out it isn't :)
Setting Font provider to null like this causes a memory leak on Shutdown if I have TextBlocks inside resources!
It is also reproducible in your examples, simply by adding GUI::SetFontProvider(0); before Shutdown.
Anyway, this was an interesting thing to debug, but it isn't even my original issue :)
First, while picking apart literally everything inside our resources, I noticed the memory leak only happens if there are TextBlock or TextBox objects in the resources (e.g. in some control templates). The more instances of TextBlock/Box there are the larger the leak.
Then, while trying to remove as much code as possible between load and Shutdown, I noticed we have following lines that are absent from your examples:
Code: Select all
Noesis::GUI::SetTextureProvider(0);
Noesis::GUI::SetXamlProvider(0);
Noesis::GUI::SetFontProvider(0);
Setting Font provider to null like this causes a memory leak on Shutdown if I have TextBlocks inside resources!
It is also reproducible in your examples, simply by adding GUI::SetFontProvider(0); before Shutdown.
Anyway, this was an interesting thing to debug, but it isn't even my original issue :)
Who is online
Users browsing this forum: Tohrwarneth and 1 guest