Memory Leak with TraceSystem

[marubinotto]

Hi,

This problem was posted in 2007, but it seems it has not been resolved
because I encountered the same memory leak problem with h2-1.2.147.

http://groups.google.com/group/h2-database/browse_thread/thread/6637ce45682bf209/

I checked out the code and found a bug:

http://code.google.com/p/h2database/source/browse/trunk/h2/src/main/org/h2/message/TraceSystem.java

public synchronized Trace getTrace(String module) {
if (module.endsWith("]")) {
new Trace(writer, module);
}

"return" is missing.

return new Trace(writer, module);


Best regards,
Daisuke

[Thomas Mueller]

Hi,

Thanks a lot! The memory leak was plugged for some time (not fixed
correctly; but it wasn't a memory leak because the trace system didn't
use a regular hash map). I messed up when I fixed it, because I didn't
add a test case.

Now I added a test case to detect the memory leak, and I fixed the
bug. This will be available in the next release.

Thanks a lot again for your help!

Regards,
Thomas

> --
> You received this message because you are subscribed to the Google Groups "H2 Database" group.
> To post to this group, send email to h2-da...@googlegroups.com.
> To unsubscribe from this group, send email to h2-database...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/h2-database?hl=en.
>
>

[marubinotto]

> but it wasn't a memory leak because the trace system didn't use a regular hash map

In my environment, the size of TraceSystem.traces is growing until an
OutOfMemoryError is thrown. With a memory analyzer, I can see a large
number of Trace objects accumulated with keys(modules) like
"jdbc[number]".

Strangely, I didn't encounter this problem when I used version 1.1.x.

I checked out the code again and found out the reason:

In version 1.1.115, TraceSystem uses a SmallLRUCache as a trace's
cache:

http://code.google.com/p/h2database/source/browse/tags/version-1.1.115/h2/src/main/org/h2/message/TraceSystem.java?r=3283

But in trunk, TraceSystem seems to use a plain HashMap created via
New.hashMap(16):

http://code.google.com/p/h2database/source/browse/trunk/h2/src/main/org/h2/message/TraceSystem.java

I think this causes the memory leak in version 1.2.x.

Best regards,
Daisuke

[marubinotto]

Hi,

In trunk, the trace system seems to use a plain HashMap instead of a
SmallLRUCache as I wrote in the previous mail (according to the change
log of the repository, the container of traces changed on Nov 12,
2010). Isn't there any risk of a memory leak?

On Jan 12, 3:57 pm, marubinotto <daisuke.marubino...@gmail.com> wrote:
> > but it wasn't a memory leak because the trace system didn't use a regular hash map
>
> In my environment, the size of TraceSystem.traces is growing until an
> OutOfMemoryError is thrown. With a memory analyzer, I can see a large
> number of Trace objects accumulated with keys(modules) like
> "jdbc[number]".
>
> Strangely, I didn't encounter this problem when I used version 1.1.x.
>
> I checked out the code again and found out the reason:
>
> In version 1.1.115, TraceSystem uses a SmallLRUCache as a trace's
> cache:
>

> http://code.google.com/p/h2database/source/browse/tags/version-1.1.11...

>
> But in trunk, TraceSystem seems to use a plain HashMap created via
> New.hashMap(16):
>

> http://code.google.com/p/h2database/source/browse/trunk/h2/src/main/o...

[Thomas Mueller]

Hi,

> In trunk, the trace system seems to use a plain HashMap instead of a
> SmallLRUCache as I wrote in the previous mail (according to the change
> log of the repository, the container of traces changed on Nov 12,
> 2010).

I know.

> Isn't there any risk of a memory leak?

Not any more, because the list of entries in the map is now limited.
It was always supposed to be limited, so the SmallLRUCache should
never have been used. It was used, so the actual bug (the forgotten
return) didn't have an effect.

The real problem is that there was no test case for the memory leak so far.

Regards,
Thomas

[marubinotto]

Thanks for the reply.

> > Isn't there any risk of a memory leak?
>
> Not any more, because the list of entries in the map is now limited.
> It was always supposed to be limited, so the SmallLRUCache should
> never have been used. It was used, so the actual bug (the forgotten
> return) didn't have an effect.

OK. I understand.


Best regards,
Daisuke