Possible memory leak?

[rwalkerands]

We seem to be increasingly a "victim of our own success" in deploying an instance of elda-lda.

We now semi-regularly have days in which a user will deploy multiple new or updated spec files, triggering a reload of all spec files (we now have over 100 spec files, each with 15 or so endpoints).

In the last week I've twice had to restart our Tomcat because of a serious memory leak that seems to be related to the reloading of the spec files.

Yesterday I used VisualVM to try to track down the problem. There seem to be multiple instances of the configuration, one created each time the reloading of the spec files is triggered.

Each copy of the configuration holds large HashMaps containing (for example) all the endpoint mappings.

The attached screenshot is from VisualVM. It is filtered by class name limited to "com.epimorphics...". Note the 22 instances of DefaultRouter and SpecManagerImpl (plus two other related classes). 22 seems to be about the number of times the spec files have been (re)loaded. I.e., I did a heap dump, saw that the numbers were 21, then triggered a reload by editing one of the spec files, and then did another heap dump, and the numbers were then 22.

I tried to find why the code does this. In RouterRestlet.getRouterFor() there are two calls to RouterRestletSupport.createRouterFor(); the second one seems to be the one associated with the reloading of spec files.

RouterRestletSupport.createRouterFor() invokes new SpecManagerImpl() and passes the resulting object to SpecManagerFactory.set().

I note the class comment at the top of SpecManager.java that says "... The (singleton) instance of the SpecManager ...", but SpecManagerFactory.set() is not a setter! It adds the value you pass in to an ArrayList<SpecManager>.

So each time there is a reload, a new SpecManagerImpl is created, which contains a new DefaultRouter. And because these new objects are stored away in an ArrayList and never removed, they are never garbage collected.

[ehedgehog]

What version of Elda?

[rwalkerands]

Oops, sorry. We are running 1.3.20.

[ehedgehog]

Hmm. It's very plausibly a bug. But it looks like it may be a subtle bug -- I don't think it's right just to replace the managers list with a single value. Note that RouterRestletSupport addBaseFilepath(String baseFilePath) says:

/**
    Add the baseFilePath to the FileManager singleton. Only do it
    once, otherwise the instance will get larger on each config load
    (at least that won't be once per query, though). Just possibly
    there may be multiple servlet contexts so we add a new only only if 
    its not already in the instance's locator list.
*/

which suggetst the correct fix is to overwrite an existing SM and add a new one.

I'll continue to look at this one.

Chris

[rwalkerands]

Subtle indeed.

FWIW catalina.log has an entry "adding locator for '/usr/share/tomcat/webapps-ands/lda/'" for every config file reload.

So "only do it once" is not happening.

[ehedgehog]

Elda 1.2.23 has cleaned up the SpecManager code, replacing it with more robust and simpler code. Updating a config file discards its previous retained specs once they have been loaded and compiled, so there should be no ever-growing collection.

[rwalkerands]

Looking good in our test environments.