Universalmediaserver: Memory leak during scan

[SubJunk]

Every time a file scan is done (navigation/share tab, button near the bottom) it's not freeing enough memory.

I have reproduced it in latest master (5453878a34d88c91ebfdc0e4d851d50d6b393329) and in 7.0.0 (ef294e4)

I'm not sure how long the bug has existed but in 7.0.0 rescans are triggered often so it makes it more visible than before.

[SubJunk]

I made https://github.com/UniversalMediaServer/UniversalMediaServer/pull/1186 initially to see if findbugs would see anything related to this but the bug is the same in that branch

[Nadahar]

@SubJunk I assume that caching is enabled when you experience this? Have you tried without caching and confirmed that it is unrelated to caching?

Memory leaks are supposed to be "impossible" to create in Java. That's not true in reality, but the traditional "alloc" but fail to "free" memory leak is. Simply keeping references to objects no longer needed is enough that the GC won't free them, so this, while not technically a memory leak because they are still referenced, will have the same effect. I also suspect that the lack of thread safety can lead to references being "messed up" so that they are either "lost" or are stuck in a state where the GC think they are still in use. I don't know enough about the details of the GC to know for certain, this is just my general thoughts.

What is the message of the OutOfMemoryError? Java Heap Space?

It might be that the best approach is to start with some profiling with something like VisualVM.

[SubJunk]

@Nadahar scanning is only possible when cache is enabled.

Yeah my suspicion is like you said, that we are keeping references to objects that are no longer needed.

I haven't seen an error yet because the memory use keeps going until it freezes

I've been going backwards in time to see when it appeared and I've found a version that doesn't have the memory creep, 4.4.0. I'll keep narrowing down when it first appeared and may be able to find a fix that way instead of profiling.

[atamariya]

Few things to check:

• A RootFolder is created for each renderer/browser instance with the corresponding file tree. Do you have multiple renderers?
• MediaLibrary creates a new RealFile for each scan. These are duplicates of the files found during folder scan but held in the memory at the same time.

[SubJunk]

Good notes @atamariya

The first released version it happens in is 5.0.0-a1 so now I'm going to look at the differences between 4.4.0 and 5.0.0-a1 to see what could have caused it.

[Nadahar]

@SubJunk It depends on how you define "scanning". When you enter folders, they will be "scanned" also without caching. While the topmost code is different, most of the code run is the same. Knowing if the same happends when caching is off could help point to at "what level" the issue is. I'm seeing quite hefty memory utilization when I do testparsing in #1061 (1 GB and more isn't uncommon), but I have a lot of RAM so I've just figured that the GC isn't triggered because there are a lot more available. The GC is triggered from time to time though, you can see it in a sharp drop in memory use, but just like you describe it seems to always end up a little higher that last time each time it runs. It could very well be the same phenomena.

I've found one insane bug that might be related (while working on #1061), although it's older than 4.4.0 so it's probably not the bug you're looking for. It's the simple fact that there's a method DLNAMediaInfo.finalize() that is called after parsing. I've simply renamed it to postParse() in #1061 for now. finalize() is a special method in Java which is tightly bound to the GC and should never be called by code. GC will call finalize() at the time when the instance is actually being destroyed, and one can use that to put special "cleanup code" in cases where there's such a need.

Even though this method isn't overridden (so it's not truely the same method), it might hide the real finalize() method so that DLNAMediaInfo instances aren't properly destroyed. Regardless of if this is related to the leak, this is very bad practice and shouldn't be allowed to stay the way it is.

[SubJunk]

I've tracked the bug down to ff11feb
Any idea why that would cause memory usage to keep growing on every scan @SharkHunter ?

[SubJunk]

It looks to me like everything from that commit has been removed except for the part that creates the cache. I'm pushing a fix that removes that since there is no point populating the cache if we don't read from it anymore. The fix is at https://github.com/UniversalMediaServer/UniversalMediaServer/pull/1189/files

[Nadahar]

@SubJunk I'm not really sure what is the "main" problem here, it's obvious that the references are being kept by RendererConfiguration.renderCache.

While this isn't the primary issue, it's obviously a problem with a cache that have no expiration. Once something gets into that cache, it will live "forever". But that's not enough to explain what happens.

It says explicitly in the HashMap documentation that they are NOT thread safe. As usual, this is ignored here.

[Nadahar]

How do you figure @SubJunk?

defaultRenderer.cachePut(child);

is still being called from DLNAResource.addChildInternal(DLNAResource child)

[SubJunk]

Yes exactly, we are populating that cache but never reading from it, so there is no point in having it around.

[Nadahar]

@SubJunk Ok, I misunderstood you. When you said "creates the cache" I though you meant new HashMap<>(). It doesn't seem like it's being read, so if global id has replaced it there's absolutely no reason to keep it around.

[SubJunk]

Cool. It has been removed in the fix I posted above so if you review that it can be merged. Ah it's satisfying fixing 2.5 year old bugs ;)

[Sami32]

@SubJunk You already fixed it in 7.0.0 branch, isn't it ?

[Nadahar]

@Sami32 It's not "fixed" as it's a "design problem", but he has limited the extra impact it has in 7.0.0 by rescanning only the files that need to be rescanned. The memory leak itself is there both in master and in 7.0.0.

[SubJunk]

Yeah, like Nadahar said. If you click manual scan a lot of times it will still happen, but it's not likely anyone will do that

[Nadahar]

It will happen without using the manual scan as well, it will happen whatever way scanning is done, either by the automatic filewatch functions i 7.0.0, folder browsing from a renderer or manual scan. The point is that by making the automatic filewatch functions not scan so much unnecessary files as it previously did, the amount of memory lost to this is reduced. But, the bug is still very real and will slowly eat memory so that UMS isn't able to run for a long time without restart.

When I initially found UMS that was my goal, to let it run 24/7 on a server that I "never" had to touch. After testing that quite a lot I gave it up, even though I think that's the way a "server" should be run. I don't think this bug is the only problem that prevents this, but it's certainly contributing. I still let an instance run on one of my VMs 24/7, and if I haven't been on that VM for weeks/months it has always crashed, is hanging or have stopped responding to requests even though no renderers use it regularly.

I think the issue should remain open until this is fixed, if ever.

[SubJunk]

That's still my goal too

[SubJunk]

This has been fixed during scanning by just not using the global IDs