Possible memory leak, can you take a look at it ?

[Bastien Billey]

Hey Seb,

I'm actually trying to resolve a memory leak, but there is some resistance. It's in our image editor, so I join you a light version of what we do.

In our app there is a button to open the image editor on a new stage. and when you close it, reopen it, close it, re open it... all the ImageEditorModel are still in memory with the image on it, so it take a lot of memory...

Maybe I do something bad in the way to open it, so if you can take a look at it it would be great. But I suspect some Jrebirth internal map who keep a reference on it.

You'll find the maven project and a screenshot of the profiler on attachments.

Thanks in advance, have a nice day.

[Sebastien Bordes]

Hi Bastien,

I will have a look tonight, and keep you informed of what should be done to fix this.

Seb

--
Vous recevez ce message, car vous êtes abonné au groupe Google Groupes "JRebirth Users".
Pour vous désabonner de ce groupe et ne plus recevoir d'e-mails le concernant, envoyez un e-mail à l'adresse jrebirth-user...@googlegroups.com.
Pour obtenir davantage d'options, consultez la page https://groups.google.com/d/optout.

--

s...@jrebirth.org - www.jrebirth.org - Blog

[Sebastien Bordes]

I've just quickly reviewed your sample project and I think that using an image as a keypart for your ImageEditorModel is really a bad idea, you should the image full name string instead.

Then you shouldn't use new Image(...) in your code, you should prefer using the JRebirth dynamic Image registry that allow to loosely link Image objects that seems to be the root cause of your memory leak.

I will propose you a patch this evening and check if there is an other error into JRebirth core classes (Did you push the project on github or just zipped it ?) .

Seb

[Bastien Billey]

Well, in the main app (not the sample) It's not an image who is given as a keypart but a Document object who contain some stuff like image uri, but also a preview Image, so maybe it come from here. And yes we always use new Image... we don't think about use JRebirth image, I'll get a try to this.

And now it's on guithub https://github.com/agonist/JrebirthTest

Thanks for your quickly answer

[Sebastien Bordes]

Dynamic images have been introduced into Paradise City (7.7.2) version with a light refactoring of previous resource engine.

final Image image = Resources.create(new LocalImage("bear.jpg")).get();

If you call 1000 times this line only one image will be loaded.

But be informed that neither LocalImage or Image Item can be used as key part like Image because hashcode method will return different values (I will fix it for ResourceParams(LocalImage) and perhaps for ResourceItem (ImageItem))

Please alos note that by default JRebirth will load Image from the images folder, it can be changed by updating the related JRebirth parameters into a 'jrebirth' properties file.

You can workaround your problem by manually unregister the ImageEditorModel (on stage closure and on stage reopen to be sure to remove from facade the model replaced into the stage)

                stage.setOnCloseRequest(new EventHandler<javafx.stage.WindowEvent>(){

                    @Override
                    public void handle(javafx.stage.WindowEvent arg0) {
                        JRebirthThread.getThread().getFacade().getUiFacade().unregister(editorKey);  // Editorkey has been refactored nito a local field
                    }
                   
                });

Note that this is a crappy workaround, the most important thing to remember is that all keyParts must implement hashCode method (I really have to add this into the the doc in bold red).

I will refactor your code tonight to do the job because several things could be done differently (ie: your service is not a real service because you are doing stuff only into JAT but never into JTP, a UI Command will be more suitable, it will remain in memory by listening a WaveType that creates a strong link with the Notifier).

I know that this small project is a reproduction of the problem you have with your real application, but I will also refactor some other parts just to demonstrate more stylish techniques.

Seb

[Sebastien Bordes]

Re,

I've just created a pull request here https://github.com/agonist/JrebirthTest/pull/1 with a some tips

Let me know if you want more explanantion or if you have any other questions

Seb

[Bastien Billey]

Thanks for all this precisions, I'll take a look at it, and tell you if I need more precisions. We have a huge app now using Jrebirth, and I think there is lot of things to improve on our app...

Thanks for your time

[Sebastien Bordes]

Don't hesitate any feedback are welcomed ! (even the worse ones)

Have you migrated to Java 8 ?

Seb

[Bastien Billey]

Yes we are full java8 since few month.

I have on question about JRebirth dynamic image, in the sample app everything work fine, but in our real case, we work with image outside the project (in the user temp directory), but JRebirth try to find it in images/ directory. So does LocalImage mean image inside the project ?

My image is not found and then I have a huge stack trace with a jvm crash at the end

15 juil. 2014-14:32:56.365 [JavaFX Application Thread] ERROR o.j.a.c.resource.image.ImageBuilder - Image : C:\WORKSPACE\app\target\temp\wallpaper-793095_1405427535723.jpg/wallpaper-793095 not found into base folder: images/

The nex line appear thousands time

15 juil. 2014-14:32:57.050 [JavaFX Application Thread] ERROR o.j.a.c.resource.image.ImageBuilder - Image : image/NotAvailable.png.png not found into base folder: images/

15 juil. 2014-14:32:57.050 [JavaFX Application Thread] ERROR o.j.a.c.resource.image.ImageBuilder - Image : image/NotAvailable.png.png not found into base folder: images/

#

# A fatal error has been detected by the Java Runtime Environment:

#

#  EXCEPTION_ACCESS_VIOLATION (0xc0000005) at pc=0x000000005b936c50, pid=3832, tid=7028

#

[Sebastien Bordes]

Arggg you are using a way not supported using an absolute local image path....

But you ca

[Sebastien Bordes]

Arggg you are using a way not supported by using an absolute local image path....

Curently Local and we image are supported, and local ones are prefixed by a JRebirth parameter

So you can try to fix it by updating the default JRebirth Parameters : imagesFolder=images => to imagesFolder=C:\Temp but it will be for all images.

I will add a new AbsoluteImage that will support this for 7.7.3 rebound.

I come back to you when it's done

 

[Sebastien Bordes]

Other thing I missed into my first reply.

The stackoverflow occurred when the resource engine cannot find the default image (it could be improved but as it's non-operationnal beahvior, any exception will fit)

Your jrebirth.properties files probably contains outdated values:

You should have:
notAvailableImage=NotAvailableImage||png
imagesFolder=images

you probably have:
notAvailableImage=NotAvailable.png

imagesFolder=image

Seb

[Sebastien Bordes]

Hi Bastien,

I pushed some changes and build a 7.7.3-SNAPSHOT that include the possibility to add several [images|fonts|style] path separated by a comma. like this => imagesFolder=images,C:\Temp

Before the final release I will include a global varenv resolver to support %TMP% as an example or %HOMEDRIVE%%HOMEPATH% by using ${tmp} and ${homedrive}${homepath} into parameter values.

Let me know if the first fix works for you.

Seb

[Bastien Billey]

Hey, thanks I'll check that later and give you feedback.

Another things we found, is in the example (you can take the refactored code you did), the ImageEditorModel is never garbage collected when is close.

After some investigations we found that come from the AbstractFXMLObjectModel. There is a listen on JRebirthWaves.SHOW_VIEW and JRebirthWaves.HIDE_VIEW, but you never do unlisten for those wave. Cause of that the ImageEditorModel is never garbage collected.

As ugly fix we can add :

        unlisten(JRebirthWaves.SHOW_VIEW);

        unlisten(JRebirthWaves.HIDE_VIEW);

on initModel() but that not really cool. So for the 7.7.3 it can be really cool if this bug can be fixed.

Thanks in advance

Bastien

[Sebastien Bordes]

You are right and there is probably other location to check

I will add it to the 7.7.3 roadmap

[Sebastien Bordes]

This issue will be resolved in two times:

For 7x branch these 2 wave types will be kept as-is and some custom methods will be added to deal with their potential leak
For 8.x branch these 2 default wave types will be handled as Behavior (new concept introduced into 8.x to easily reuse code dynamically)

So for 7.x, the following changes are going to be pushed:

-> fxml model doesn't have a view a doesn't offer the release ability when the fxml root node is not attached to the scene => it will be fixed to call the release method

-> the release method will unlisten all wave type previously listened

-> the sample project will updated with core-7.7.3-SNAPSHOT to detect when the stage is closed and to detach the fxml root node of the ImageModel

Seb

[Sebastien Bordes]

7.7.3-SNAPSHOT has been  built last night, it solves all troubles.

I have also submit a pull-request to the sample project to reflect changes and fixed features.

When a model's root node is detached from its parent, the Model.release method is called (this wasn't called for fxml model, bug fixed)
 and unlisten all wave type (new convenient feature).
 

You can manually call this method if you want to release the model, but take care of the root node strong link that can retain all objects.

Please note that you can use @OnRelease on any method you want to be called during this phase.

JRebirth Parameters supports environment variable resolution.

Several Image, Style, Fonts folders are supported (separated by a coma).

LocalImage has been split to RelImage (with Image folder base) and AbsoluteImage (without Image folder base)

During next week I will add some documentation before performing the final release of the 7.7.3, obviously all these changes will be ported on 8.x branch

Seb

--

Vous recevez ce message, car vous êtes abonné au groupe Google Groupes "JRebirth Users".
Pour vous désabonner de ce groupe et ne plus recevoir d'e-mails le concernant, envoyez un e-mail à l'adresse jrebirth-user...@googlegroups.com.
Pour obtenir davantage d'options, consultez la page https://groups.google.com/d/optout.