Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SslHandler - DirectByteBuffer - OutOfMemory #3057

Closed
Lekanich opened this issue Oct 27, 2014 · 18 comments

Comments

@Lekanich
Copy link

commented Oct 27, 2014

I tested the performance of my project with and without SSL (configuration: 25 threads that run 10 more threads (total 250) each of them refers to the server for download the file).
Test without Ssl completed without error, but the test with Ssl threw OutOfMemoryError when SslHandler trying to create the new DirectByteBuffer in his methods wrap() (Bits.reserveMemory).

System Monitor shows memory usage in 10 times higher than the same test without using Ssl.
Perhaps you need to add a method call Util.free(ByteBuffer) for sended DirectByteBuffer and thus exempt are reserved memory? Because I haven't found such place in which someone performs this action and free direct memory.

PS (In my test I send and receive over 5GB of data for test)

@Scottmitch

This comment has been minimized.

Copy link
Member

commented Oct 27, 2014

Hi @Lekanich and thanks for reaching out. We will need a few more details to narrow in on the issue:

  • What version of netty are you using?
  • What is your system configuration (mvn --version)?
  • Are you using openssl or JDK providers for SSL?
  • Where is the location in the code that the buffer is not being released?
  • Have you tried to release this buffer and see if the behavior improves?
@Lekanich

This comment has been minimized.

Copy link
Author

commented Oct 27, 2014

  1. netty 4.0.23
  2. Maven home: /usr/lib/apache-maven/apache-maven-3.2.1
    Java version: 1.8.0, vendor: Oracle Corporation
    Java home: /usr/java/jdk1.8.0/jre
    Default locale: en_US, platform encoding: UTF-8
    OS name: "linux", version: "3.11.0-26-generic", arch: "amd64", family: "unix"
  3. JDK provider
  4. line: 596 (method SslHandler.wrap(SslEngine, ByteBuf, ByteBuf)
    in line 599 ByteBuffer.allocateDirect(in0.remaining())
    and then it never dealloc
  5. It's Internall netty SslHandler call, so haven't try to release it

PS I think it passed to SslEngine and then someone forget to release it (line 606)

@Scottmitch

This comment has been minimized.

Copy link
Member

commented Oct 27, 2014

@Lekanich - Would you mind verifying your assumptions? If you could make the code change you suggested and test that your issues are resolved that would be very helpful. Also you could then submit a PR and get credit for your contributions ;)

@Lekanich

This comment has been minimized.

Copy link
Author

commented Oct 27, 2014

@Scottmitch I try...

@Scottmitch

This comment has been minimized.

Copy link
Member

commented Oct 27, 2014

@Lekanich - I think your line numbers may be a bit off? Is line 580 where the allocation in question is done? If so can you describe why you feel this is an issue, and where/how you would suggest releasing the memory?

@Lekanich

This comment has been minimized.

Copy link
Author

commented Oct 27, 2014

@Scottmitch LIne is correct, because that DirectBuffer never clean?
I think rewrite as
From line 587

        for (;;) {
            ByteBuffer out0 = out.nioBuffer(out.writerIndex(), out.writableBytes());
            SSLEngineResult result = engine.wrap(in0, out0);
            cleanDirectBuffer(in0);
            cleanDirectBuffer(out0);

Where cleanDirectBuffer

    private void cleanDirectBuffer(ByteBuffer buffer) {
        if (!buffer.isDirect()) {
            return;
        }
        Cleaner cleaner = ((DirectBuffer) buffer).cleaner();
        if (cleaner == null) {
            return;
        }

        cleaner.clean();
    }

PS Maybe need clean only in0

@Scottmitch

This comment has been minimized.

Copy link
Member

commented Oct 27, 2014

@Lekanich - I believe the allocations in the loop will be cleaned up by (and is the responsibility of) the ByteBuf after it has been released (for the internal buffers). In your situation is the memory ever released (despite the OOM exception)? Is the GC ever running when you observe the issue? Is it possible you are holding on to references to ByteBuf parameters or not calling release() on them?

I'm investigating a potential for a leak on line 580 but I don't think the internal buffers to ByteBuf should be released here.

@Lekanich

This comment has been minimized.

Copy link
Author

commented Oct 27, 2014

@Scottmitch I think ByteBuffer in0 is in no way connected with the sending ByteBuf, so when will be called release() it's not release ByteBuffer in0.I think it release only ByteBuffer out0

@Scottmitch

This comment has been minimized.

Copy link
Member

commented Oct 27, 2014

@Lekanich - Can you confirm that you are not using a direct buffer in this method (i.e. !in0.isDirect() returns true). Also can you see if PR #3058 changes anything for you?

@Scottmitch

This comment has been minimized.

Copy link
Member

commented Oct 27, 2014

@Lekanich - When you say:

I think ByteBuffer in0 is in no way connected with the sending ByteBuf

Do you still think this is the case if in.nioBuffer().isDirect() is true?

@Lekanich

This comment has been minimized.

Copy link
Author

commented Oct 27, 2014

@Scottmitch During debug I was into this block

@Scottmitch

This comment has been minimized.

Copy link
Member

commented Oct 27, 2014

@Lekanich - Great news...not that there is a bug but that #3058 may fix your issue. Please try this PR and let me know if this is the case.

@Scottmitch Scottmitch self-assigned this Oct 27, 2014

@Scottmitch Scottmitch added this to the 4.0.24.Final milestone Oct 27, 2014

@Lekanich

This comment has been minimized.

Copy link
Author

commented Oct 27, 2014

@Scottmitch Yes it seems to help.

@Scottmitch

This comment has been minimized.

Copy link
Member

commented Oct 27, 2014

@Lekanich Thank you for testing this out and reporting this issue!

Just to clarify when you say "it seems to help" can you be a bit more specific? In the issue description you provide details as to how much more memory is used while SSL is enabled.

  • Can you provide these metrics while using #3058? I'm more just looking to see that it is not 10x more and that allocation/releasing pattern follows a similar pattern in the ssl/plaintext cases.
  • Are you no longer seeing the OutOfMemory exception?
  • Are there other areas you suspect to be leaky?
@normanmaurer

This comment has been minimized.

Copy link
Member

commented Oct 28, 2014

Fixed by 8db8aca

@Lekanich

This comment has been minimized.

Copy link
Author

commented Oct 28, 2014

@Scottmitch Perf Test with fix uses 3GB of memory, and without over 13GB (memory + swap)

@Scottmitch

This comment has been minimized.

Copy link
Member

commented Oct 28, 2014

@Lekanich - Sounds much better. Thanks again for reporting!

@Lekanich

This comment has been minimized.

Copy link
Author

commented Oct 28, 2014

@Scottmitch Thanks for the quick reply)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.