AnKor
Topic Author
Posts: 17
Joined: 22 Jul 2022, 16:14

Is it valid to call GUI::SetApplicationResources with a NULL pointer?

16 Dec 2022, 17:08

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:
[NOESIS/E] 'Converter<BaseComponent>' binding converter failed to convert value 'null' (type 'BaseComponent')
For example it happens for TreeView with default theme and is caused by a binding to Tag:
<Decorator x:Name="CurrentIndent" Grid.Column="0" Width="{Binding Tag, RelativeSource={RelativeSource AncestorType={x:Type ItemsPresenter}, AncestorLevel=1}}"/>
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?
 
User avatar
sfernandez
Site Admin
Posts: 2865
Joined: 22 Dec 2011, 19:20

Re: Is it valid to call GUI::SetApplicationResources with a NULL pointer?

16 Dec 2022, 19:03

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?
 
AnKor
Topic Author
Posts: 17
Joined: 22 Jul 2022, 16:14

Re: Is it valid to call GUI::SetApplicationResources with a NULL pointer?

16 Dec 2022, 21:03

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.
 
User avatar
sfernandez
Site Admin
Posts: 2865
Joined: 22 Dec 2011, 19:20

Re: Is it valid to call GUI::SetApplicationResources with a NULL pointer?

19 Dec 2022, 10:45

Could you just call SetApplicationResources with the new set of resources?
Noesis::GUI::SetApplicationResources(main_resources);
...
Noesis::GUI::SetApplicationResources(loader_resources);
...
Noesis::GUI::SetApplicationResources(main_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.
 
AnKor
Topic Author
Posts: 17
Joined: 22 Jul 2022, 16:14

Re: Is it valid to call GUI::SetApplicationResources with a NULL pointer?

19 Dec 2022, 11:14

Ok, understood.
I'll change our code to set resources to null only after closing everything before shutdown.

Thank you!
 
User avatar
jsantos
Site Admin
Posts: 3747
Joined: 20 Jan 2012, 17:18
Contact:

Re: Is it valid to call GUI::SetApplicationResources with a NULL pointer?

20 Dec 2022, 17:05

I'll change our code to set resources to null only after closing everything before shutdown.
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.
 
AnKor
Topic Author
Posts: 17
Joined: 22 Jul 2022, 16:14

Re: Is it valid to call GUI::SetApplicationResources with a NULL pointer?

20 Dec 2022, 17:27

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.
Hmm... our bug or yours? :)
Without clearing the resources before Shutdown I see this:
[NOESIS/I] Noesis Shutdown
[NOESIS/W] Memory leaks detected: 1024 bytes
The number of bytes may be larger if I do more actions in the application.

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.
 
User avatar
jsantos
Site Admin
Posts: 3747
Joined: 20 Jan 2012, 17:18
Contact:

Re: Is it valid to call GUI::SetApplicationResources with a NULL pointer?

21 Dec 2022, 11:55

Hmm... our bug or yours? :)
Without clearing the resources before Shutdown I see this:
[NOESIS/I] Noesis Shutdown
[NOESIS/W] Memory leaks detected: 1024 bytes
The number of bytes may be larger if I do more actions in the application.
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.
 
AnKor
Topic Author
Posts: 17
Joined: 22 Jul 2022, 16:14

Re: Is it valid to call GUI::SetApplicationResources with a NULL pointer?

21 Dec 2022, 12:38

I see. I will review our code and try with your examples when I have time.
Thanks!
 
AnKor
Topic Author
Posts: 17
Joined: 22 Jul 2022, 16:14

Re: Is it valid to call GUI::SetApplicationResources with a NULL pointer?

22 Dec 2022, 16:05

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:
Noesis::GUI::SetTextureProvider(0);
Noesis::GUI::SetXamlProvider(0);
Noesis::GUI::SetFontProvider(0);
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 :)

Who is online

Users browsing this forum: Tohrwarneth and 1 guest