Memory Leaks

• Key: DIRMINA-41
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Trustin Lee
• Reporter: elliot schlegelmilch
• Affects Version/s: 0.7.0, 0.7.1
• Fix Version/s: 0.7.2

Description

Discovered by adding 10,000 users and searching repeatedly. Ran in profiler and compared mid search to post search to find many objects aren't getting gc'd.

Comments

[Trustin Lee]

I looked at the patch, and it looks very simple. But I have a question before I apply this patch. Please look at this part:

- Stack[] bufferStacks = buf.isDirect()? directBufferStacks : heapBufferStacks;
- Stack stack = bufferStacks[ getBufferStackIndex( bufferStacks, buf.capacity() ) ];
+ Stack stack;
+ if (buf.isDirect())
+ {
+ stack = directBufferStacks[ getBufferStackIndex( directBufferStacks, buf.capacity() ) ];
+ }
+ else
+ {
+ stack = heapBufferStacks[ getBufferStackIndex( heapBufferStacks, buf.capacity() ) ];
+ }
+

I don't see any differences in this patch fragment IMHO. Any ideas?

[Trustin Lee]

I checked in the fix.

Could you review the fix and close this issue?

[David Boreham]

The code you asked about is semantically the same as the original, it was
inadvertantly left in the patch.

[David Boreham]

So, there's another leak in this same file, which has proven harder to fix.
I think it needs Trustin's brain to figure out the correct fix. For now we have
a hack in place that removes the leak but it also removes the nio byte buffer
pools too :(

The leak is caused by the fact that all nio.ByteBuffer objects are pushed
onto a stack when they are released. However, not all nio.ByteBuffer objects
used in allocate are pulled off the stacks. Specifically the two wrap() methods
just call the underlying java.nio.ByteBuffer wrap() methods. This results
in objects being allocated more often than they are freed and more and more
memory leaked onto the stacks.

I'm not sure if it's just a case of adding code to try to pull a buffer off
the stacks that can then wrap the payload, or if we need to mark the wrapper
buffers and not push them on the stack. The latter is not a good fix because
in our tests 90%+ of all the buffers were allocated via the wrap calls.

[Trustin Lee]

You're right. If users keep wrapping nio buffers, it becomes a severe memory leak. We'll have to add some property like 'managed' so that users disable MINA from releasing it. WDYT?

[Alex Karasulu]

I like the idea of a marker. You can then check isManaged() to figure out if you have to handle the release. It's a fairly quick way to fix the problem too. However if you decide to change it in the future then it might not be a backwards compat. change. Anyways it's not like we're 1.0 so ... go for it.

[David Boreham]

Yeah, I thought of that as a fix. However...what made me pause
was the fact that in invesigating the leaks Elliot instrumented
the allocs and releases, and we looked at the resulting counters.
In the case we were checking (searching for users), something
like 90% of the buffer allocations were of the wrapped type.
So I was thinking if we disable pooling for those buffers then
what value is the pooling mechanism ?

If it were known that we could re-cycle nio buffers that were
originally made with a wrap() call, then all would be well:
just try to get them from the pool first.

However, reading the Sun doc I wasn't confident that the internal
behavior is well understood. E.g. for the clear() method they
say something like : this is a bit like cleaning out the object
but not quite (nothing like knowing exactly what your code
does, eh ?!).

[Trustin Lee]

I added more documentation related with wrapping existing NIO buffers or byte arrays so that users can understand the root cause of this issue. I also provided example code that resolves this issue. I'll fix Apache ASN1 codec instead. Please look at this:

http://svn.apache.org/viewcvs.cgi/directory/network/branches/0.7/src/java/org/apache/mina/common/ByteBuffer.java?rev=171076&view=markup

[Trustin Lee]

I checked in the fix for Apache ASN1 and updated dependencies of ApacheDS.

http://svn.apache.org/viewcvs.cgi/directory/asn1/trunk/codec/src/java/org/apache/asn1/codec/mina/Asn1CodecEncoder.java?rev=171078&view=markup