Remove ScopedGlobalRef (and other cleanups)
ScopedGlobalRef caused more trouble that it was worth. Rather than
trying to fix it to require updating of the JNIEnv, remove it to
remove the temptation for others to use it.
Also update SSL_set_ciphers_lists to use ScopedLocalRef and add HTML
anchors to Standard names javadoc JSEE references.
Change-Id: Ic3ed1bae3f29ee971d4461de31395b78c4949090
diff --git a/include/ScopedGlobalRef.h b/include/ScopedGlobalRef.h
deleted file mode 100644
index 1fbeabb..0000000
--- a/include/ScopedGlobalRef.h
+++ /dev/null
@@ -1,56 +0,0 @@
-/*
- * Copyright (C) 2010 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#ifndef SCOPED_GLOBAL_REF_H_included
-#define SCOPED_GLOBAL_REF_H_included
-
-#include "JNIHelp.h"
-
-// A smart pointer that creates a JNI global reference from a weak reference.
-// The global reference is deleted when the smart pointer goes out of scope.
-class ScopedGlobalRef {
-public:
- ScopedGlobalRef(JNIEnv* env, jobject localRef)
- : mEnv(env), mGlobalRef(NULL)
- {
- mGlobalRef = env->NewGlobalRef(localRef);
- }
-
- ~ScopedGlobalRef() {
- reset();
- }
-
- void reset() {
- if (mGlobalRef != NULL) {
- mEnv->DeleteGlobalRef(mGlobalRef);
- mGlobalRef = NULL;
- }
- }
-
- jobject get() const {
- return mGlobalRef;
- }
-
-private:
- JNIEnv* mEnv;
- jobject mGlobalRef;
-
- // Disallow copy and assignment.
- ScopedGlobalRef(const ScopedGlobalRef&);
- void operator=(const ScopedGlobalRef&);
-};
-
-#endif // SCOPED_GLOBAL_REF_H_included
diff --git a/luni/src/main/native/org_apache_harmony_xnet_provider_jsse_NativeCrypto.cpp b/luni/src/main/native/org_apache_harmony_xnet_provider_jsse_NativeCrypto.cpp
index 0e9d641..87cedca 100644
--- a/luni/src/main/native/org_apache_harmony_xnet_provider_jsse_NativeCrypto.cpp
+++ b/luni/src/main/native/org_apache_harmony_xnet_provider_jsse_NativeCrypto.cpp
@@ -37,7 +37,7 @@
#include <openssl/ssl.h>
#include "ScopedByteArray.h"
-#include "ScopedGlobalRef.h"
+#include "ScopedLocalRef.h"
#include "ScopedUtfChars.h"
#include "UniquePtr.h"
@@ -986,50 +986,61 @@
int fdsEmergency[2];
MUTEX_TYPE mutex;
JNIEnv* env;
- ScopedGlobalRef certificateChainVerifier;
- ScopedGlobalRef handshakeCompletedCallback;
+ jobject certificateChainVerifier;
+ jobject handshakeCompletedCallback;
/**
* Creates our application data and attaches it to a given SSL connection.
*
- * @param ssl The SSL connection to attach the data to.
* @param env The JNIEnv
* @param ccv The CertificateChainVerifier
* @param hcc The HandshakeCompletedCallback
*/
public:
- static AppData* create(JNIEnv* e,
- jobject certificateChainVerifier,
- jobject handshakeCompletedCallback) {
- if (certificateChainVerifier == NULL) {
+ static AppData* create(JNIEnv* env,
+ jobject ccv,
+ jobject hcc) {
+ if (ccv == NULL || hcc == NULL) {
return NULL;
}
- if (handshakeCompletedCallback == NULL) {
- return NULL;
- }
- UniquePtr<AppData> appData(new AppData(e, certificateChainVerifier, handshakeCompletedCallback));
- if (appData->certificateChainVerifier.get() == NULL) {
- return NULL;
- }
- if (appData->handshakeCompletedCallback.get() == NULL) {
- return NULL;
- }
+ AppData* appData = new AppData(env);
if (pipe(appData->fdsEmergency) == -1) {
+ destroy(env, appData);
return NULL;
}
if (MUTEX_SETUP(appData->mutex) == -1) {
+ destroy(env, appData);
return NULL;
}
- return appData.release();
+ appData->certificateChainVerifier = env->NewGlobalRef(ccv);
+ if (appData->certificateChainVerifier == NULL) {
+ destroy(env, appData);
+ return NULL;
+ }
+ appData->handshakeCompletedCallback = env->NewGlobalRef(hcc);
+ if (appData->handshakeCompletedCallback == NULL) {
+ destroy(env, appData);
+ return NULL;
+ }
+ return appData;
+ }
+
+ static void destroy(JNIEnv* env, AppData* appData) {
+ if (appData == NULL) {
+ return;
+ }
+ appData->cleanupGlobalRefs(env);
+ delete appData;
}
private:
- AppData(JNIEnv* e, jobject ccv, jobject hcc) :
+ AppData(JNIEnv* env) :
aliveAndKicking(1),
waitingThreads(0),
- env(e),
- certificateChainVerifier(e, ccv),
- handshakeCompletedCallback(e, hcc) {
+ env(NULL),
+ certificateChainVerifier(NULL),
+ handshakeCompletedCallback(NULL) {
+ setEnv(env);
fdsEmergency[0] = -1;
fdsEmergency[1] = -1;
}
@@ -1037,7 +1048,6 @@
/**
* Destroys our application data, cleaning up everything in the process.
*/
- public:
~AppData() {
aliveAndKicking = 0;
if (fdsEmergency[0] != -1) {
@@ -1049,20 +1059,29 @@
MUTEX_CLEANUP(mutex);
}
- void setEnv(JNIEnv* e) {
- if (handshakeCompletedCallback.get() == NULL) {
- return;
+ void cleanupGlobalRefs(JNIEnv* env) {
+ if (certificateChainVerifier != NULL) {
+ env->DeleteGlobalRef(certificateChainVerifier);
+ certificateChainVerifier = NULL;
}
+ if (handshakeCompletedCallback != NULL) {
+ env->DeleteGlobalRef(handshakeCompletedCallback);
+ handshakeCompletedCallback = NULL;
+ }
+ clearEnv();
+ }
+
+ public:
+ void setEnv(JNIEnv* e) {
env = e;
}
+
void clearEnv() {
env = NULL;
}
- void handshakeCompleted() {
- certificateChainVerifier.reset();
- handshakeCompletedCallback.reset();
- clearEnv();
+ void handshakeCompleted(JNIEnv* e) {
+ cleanupGlobalRefs(e);
}
};
@@ -1229,7 +1248,7 @@
JNI_TRACE("ssl=%p cert_verify_callback => 0", ssl);
return 0;
}
- jobject certificateChainVerifier = appData->certificateChainVerifier.get();
+ jobject certificateChainVerifier = appData->certificateChainVerifier;
jclass cls = env->GetObjectClass(certificateChainVerifier);
jmethodID methodID = env->GetMethodID(cls, "verifyCertificateChain", "([[BLjava/lang/String;)V");
@@ -1281,7 +1300,7 @@
JNI_TRACE("ssl=%p info_callback env error", ssl);
return;
}
- jobject handshakeCompletedCallback = appData->handshakeCompletedCallback.get();
+ jobject handshakeCompletedCallback = appData->handshakeCompletedCallback;
jclass cls = env->GetObjectClass(handshakeCompletedCallback);
jmethodID methodID = env->GetMethodID(cls, "handshakeCompleted", "()V");
@@ -1293,7 +1312,7 @@
JNI_TRACE("ssl=%p info_callback exception", ssl);
}
- appData->handshakeCompleted();
+ appData->handshakeCompleted(env);
JNI_TRACE("ssl=%p info_callback completed", ssl);
}
@@ -1603,8 +1622,8 @@
int length = env->GetArrayLength(cipherSuites);
JNI_TRACE("ssl=%p NativeCrypto_SSL_set_cipher_lists length=%d", ssl, length);
for (int i = 0; i < length; i++) {
- jstring cipherSuite = (jstring) env->GetObjectArrayElement(cipherSuites, i);
- ScopedUtfChars c(env, cipherSuite);
+ ScopedLocalRef cipherSuite(env, env->GetObjectArrayElement(cipherSuites, i));
+ ScopedUtfChars c(env, reinterpret_cast<jstring>(cipherSuite.get()));
JNI_TRACE("ssl=%p NativeCrypto_SSL_set_cipher_lists cipherSuite=%s", ssl, c.c_str());
bool found = false;
for (int j = 0; j < num_ciphers; j++) {
@@ -1776,6 +1795,7 @@
return 0;
}
SSL_set_app_data(ssl, (char*) appData);
+ JNI_TRACE("ssl=%p AppData::create => %p", ssl, appData);
if (client_mode) {
SSL_set_connect_state(ssl);
@@ -2368,11 +2388,12 @@
SSL* ssl = to_SSL(env, ssl_address, true);
JNI_TRACE("ssl=%p NativeCrypto_SSL_free", ssl);
if (ssl == NULL) {
- return;
+ return;
}
AppData* appData = (AppData*) SSL_get_app_data(ssl);
SSL_set_app_data(ssl, NULL);
- delete appData;
+ JNI_TRACE("ssl=%p AppData::destroy(%p)", ssl, appData);
+ AppData::destroy(env, appData);
SSL_free(ssl);
}
diff --git a/support/src/test/java/javax/net/ssl/StandardNames.java b/support/src/test/java/javax/net/ssl/StandardNames.java
index 86dbd78..bc0e4e3 100644
--- a/support/src/test/java/javax/net/ssl/StandardNames.java
+++ b/support/src/test/java/javax/net/ssl/StandardNames.java
@@ -23,10 +23,16 @@
/**
* This class defines expected string names for protocols, key types, client and server auth types, cipher suites.
- *
- * Based on documentation from http://java.sun.com/j2se/1.5.0/docs/guide/security/jsse/JSSERefGuide.html#AppA
*
- * Java 6 version http://java.sun.com/javase/6/docs/technotes/guides/security/SunProviders.html
+ * Initially based on "Appendix A: Standard Names" of
+ * <a href="http://java.sun.com/j2se/1.5.0/docs/guide/security/jsse/JSSERefGuide.html#AppA">
+ * Java Secure Socket Extension (JSSE) Reference Guide for the JavaTM 2 Platform Standard Edition 5
+ * </a>
+ *
+ * Updated based on the "The SunJSSE Provider" section of
+ * <a href="java.sun.com/javase/6/docs/technotes/guides/security/SunProviders.html#SunJSSEProvider">
+ * Java Cryptography Architecture Sun Providers Documentation for JavaTM Platform Standard Edition 6
+ * </a>
*/
public final class StandardNames {
@@ -161,12 +167,12 @@
// Old non standard exportable encryption
CIPHER_SUITES_NEITHER.add("SSL_RSA_EXPORT1024_WITH_DES_CBC_SHA");
CIPHER_SUITES_NEITHER.add("SSL_RSA_EXPORT1024_WITH_RC4_56_SHA");
-
+
// No RC2
CIPHER_SUITES_NEITHER.add("SSL_RSA_EXPORT_WITH_RC2_CBC_40_MD5");
CIPHER_SUITES_NEITHER.add("TLS_KRB5_EXPORT_WITH_RC2_CBC_40_SHA");
CIPHER_SUITES_NEITHER.add("TLS_KRB5_EXPORT_WITH_RC2_CBC_40_MD5");
-
+
CIPHER_SUITES = (TestSSLContext.IS_RI) ? CIPHER_SUITES_RI : CIPHER_SUITES_OPENSSL;
}
}