You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by md...@apache.org on 2018/02/16 15:12:32 UTC
[2/4] hbase git commit: HBASE-19920 Lazy init for ProtobufUtil
classloader
HBASE-19920 Lazy init for ProtobufUtil classloader
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/138f82c8
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/138f82c8
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/138f82c8
Branch: refs/heads/branch-2
Commit: 138f82c8c57aeeb953d537114b02c2e8ca21c4c5
Parents: 70d3413
Author: Mike Drob <md...@apache.org>
Authored: Thu Feb 8 16:12:46 2018 -0600
Committer: Mike Drob <md...@apache.org>
Committed: Fri Feb 16 09:11:45 2018 -0600
----------------------------------------------------------------------
.../hbase/ipc/RemoteWithExtrasException.java | 23 ++++--
.../hadoop/hbase/protobuf/ProtobufUtil.java | 30 ++++----
.../hbase/shaded/protobuf/ProtobufUtil.java | 35 ++++++---
.../hadoop/hbase/security/token/TokenUtil.java | 13 +++-
.../hbase/security/token/TestTokenUtil.java | 80 ++++++++++++++++++++
5 files changed, 146 insertions(+), 35 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/138f82c8/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RemoteWithExtrasException.java
----------------------------------------------------------------------
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RemoteWithExtrasException.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RemoteWithExtrasException.java
index 1374ab0..3a7540e 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RemoteWithExtrasException.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RemoteWithExtrasException.java
@@ -19,6 +19,8 @@ package org.apache.hadoop.hbase.ipc;
import java.io.IOException;
import java.lang.reflect.Constructor;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.DoNotRetryIOException;
@@ -36,19 +38,24 @@ import org.apache.hadoop.ipc.RemoteException;
*/
@SuppressWarnings("serial")
@InterfaceAudience.Public
-@edu.umd.cs.findbugs.annotations.SuppressWarnings(
- value = "DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED", justification = "None. Address sometime.")
public class RemoteWithExtrasException extends RemoteException {
private final String hostname;
private final int port;
private final boolean doNotRetry;
- private final static ClassLoader CLASS_LOADER;
+ /**
+ * Dynamic class loader to load filter/comparators
+ */
+ private final static class ClassLoaderHolder {
+ private final static ClassLoader CLASS_LOADER;
- static {
- ClassLoader parent = RemoteWithExtrasException.class.getClassLoader();
- Configuration conf = HBaseConfiguration.create();
- CLASS_LOADER = new DynamicClassLoader(conf, parent);
+ static {
+ ClassLoader parent = RemoteWithExtrasException.class.getClassLoader();
+ Configuration conf = HBaseConfiguration.create();
+ CLASS_LOADER = AccessController.doPrivileged((PrivilegedAction<ClassLoader>)
+ () -> new DynamicClassLoader(conf, parent)
+ );
+ }
}
public RemoteWithExtrasException(String className, String msg, final boolean doNotRetry) {
@@ -69,7 +76,7 @@ public class RemoteWithExtrasException extends RemoteException {
try {
// try to load a exception class from where the HBase classes are loaded or from Dynamic
// classloader.
- realClass = Class.forName(getClassName(), false, CLASS_LOADER);
+ realClass = Class.forName(getClassName(), false, ClassLoaderHolder.CLASS_LOADER);
} catch (ClassNotFoundException cnfe) {
try {
// cause could be a hadoop exception, try to load from hadoop classpath
http://git-wip-us.apache.org/repos/asf/hbase/blob/138f82c8/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
----------------------------------------------------------------------
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
index 54837dc..3033da7 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
@@ -31,6 +31,8 @@ import com.google.protobuf.TextFormat;
import java.io.IOException;
import java.lang.reflect.Constructor;
import java.lang.reflect.Method;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
@@ -113,8 +115,6 @@ import org.apache.yetus.audience.InterfaceAudience;
* @see ProtobufUtil
*/
// TODO: Generate this class from the shaded version.
-@edu.umd.cs.findbugs.annotations.SuppressWarnings(
- value="DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED", justification="None. Address sometime.")
@InterfaceAudience.Private // TODO: some clients (Hive, etc) use this class.
public final class ProtobufUtil {
private ProtobufUtil() {
@@ -168,14 +168,18 @@ public final class ProtobufUtil {
}
/**
- * Dynamic class loader to load filter/comparators
- */
- private final static ClassLoader CLASS_LOADER;
+ * Dynamic class loader to load filter/comparators
+ */
+ private final static class ClassLoaderHolder {
+ private final static ClassLoader CLASS_LOADER;
- static {
- ClassLoader parent = ProtobufUtil.class.getClassLoader();
- Configuration conf = HBaseConfiguration.create();
- CLASS_LOADER = new DynamicClassLoader(conf, parent);
+ static {
+ ClassLoader parent = ProtobufUtil.class.getClassLoader();
+ Configuration conf = HBaseConfiguration.create();
+ CLASS_LOADER = AccessController.doPrivileged((PrivilegedAction<ClassLoader>)
+ () -> new DynamicClassLoader(conf, parent)
+ );
+ }
}
/**
@@ -1430,8 +1434,7 @@ public final class ProtobufUtil {
String funcName = "parseFrom";
byte [] value = proto.getSerializedComparator().toByteArray();
try {
- Class<? extends ByteArrayComparable> c =
- (Class<? extends ByteArrayComparable>)Class.forName(type, true, CLASS_LOADER);
+ Class<?> c = Class.forName(type, true, ClassLoaderHolder.CLASS_LOADER);
Method parseFrom = c.getMethod(funcName, byte[].class);
if (parseFrom == null) {
throw new IOException("Unable to locate function: " + funcName + " in type: " + type);
@@ -1454,8 +1457,7 @@ public final class ProtobufUtil {
final byte [] value = proto.getSerializedFilter().toByteArray();
String funcName = "parseFrom";
try {
- Class<? extends Filter> c =
- (Class<? extends Filter>)Class.forName(type, true, CLASS_LOADER);
+ Class<?> c = Class.forName(type, true, ClassLoaderHolder.CLASS_LOADER);
Method parseFrom = c.getMethod(funcName, byte[].class);
if (parseFrom == null) {
throw new IOException("Unable to locate function: " + funcName + " in type: " + type);
@@ -1541,7 +1543,7 @@ public final class ProtobufUtil {
String type = parameter.getName();
try {
Class<? extends Throwable> c =
- (Class<? extends Throwable>)Class.forName(type, true, CLASS_LOADER);
+ (Class<? extends Throwable>)Class.forName(type, true, ClassLoaderHolder.CLASS_LOADER);
Constructor<? extends Throwable> cn = null;
try {
cn = c.getDeclaredConstructor(String.class);
http://git-wip-us.apache.org/repos/asf/hbase/blob/138f82c8/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java
----------------------------------------------------------------------
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java
index 3a59492..4472846 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java
@@ -23,6 +23,8 @@ import java.io.InputStream;
import java.lang.reflect.Constructor;
import java.lang.reflect.Method;
import java.nio.ByteBuffer;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@@ -105,6 +107,7 @@ import org.apache.hadoop.hbase.util.VersionInfo;
import org.apache.hadoop.ipc.RemoteException;
import org.apache.yetus.audience.InterfaceAudience;
+import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting;
import org.apache.hbase.thirdparty.com.google.common.io.ByteStreams;
import org.apache.hbase.thirdparty.com.google.gson.JsonArray;
import org.apache.hbase.thirdparty.com.google.gson.JsonElement;
@@ -191,8 +194,6 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.ZooKeeperProtos;
* @see ProtobufUtil
*/
// TODO: Generate the non-shaded protobufutil from this one.
-@edu.umd.cs.findbugs.annotations.SuppressWarnings(
- value="DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED", justification="None. Address sometime.")
@InterfaceAudience.Private // TODO: some clients (Hive, etc) use this class
public final class ProtobufUtil {
private ProtobufUtil() {
@@ -244,15 +245,27 @@ public final class ProtobufUtil {
EMPTY_RESULT_PB_STALE = builder.build();
}
+ private static volatile boolean classLoaderLoaded = false;
+
/**
* Dynamic class loader to load filter/comparators
*/
- private final static ClassLoader CLASS_LOADER;
+ private final static class ClassLoaderHolder {
+ private final static ClassLoader CLASS_LOADER;
- static {
- ClassLoader parent = ProtobufUtil.class.getClassLoader();
- Configuration conf = HBaseConfiguration.create();
- CLASS_LOADER = new DynamicClassLoader(conf, parent);
+ static {
+ ClassLoader parent = ProtobufUtil.class.getClassLoader();
+ Configuration conf = HBaseConfiguration.create();
+ CLASS_LOADER = AccessController.doPrivileged((PrivilegedAction<ClassLoader>)
+ () -> new DynamicClassLoader(conf, parent)
+ );
+ classLoaderLoaded = true;
+ }
+ }
+
+ @VisibleForTesting
+ public static boolean isClassLoaderLoaded() {
+ return classLoaderLoaded;
}
/**
@@ -1586,8 +1599,7 @@ public final class ProtobufUtil {
String funcName = "parseFrom";
byte [] value = proto.getSerializedComparator().toByteArray();
try {
- Class<? extends ByteArrayComparable> c =
- (Class<? extends ByteArrayComparable>)Class.forName(type, true, CLASS_LOADER);
+ Class<?> c = Class.forName(type, true, ClassLoaderHolder.CLASS_LOADER);
Method parseFrom = c.getMethod(funcName, byte[].class);
if (parseFrom == null) {
throw new IOException("Unable to locate function: " + funcName + " in type: " + type);
@@ -1610,8 +1622,7 @@ public final class ProtobufUtil {
final byte [] value = proto.getSerializedFilter().toByteArray();
String funcName = "parseFrom";
try {
- Class<? extends Filter> c =
- (Class<? extends Filter>)Class.forName(type, true, CLASS_LOADER);
+ Class<?> c = Class.forName(type, true, ClassLoaderHolder.CLASS_LOADER);
Method parseFrom = c.getMethod(funcName, byte[].class);
if (parseFrom == null) {
throw new IOException("Unable to locate function: " + funcName + " in type: " + type);
@@ -1697,7 +1708,7 @@ public final class ProtobufUtil {
String type = parameter.getName();
try {
Class<? extends Throwable> c =
- (Class<? extends Throwable>)Class.forName(type, true, CLASS_LOADER);
+ (Class<? extends Throwable>)Class.forName(type, true, ClassLoaderHolder.CLASS_LOADER);
Constructor<? extends Throwable> cn = null;
try {
cn = c.getDeclaredConstructor(String.class);
http://git-wip-us.apache.org/repos/asf/hbase/blob/138f82c8/hbase-server/src/main/java/org/apache/hadoop/hbase/security/token/TokenUtil.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/token/TokenUtil.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/token/TokenUtil.java
index 5461760..c54d905 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/token/TokenUtil.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/token/TokenUtil.java
@@ -26,7 +26,6 @@ import com.google.protobuf.ByteString;
import com.google.protobuf.ServiceException;
import org.apache.hadoop.hbase.zookeeper.ZKWatcher;
-import org.apache.yetus.audience.InterfaceAudience;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.TableName;
@@ -41,6 +40,7 @@ import org.apache.hadoop.io.Text;
import org.apache.hadoop.mapred.JobConf;
import org.apache.hadoop.mapreduce.Job;
import org.apache.hadoop.security.token.Token;
+import org.apache.yetus.audience.InterfaceAudience;
import org.apache.zookeeper.KeeperException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -53,6 +53,15 @@ public class TokenUtil {
// This class is referenced indirectly by User out in common; instances are created by reflection
private static final Logger LOG = LoggerFactory.getLogger(TokenUtil.class);
+ // Set in TestTokenUtil via reflection
+ private static ServiceException injectedException;
+
+ private static void injectFault() throws ServiceException {
+ if (injectedException != null) {
+ throw injectedException;
+ }
+ }
+
/**
* Obtain and return an authentication token for the current user.
* @param conn The HBase cluster connection
@@ -63,6 +72,8 @@ public class TokenUtil {
Connection conn) throws IOException {
Table meta = null;
try {
+ injectFault();
+
meta = conn.getTable(TableName.META_TABLE_NAME);
CoprocessorRpcChannel rpcChannel = meta.coprocessorService(HConstants.EMPTY_START_ROW);
AuthenticationProtos.AuthenticationService.BlockingInterface service =
http://git-wip-us.apache.org/repos/asf/hbase/blob/138f82c8/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/TestTokenUtil.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/TestTokenUtil.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/TestTokenUtil.java
new file mode 100644
index 0000000..32fcddb
--- /dev/null
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/TestTokenUtil.java
@@ -0,0 +1,80 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+package org.apache.hadoop.hbase.security.token;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.fail;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.net.URL;
+import java.net.URLClassLoader;
+
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.client.Connection;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
+
+@Category(SmallTests.class)
+public class TestTokenUtil {
+ @ClassRule
+ public static final HBaseClassTestRule CLASS_RULE =
+ HBaseClassTestRule.forClass(TestTokenUtil.class);
+
+ @Test
+ public void testObtainToken() throws Exception {
+ URL urlPU = ProtobufUtil.class.getProtectionDomain().getCodeSource().getLocation();
+ URL urlTU = TokenUtil.class.getProtectionDomain().getCodeSource().getLocation();
+
+ ClassLoader cl = new URLClassLoader(new URL[] { urlPU, urlTU }, getClass().getClassLoader());
+
+ Throwable injected = new com.google.protobuf.ServiceException("injected");
+
+ Class<?> tokenUtil = cl.loadClass(TokenUtil.class.getCanonicalName());
+ Field shouldInjectFault = tokenUtil.getDeclaredField("injectedException");
+ shouldInjectFault.setAccessible(true);
+ shouldInjectFault.set(null, injected);
+
+ try {
+ tokenUtil.getMethod("obtainToken", Connection.class)
+ .invoke(null, new Object[] { null });
+ fail("Should have injected exception.");
+ } catch (InvocationTargetException e) {
+ Throwable t = e;
+ boolean serviceExceptionFound = false;
+ while ((t = t.getCause()) != null) {
+ if (t == injected) { // reference equality
+ serviceExceptionFound = true;
+ break;
+ }
+ }
+ if (!serviceExceptionFound) {
+ throw e; // wrong exception, fail the test
+ }
+ }
+
+ Boolean loaded = (Boolean) cl.loadClass(ProtobufUtil.class.getCanonicalName())
+ .getDeclaredMethod("isClassLoaderLoaded")
+ .invoke(null);
+ assertFalse("Should not have loaded DynamicClassLoader", loaded);
+ }
+}