Commit ab8e4744 authored by Brian Carlstrom's avatar Brian Carlstrom
Browse files

SSL* AppData should not hold onto JNI global references

Summary:

NativeCrypto.SSL_do_handshake stored JNI global references in its
AppData instance for use in upcalls from OpenSSL that invoke Java
callbacks. However, one of the references was to the
SSLHandshakeCallbacks which in the common case of OpenSSLSocketImpl is
the OpenSSLSocketImpl instance itself. This meant that if code dropped
the OpenSSLSocketImpl without closing (such as Apache HTTP Client),
the instances would never be collected, and perhaps more importantly,
file descriptors would not be closed.

The fix is to pass in the objects required during a callback in all
downcalls to SSL_* methods that could result in a callback and clear
them on return. The existing code already did this for the JNIEnv*, so
that code was expanded to handle setting the jobjects as well.

Details:

In the native code used to extract the FileDescriptor object from a
Socket on the call to NativeCrypto.SSL_do_handshake. However, since we
need this for every read and write operations, we now do this in Java
to avoid the repeated overhead. NativeCrypto.SSL_do_handshake now
takes a FileDescriptor, which it extracted from the Socket the
convenience function using NativeCrypto.getFileDescriptor(Socket)

	luni/src/main/java/org/apache/harmony/xnet/provider/jsse/NativeCrypto.java

   In addition to tracking changes to pass FileDescriptor and
   SSLHandshakeCallbacks, removed final uses of getFieldId since the
   code no longer needs to extract FileDescriptors itself

	luni/src/main/native/NativeCrypto.cpp

The Socket field used to be non-null in the wrapper case and null in
the non-wrapper case. To simplify things a bit, "socket == this" in
the non-wrapper case. The socket field is now also final and joined by
a final FileDescriptor field.

	luni/src/main/java/org/apache/harmony/xnet/provider/jsse/OpenSSLSocketImpl.java

Updated NativeCryptoTest to track FileDescriptor and
SSLHandshakeCallbacks by expanding the Hooks.afterHandshake to provide
them. Also changed to add a 5 second timeout to many test cases.

	luni/src/test/java/org/apache/harmony/xnet/provider/jsse/NativeCryptoTest.java

Bug: 2989218

Change-Id: Iccef92b59475f3c1929e990893579493ece9d442
parent a4315cab
Loading
Loading
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please to comment