Leak in OpenSslContext

[ylgrgyq]

OpenSslContext (I mean OpenSslClientContext or OpenSslServerContext exactly) object hold some resources (like custom verification callback to verify ssl certificate) which need to be released when this OpenSslContext object is GC. This is done by JVM calling finalize in OpenSslContext.

But I found that OpenSslContext will never be GC so all the resources possessed by it will never be released. Take OpenSslClientContext as example but the same bug exists in OpenSslServerContext too. In the constructor of OpenSslClientContext, there's an anonymous class object which is used as a custom verification callback to verify certificate and is set into SSLContext by calling SSLContext.setCertVerifyCallback. This SSLContext.setCertVerifyCallback is a native function in which a global reference is created and ref to the custom verification callback passed to SSLContext.setCertVerifyCallback.

So the custom verification callback used to verify certificate will be released only when OpenSslClientContext object is GC. And the OpenSslClientContext object will be GC only when the custom verification callback is removed from JNI global reference.

[normanmaurer]

@ylgrgyq good catch! Let me fix this.

[normanmaurer]

@ylgrgyq can you check #5380

[normanmaurer]

Fixed by #5380

[ylgrgyq]

Hi, @normanmaurer

Could you please tell me why OpenSslContext use finalize to release resources used by the Context? Why not just provide an explicit termination method and let the user to call it and do the releasing stuff in this termination method?

Because, I think you already heard, a lot of people say that finalize is evil. It's performance is bad, when even weather it will be called is not determined. Maybe the object is already unreachable for a long time but finalize method on this object is not called which could lead to a lot of unreachable object linger on heap.

I admit that use finalize here is more user friendly, but still think an explicit termination method maybe much safer. Like showed in this issue, if let user call the termination method I think the OpenSslContext will not leak. WDYT?