diff --git beeline/src/java/org/apache/hive/beeline/Commands.java beeline/src/java/org/apache/hive/beeline/Commands.java index f4dd586e11..90cae9f408 100644 --- beeline/src/java/org/apache/hive/beeline/Commands.java +++ beeline/src/java/org/apache/hive/beeline/Commands.java @@ -169,7 +169,7 @@ public boolean addlocaldriverjar(String line) { return false; } - URLClassLoader classLoader = (URLClassLoader) Thread.currentThread().getContextClassLoader(); + ClassLoader classLoader = Thread.currentThread().getContextClassLoader(); try { beeLine.debug(jarPath + " is added to the local beeline."); URLClassLoader newClassLoader = new URLClassLoader(new URL[]{p.toURL()}, classLoader); diff --git common/src/java/org/apache/hadoop/hive/common/JavaUtils.java common/src/java/org/apache/hadoop/hive/common/JavaUtils.java index c011cd1626..19066196aa 100644 --- common/src/java/org/apache/hadoop/hive/common/JavaUtils.java +++ common/src/java/org/apache/hadoop/hive/common/JavaUtils.java @@ -91,8 +91,10 @@ public static boolean closeClassLoadersTo(ClassLoader current, ClassLoader stop) try { closeClassLoader(current); } catch (IOException e) { - LOG.info("Failed to close class loader " + current + - Arrays.toString(((URLClassLoader) current).getURLs()), e); + String detailedMessage = current instanceof URLClassLoader ? + Arrays.toString(((URLClassLoader) current).getURLs()) : + ""; + LOG.info("Failed to close class loader " + current + " " + detailedMessage, e); } } return true; diff --git llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/FunctionLocalizer.java llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/FunctionLocalizer.java index 2a6ef3a246..136fe2a3b3 100644 --- llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/FunctionLocalizer.java +++ llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/FunctionLocalizer.java @@ -18,8 +18,10 @@ import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; -import java.net.URLClassLoader; +import java.security.AccessController; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.IdentityHashMap; import java.util.LinkedList; import java.util.List; @@ -33,10 +35,10 @@ import org.apache.hadoop.hive.metastore.api.Function; import org.apache.hadoop.hive.metastore.api.MetaException; import org.apache.hadoop.hive.metastore.api.ResourceUri; -import org.apache.hadoop.hive.metastore.HiveMetaStoreClient; +import org.apache.hadoop.hive.ql.exec.AddToClassPathAction; import org.apache.hadoop.hive.ql.exec.FunctionRegistry; import org.apache.hadoop.hive.ql.exec.FunctionTask; -import org.apache.hadoop.hive.ql.exec.Utilities; +import org.apache.hadoop.hive.ql.exec.UDFClassLoader; import org.apache.hadoop.hive.ql.exec.FunctionInfo.FunctionResource; import org.apache.hadoop.hive.ql.metadata.Hive; import org.apache.hadoop.hive.ql.metadata.HiveException; @@ -60,7 +62,7 @@ private final Thread workThread; private final File localDir; private final Configuration conf; - private final URLClassLoader executorClassloader; + private final UDFClassLoader executorClassloader; private final IdentityHashMap, Boolean> allowedUdfClasses = new IdentityHashMap<>(); @@ -70,8 +72,9 @@ public FunctionLocalizer(Configuration conf, String localDir) { this.conf = conf; this.localDir = new File(localDir, DIR_NAME); - this.executorClassloader = (URLClassLoader)Utilities.createUDFClassLoader( - (URLClassLoader)Thread.currentThread().getContextClassLoader(), new String[]{}); + AddToClassPathAction addAction = new AddToClassPathAction( + Thread.currentThread().getContextClassLoader(), Collections.emptyList(), true); + this.executorClassloader = AccessController.doPrivileged(addAction); this.workThread = new Thread(new Runnable() { @Override public void run() { @@ -223,7 +226,8 @@ public void refreshClassloader() throws IOException { recentlyLocalizedJars.clear(); ClassLoader updatedCl = null; try { - updatedCl = Utilities.addToClassPath(executorClassloader, jars); + AddToClassPathAction addAction = new AddToClassPathAction(executorClassloader, Arrays.asList(jars)); + updatedCl = AccessController.doPrivileged(addAction); if (LOG.isInfoEnabled()) { LOG.info("Added " + jars.length + " jars to classpath"); } diff --git ql/src/java/org/apache/hadoop/hive/ql/exec/AddToClassPathAction.java ql/src/java/org/apache/hadoop/hive/ql/exec/AddToClassPathAction.java new file mode 100644 index 0000000000..ab923b2730 --- /dev/null +++ ql/src/java/org/apache/hadoop/hive/ql/exec/AddToClassPathAction.java @@ -0,0 +1,87 @@ +/* + * 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.hive.ql.exec; + +import java.net.URL; +import java.security.PrivilegedAction; +import java.util.Collection; +import java.util.Collections; +import java.util.Objects; + +/** + * Helper class to create UDFClassLoader when running under a security manager. To create a class loader: + * > AddToClassPathAction addAction = new AddToClassPathAction(parentLoader, newPaths, true); + * > UDFClassLoader childClassLoader = AccessController.doPrivileged(addAction); + * To try to add to the class path of the existing class loader, do the save without forceNewClassLoader=true. + * Note that a class loader might be still created as fallback method. + *

+ * This is slightly inconvenient, but forces the caller code to make the doPriviliged call, rather than us making the + * call on the caller's behalf, in accordance with the security guidelines at: + * https://docs.oracle.com/javase/8/docs/technotes/guides/security/doprivileged.html + */ +public class AddToClassPathAction implements PrivilegedAction { + + private final ClassLoader parentLoader; + private final Collection newPaths; + private final boolean forceNewClassLoader; + + public AddToClassPathAction(ClassLoader parentLoader, Collection newPaths, boolean forceNewClassLoader) { + this.parentLoader = parentLoader; + this.newPaths = newPaths != null ? newPaths : Collections.emptyList(); + this.forceNewClassLoader = forceNewClassLoader; + if (parentLoader == null) { + throw new IllegalArgumentException("UDFClassLoader is not designed to be a bootstrap class loader!"); + } + } + + public AddToClassPathAction(ClassLoader parentLoader, Collection newPaths) { + this(parentLoader, newPaths, false); + } + + @Override + public UDFClassLoader run() { + if (useExistingClassLoader()) { + final UDFClassLoader udfClassLoader = (UDFClassLoader) parentLoader; + for (String path : newPaths) { + udfClassLoader.addURL(Utilities.urlFromPathString(path)); + } + return udfClassLoader; + } else { + return createUDFClassLoader(); + } + } + + private boolean useExistingClassLoader() { + if (!forceNewClassLoader && parentLoader instanceof UDFClassLoader) { + final UDFClassLoader udfClassLoader = (UDFClassLoader) parentLoader; + // The classloader may have been closed, Cannot add to the same instance + return !udfClassLoader.isClosed(); + } + // Cannot use the same classloader if it is not an instance of {@code UDFClassLoader}, or new loader was explicily + // requested + return false; + } + + private UDFClassLoader createUDFClassLoader() { + return new UDFClassLoader(newPaths.stream() + .map(Utilities::urlFromPathString) + .filter(Objects::nonNull) + .toArray(URL[]::new), parentLoader); + } +} diff --git ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java index 36bc08f34e..5184e0c2b4 100644 --- ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java +++ ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java @@ -34,8 +34,8 @@ import java.io.Serializable; import java.net.URI; import java.net.URL; -import java.net.URLClassLoader; import java.net.URLDecoder; +import java.security.AccessController; import java.sql.Connection; import java.sql.DriverManager; import java.sql.PreparedStatement; @@ -208,8 +208,6 @@ import com.esotericsoftware.kryo.Kryo; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; -import com.google.common.collect.Lists; -import com.google.common.collect.Sets; import com.google.common.util.concurrent.MoreExecutors; import com.google.common.util.concurrent.ThreadFactoryBuilder; @@ -443,8 +441,10 @@ private static BaseWork getBaseWork(Configuration conf, String name) { // threads, should be unnecessary while SPARK-5377 is resolved. String addedJars = conf.get(HIVE_ADDED_JARS); if (StringUtils.isNotEmpty(addedJars)) { - ClassLoader loader = Thread.currentThread().getContextClassLoader(); - ClassLoader newLoader = addToClassPath(loader, addedJars.split(";")); + AddToClassPathAction addAction = new AddToClassPathAction( + Thread.currentThread().getContextClassLoader(), Arrays.asList(addedJars.split(";")) + ); + ClassLoader newLoader = AccessController.doPrivileged(addAction); Thread.currentThread().setContextClassLoader(newLoader); kryo.setClassLoader(newLoader); } @@ -1582,9 +1582,8 @@ public static void mvFileToFinalPath(Path specPath, Configuration hconf, * Check the existence of buckets according to bucket specification. Create empty buckets if * needed. * - * @param hconf + * @param hconf The definition of the FileSink. * @param paths A list of empty buckets to create - * @param conf The definition of the FileSink. * @param reporter The mapreduce reporter object * @throws HiveException * @throws IOException @@ -2076,7 +2075,7 @@ public static void restoreSessionSpecifiedClassLoader(ClassLoader prev) { * @param onestr path string * @return */ - private static URL urlFromPathString(String onestr) { + static URL urlFromPathString(String onestr) { URL oneurl = null; try { if (StringUtils.indexOf(onestr, "file:/") == 0) { @@ -2090,50 +2089,6 @@ private static URL urlFromPathString(String onestr) { return oneurl; } - private static boolean useExistingClassLoader(ClassLoader cl) { - if (!(cl instanceof UDFClassLoader)) { - // Cannot use the same classloader if it is not an instance of {@code UDFClassLoader} - return false; - } - final UDFClassLoader udfClassLoader = (UDFClassLoader) cl; - if (udfClassLoader.isClosed()) { - // The classloader may have been closed, Cannot add to the same instance - return false; - } - return true; - } - - /** - * Add new elements to the classpath. - * - * @param newPaths - * Array of classpath elements - */ - public static ClassLoader addToClassPath(ClassLoader cloader, String[] newPaths) { - final URLClassLoader loader = (URLClassLoader) cloader; - if (useExistingClassLoader(cloader)) { - final UDFClassLoader udfClassLoader = (UDFClassLoader) loader; - for (String path : newPaths) { - udfClassLoader.addURL(urlFromPathString(path)); - } - return udfClassLoader; - } else { - return createUDFClassLoader(loader, newPaths); - } - } - - public static ClassLoader createUDFClassLoader(URLClassLoader loader, String[] newPaths) { - final Set curPathsSet = Sets.newHashSet(loader.getURLs()); - final List curPaths = Lists.newArrayList(curPathsSet); - for (String onestr : newPaths) { - final URL oneurl = urlFromPathString(onestr); - if (oneurl != null && !curPathsSet.contains(oneurl)) { - curPaths.add(oneurl); - } - } - return new UDFClassLoader(curPaths.toArray(new URL[0]), loader); - } - /** * remove elements from the classpath. * @@ -2142,7 +2097,15 @@ public static ClassLoader createUDFClassLoader(URLClassLoader loader, String[] n */ public static void removeFromClassPath(String[] pathsToRemove) throws IOException { Thread curThread = Thread.currentThread(); - URLClassLoader loader = (URLClassLoader) curThread.getContextClassLoader(); + if (!(curThread.getContextClassLoader() instanceof UDFClassLoader)) { + // Current class loader is + // * either system class loader -- we shouldn't close it! + // * or UDFClassLoader -- then we can remove by closing and creating a more limited one + return; + } + + UDFClassLoader loader = (UDFClassLoader) curThread.getContextClassLoader(); + Set newPath = new HashSet(Arrays.asList(loader.getURLs())); for (String onestr : pathsToRemove) { @@ -2152,9 +2115,9 @@ public static void removeFromClassPath(String[] pathsToRemove) throws IOExceptio } } JavaUtils.closeClassLoader(loader); - // This loader is closed, remove it from cached registry loaders to avoid removing it again. + // This loader is closed, remove it from cached registry loaders to avoid removing it again. Registry reg = SessionState.getRegistry(); - if(reg != null) { + if (reg != null) { reg.removeFromUDFLoaders(loader); } diff --git ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecDriver.java ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecDriver.java index 01dd93c527..ab1b52e07e 100644 --- ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecDriver.java +++ ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecDriver.java @@ -24,13 +24,16 @@ import java.io.Serializable; import java.lang.management.ManagementFactory; import java.lang.management.MemoryMXBean; +import java.security.AccessController; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Properties; import org.apache.commons.lang.StringUtils; +import org.apache.hadoop.hive.ql.exec.AddToClassPathAction; import org.apache.hadoop.hive.ql.exec.SerializationUtilities; import org.apache.hadoop.hive.ql.log.LogDivertAppenderForTest; import org.apache.hadoop.mapreduce.MRJobConfig; @@ -744,7 +747,9 @@ public static void main(String[] args) throws IOException, HiveException { // see also - code in CliDriver.java ClassLoader loader = conf.getClassLoader(); if (StringUtils.isNotBlank(libjars)) { - loader = Utilities.addToClassPath(loader, StringUtils.split(libjars, ",")); + AddToClassPathAction addAction = new AddToClassPathAction( + loader, Arrays.asList(StringUtils.split(libjars, ","))); + loader = AccessController.doPrivileged(addAction); } conf.setClassLoader(loader); // Also set this to the Thread ContextClassLoader, so new threads will diff --git ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecMapper.java ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecMapper.java index 91868a4667..10eeba8f98 100644 --- ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecMapper.java +++ ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecMapper.java @@ -73,20 +73,22 @@ private MapredLocalWork localWork = null; private ExecMapperContext execContext = null; + private static String tryGetClassPath(ClassLoader loader) { + if(loader instanceof URLClassLoader) { + return Arrays.asList(((URLClassLoader) loader).getURLs()).toString(); + } else { + return "unavailable for " + loader.getClass().getSimpleName(); + } + } + @Override public void configure(JobConf job) { execContext = new ExecMapperContext(job); // Allocate the bean at the beginning - - try { - l4j.info("conf classpath = " - + Arrays.asList(((URLClassLoader) job.getClassLoader()).getURLs())); - l4j.info("thread classpath = " - + Arrays.asList(((URLClassLoader) Thread.currentThread() - .getContextClassLoader()).getURLs())); - } catch (Exception e) { - l4j.info("cannot get classpath: " + e.getMessage()); + if (l4j.isInfoEnabled()) { + l4j.info("conf classpath = " + tryGetClassPath(job.getClassLoader())); + l4j.info("thread classpath = " + tryGetClassPath(Thread.currentThread().getContextClassLoader())); } - setDone(false); try { diff --git ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecReducer.java ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecReducer.java index e106bc9149..1bd2cc73dc 100644 --- ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecReducer.java +++ ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecReducer.java @@ -88,6 +88,14 @@ private transient Object keyObject; private transient BytesWritable groupKey; + private static String tryGetClassPath(ClassLoader loader) { + if(loader instanceof URLClassLoader) { + return Arrays.asList(((URLClassLoader) loader).getURLs()).toString(); + } else { + return "unavailable for " + loader.getClass().getSimpleName(); + } + } + @Override public void configure(JobConf job) { rowObjectInspector = new ObjectInspector[Byte.MAX_VALUE]; @@ -95,15 +103,8 @@ public void configure(JobConf job) { ObjectInspector keyObjectInspector; if (LOG.isInfoEnabled()) { - try { - LOG.info("conf classpath = " - + Arrays.asList(((URLClassLoader) job.getClassLoader()).getURLs())); - LOG.info("thread classpath = " - + Arrays.asList(((URLClassLoader) Thread.currentThread() - .getContextClassLoader()).getURLs())); - } catch (Exception e) { - LOG.info("cannot get classpath: " + e.getMessage()); - } + LOG.info("conf classpath = " + tryGetClassPath(job.getClassLoader())); + LOG.info("thread classpath = " + tryGetClassPath(Thread.currentThread().getContextClassLoader())); } jc = job; diff --git ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java index f7ea212cfb..2672be6e90 100644 --- ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java +++ ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java @@ -78,6 +78,14 @@ private ScheduledThreadPoolExecutor getMemoryAndRowLogExecutor() { return executor; } + private static String tryGetClassPath(ClassLoader loader) { + if(loader instanceof URLClassLoader) { + return Arrays.asList(((URLClassLoader) loader).getURLs()).toString(); + } else { + return "unavailable for " + loader.getClass().getSimpleName(); + } + } + public void init(JobConf job, OutputCollector output, Reporter reporter) throws Exception { jc = job; MapredContext.init(false, new JobConf(jc)); @@ -89,13 +97,8 @@ private ScheduledThreadPoolExecutor getMemoryAndRowLogExecutor() { LOG.info("maximum memory = " + memoryMXBean.getHeapMemoryUsage().getMax()); MemoryInfoLogger memoryInfoLogger = new MemoryInfoLogger(); memoryInfoLogger.run(); - try { - LOG.info("conf classpath = " + Arrays.asList(((URLClassLoader) job.getClassLoader()).getURLs())); - LOG.info("thread classpath = " + Arrays - .asList(((URLClassLoader) Thread.currentThread().getContextClassLoader()).getURLs())); - } catch (Exception e) { - LOG.info("cannot get classpath: " + e.getMessage()); - } + LOG.info("conf classpath = " + tryGetClassPath(job.getClassLoader())); + LOG.info("thread classpath = " + tryGetClassPath(Thread.currentThread().getContextClassLoader())); } /** diff --git ql/src/java/org/apache/hadoop/hive/ql/exec/tez/RecordProcessor.java ql/src/java/org/apache/hadoop/hive/ql/exec/tez/RecordProcessor.java index 0ec7a04ce7..90454b81ad 100644 --- ql/src/java/org/apache/hadoop/hive/ql/exec/tez/RecordProcessor.java +++ ql/src/java/org/apache/hadoop/hive/ql/exec/tez/RecordProcessor.java @@ -66,6 +66,14 @@ public RecordProcessor(JobConf jConf, ProcessorContext processorContext) { this.processorContext = processorContext; } + private static String tryGetClassPath(ClassLoader loader) { + if(loader instanceof URLClassLoader) { + return Arrays.asList(((URLClassLoader) loader).getURLs()).toString(); + } else { + return "unavailable for " + loader.getClass().getSimpleName(); + } + } + /** * Common initialization code for RecordProcessors * @param mrReporter @@ -82,16 +90,9 @@ void init(MRTaskReporter mrReporter, checkAbortCondition(); //log classpaths - try { - if (l4j.isDebugEnabled()) { - l4j.debug("conf classpath = " - + Arrays.asList(((URLClassLoader) jconf.getClassLoader()).getURLs())); - l4j.debug("thread classpath = " - + Arrays.asList(((URLClassLoader) Thread.currentThread() - .getContextClassLoader()).getURLs())); - } - } catch (Exception e) { - l4j.info("cannot get classpath: " + e.getMessage()); + if (l4j.isDebugEnabled()) { + l4j.debug("conf classpath = " + tryGetClassPath(jconf.getClassLoader())); + l4j.debug("thread classpath = " + tryGetClassPath(Thread.currentThread().getContextClassLoader())); } } diff --git ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java index de5cd8b992..9d631ed43d 100644 --- ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java +++ ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java @@ -27,11 +27,12 @@ import java.lang.management.ManagementFactory; import java.net.URI; import java.net.URISyntaxException; -import java.net.URLClassLoader; +import java.security.AccessController; import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; @@ -72,6 +73,7 @@ import org.apache.hadoop.hive.metastore.conf.MetastoreConf; import org.apache.hadoop.hive.metastore.utils.MetaStoreUtils; import org.apache.hadoop.hive.ql.MapRedStats; +import org.apache.hadoop.hive.ql.exec.AddToClassPathAction; import org.apache.hadoop.hive.ql.exec.FunctionInfo; import org.apache.hadoop.hive.ql.exec.Registry; import org.apache.hadoop.hive.ql.exec.Utilities; @@ -423,7 +425,9 @@ public SessionState(HiveConf conf, String userName) { // classloader as parent can pollute the session. See HIVE-11878 parentLoader = SessionState.class.getClassLoader(); // Make sure that each session has its own UDFClassloader. For details see {@link UDFClassLoader} - final ClassLoader currentLoader = Utilities.createUDFClassLoader((URLClassLoader) parentLoader, new String[]{}); + AddToClassPathAction addAction = new AddToClassPathAction( + parentLoader, Collections.emptyList(), true); + final ClassLoader currentLoader = AccessController.doPrivileged(addAction); this.sessionConf.setClassLoader(currentLoader); resourceDownloader = new ResourceDownloader(conf, HiveConf.getVar(conf, ConfVars.DOWNLOADED_RESOURCES_DIR)); @@ -1325,17 +1329,17 @@ public void loadAuxJars() throws IOException { if (ArrayUtils.isEmpty(jarPaths)) { return; } - - URLClassLoader currentCLoader = - (URLClassLoader) SessionState.get().getConf().getClassLoader(); - currentCLoader = - (URLClassLoader) Utilities.addToClassPath(currentCLoader, jarPaths); + AddToClassPathAction addAction = new AddToClassPathAction( + SessionState.get().getConf().getClassLoader(), Arrays.asList(jarPaths) + ); + final ClassLoader currentCLoader = AccessController.doPrivileged(addAction); sessionConf.setClassLoader(currentCLoader); Thread.currentThread().setContextClassLoader(currentCLoader); } /** * Reload the jars under the path specified in hive.reloadable.aux.jars.path property. + * * @throws IOException */ public void loadReloadableAuxJars() throws IOException { @@ -1350,7 +1354,7 @@ public void loadReloadableAuxJars() throws IOException { Set jarPaths = FileUtils.getJarFilesByPath(renewableJarPath, sessionConf); // load jars under the hive.reloadable.aux.jars.path - if(!jarPaths.isEmpty()){ + if (!jarPaths.isEmpty()) { reloadedAuxJars.addAll(jarPaths); } @@ -1360,11 +1364,9 @@ public void loadReloadableAuxJars() throws IOException { } if (reloadedAuxJars != null && !reloadedAuxJars.isEmpty()) { - URLClassLoader currentCLoader = - (URLClassLoader) SessionState.get().getConf().getClassLoader(); - currentCLoader = - (URLClassLoader) Utilities.addToClassPath(currentCLoader, - reloadedAuxJars.toArray(new String[0])); + AddToClassPathAction addAction = new AddToClassPathAction( + SessionState.get().getConf().getClassLoader(), reloadedAuxJars); + final ClassLoader currentCLoader = AccessController.doPrivileged(addAction); sessionConf.setClassLoader(currentCLoader); Thread.currentThread().setContextClassLoader(currentCLoader); } @@ -1375,8 +1377,9 @@ public void loadReloadableAuxJars() throws IOException { static void registerJars(List newJars) throws IllegalArgumentException { LogHelper console = getConsole(); try { - ClassLoader loader = Thread.currentThread().getContextClassLoader(); - ClassLoader newLoader = Utilities.addToClassPath(loader, newJars.toArray(new String[0])); + AddToClassPathAction addAction = new AddToClassPathAction( + Thread.currentThread().getContextClassLoader(), newJars); + final ClassLoader newLoader = AccessController.doPrivileged(addAction); Thread.currentThread().setContextClassLoader(newLoader); SessionState.get().getConf().setClassLoader(newLoader); console.printInfo("Added " + newJars + " to class path"); diff --git ql/src/test/org/apache/hadoop/hive/ql/exec/TestAddToClassPathAction.java ql/src/test/org/apache/hadoop/hive/ql/exec/TestAddToClassPathAction.java new file mode 100644 index 0000000000..e524bb5772 --- /dev/null +++ ql/src/test/org/apache/hadoop/hive/ql/exec/TestAddToClassPathAction.java @@ -0,0 +1,142 @@ +/* + * 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.hive.ql.exec; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.IOException; +import java.net.URL; +import java.security.AccessController; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.fail; + +/** + * Minimal tests for AddToClassPathAction class. Most of the tests don't use + * {@link java.security.AccessController#doPrivileged(java.security.PrivilegedAction)}, + * presumably the tests will not be executed under security manager. + */ +public class TestAddToClassPathAction { + + private ClassLoader originalClassLoader; + + private static void assertURLsMatch(String message, List expected, URL[] actual) { + List actualStrings = Arrays.stream(actual).map(URL::toExternalForm).collect(Collectors.toList()); + assertEquals(message, expected, actualStrings); + } + + private static void assertURLsMatch(List expected, URL[] actual) { + assertURLsMatch("", expected, actual); + } + + @Before + public void saveClassLoader() { + originalClassLoader = Thread.currentThread().getContextClassLoader(); + } + + @After + public void restoreClassLoader() { + Thread.currentThread().setContextClassLoader(originalClassLoader); + } + + @Test + public void testNullClassLoader() { + try { + new AddToClassPathAction(null, Collections.emptyList()); + fail("When pafrent class loader is null, IllegalArgumentException is expected!"); + } catch (IllegalArgumentException e) { + // pass + } + } + + @Test + public void testNullPaths() { + ClassLoader rootLoader = Thread.currentThread().getContextClassLoader(); + AddToClassPathAction action = new AddToClassPathAction(rootLoader, null); + UDFClassLoader childLoader = action.run(); + assertURLsMatch( + "When newPaths is null, loader shall be created normally with no extra paths.", + Collections.emptyList(), childLoader.getURLs()); + } + + @Test + public void testUseExisting() { + ClassLoader rootLoader = Thread.currentThread().getContextClassLoader(); + AddToClassPathAction action1 = new AddToClassPathAction(rootLoader, Arrays.asList("/a/1", "/c/3")); + UDFClassLoader parentLoader = action1.run(); + AddToClassPathAction action2 = new AddToClassPathAction(parentLoader, Arrays.asList("/b/2", "/d/4")); + UDFClassLoader childLoader = action2.run(); + assertSame( + "Normally, the existing class loader should be reused (not closed, no force new).", + parentLoader, childLoader); + assertURLsMatch( + "The class path of the class loader should be updated.", + Arrays.asList("file:/a/1", "file:/c/3", "file:/b/2", "file:/d/4"), childLoader.getURLs()); + } + + @Test + public void testClosed() throws IOException { + ClassLoader rootLoader = Thread.currentThread().getContextClassLoader(); + AddToClassPathAction action1 = new AddToClassPathAction(rootLoader, Arrays.asList("/a/1", "/c/3")); + UDFClassLoader parentLoader = action1.run(); + parentLoader.close(); + AddToClassPathAction action2 = new AddToClassPathAction(parentLoader, Arrays.asList("/b/2", "/d/4")); + UDFClassLoader childLoader = action2.run(); + assertNotSame( + "When the parent class loader is closed, a new instance must be created.", + parentLoader, childLoader); + assertURLsMatch(Arrays.asList("file:/b/2", "file:/d/4"), childLoader.getURLs()); + } + + @Test + public void testForceNew() { + ClassLoader rootLoader = Thread.currentThread().getContextClassLoader(); + AddToClassPathAction action1 = new AddToClassPathAction(rootLoader, Arrays.asList("/a/1", "/c/3")); + UDFClassLoader parentLoader = action1.run(); + AddToClassPathAction action2 = new AddToClassPathAction(parentLoader, Arrays.asList("/b/2", "/d/4"), true); + UDFClassLoader childLoader = action2.run(); + assertNotSame( + "When forceNewClassLoader is set, a new instance must be created.", + parentLoader, childLoader); + assertURLsMatch(Arrays.asList("file:/b/2", "file:/d/4"), childLoader.getURLs()); + } + + @Test + public void testLegalPaths() { + ClassLoader rootLoader = Thread.currentThread().getContextClassLoader(); + List newPaths = Arrays.asList("file://a/aa", "c/cc", "/bb/b"); + String userDir = System.getProperty("user.dir"); + List expectedURLs = Arrays.asList( + "file://a/aa", + "file:" + userDir + "/c/cc", + "file:/bb/b"); + AddToClassPathAction action = new AddToClassPathAction(rootLoader, newPaths); + UDFClassLoader loader = AccessController.doPrivileged(action); + assertURLsMatch(expectedURLs, loader.getURLs()); + } + +} diff --git spark-client/src/main/java/org/apache/hive/spark/client/SparkClientUtilities.java spark-client/src/main/java/org/apache/hive/spark/client/SparkClientUtilities.java index b434d8f7b7..4f8d88046e 100644 --- spark-client/src/main/java/org/apache/hive/spark/client/SparkClientUtilities.java +++ spark-client/src/main/java/org/apache/hive/spark/client/SparkClientUtilities.java @@ -28,6 +28,7 @@ import java.net.URL; import java.net.URLClassLoader; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -51,16 +52,33 @@ public static final String HIVE_KRYO_REG_NAME = "org.apache.hive.spark.HiveKryoRegistrator"; private static final String HIVE_KRYO_REG_JAR_NAME = "hive-kryo-registrator"; private static final ImmutableList ERROR_KEYWORDS = ImmutableList.of("error", "exception"); + /** * Add new elements to the classpath. + * Returns currently known class paths as best effort. For system class loader, this may return empty. + * In such cases we will anyway create new child class loader in {@link #addToClassPath(Map, Configuration, File)}, + * so all new class paths will be added and next time we will have a URLClassLoader to work with. + */ + private static List getCurrentClassPaths(ClassLoader parentLoader) { + if(parentLoader instanceof URLClassLoader) { + return Lists.newArrayList(((URLClassLoader) parentLoader).getURLs()); + } else { + return Collections.emptyList(); + } + } + + /** + * Add new elements to the classpath by creating a child ClassLoader containing both old and new paths. + * This method supports downloading HDFS files to local FS if missing from cache or later timestamp. + * However, this method has no tricks working around HIVE-11878, like UDFClassLoader.... * * @param newPaths Map of classpath elements and corresponding timestamp * @return locally accessible files corresponding to the newPaths */ public static List addToClassPath(Map newPaths, Configuration conf, File localTmpDir) throws Exception { - URLClassLoader loader = (URLClassLoader) Thread.currentThread().getContextClassLoader(); - List curPath = Lists.newArrayList(loader.getURLs()); + ClassLoader parentLoader = Thread.currentThread().getContextClassLoader(); + List curPath = getCurrentClassPaths(parentLoader); List localNewPaths = new ArrayList<>(); boolean newPathAdded = false; @@ -76,7 +94,7 @@ if (newPathAdded) { URLClassLoader newLoader = - new URLClassLoader(curPath.toArray(new URL[curPath.size()]), loader); + new URLClassLoader(curPath.toArray(new URL[curPath.size()]), parentLoader); Thread.currentThread().setContextClassLoader(newLoader); } return localNewPaths; diff --git standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java index 0642b39f58..5bd23b7748 100644 --- standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java +++ standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java @@ -17,14 +17,13 @@ */ package org.apache.hadoop.hive.metastore.utils; -import java.beans.PropertyDescriptor; import java.io.File; import java.net.URL; import java.net.URLClassLoader; import java.text.DateFormat; import java.text.SimpleDateFormat; import java.util.ArrayList; -import java.util.Arrays; +import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -33,13 +32,13 @@ import java.util.function.Predicate; import java.util.regex.Matcher; import java.util.regex.Pattern; -import java.util.regex.PatternSyntaxException; import java.util.stream.Collectors; import static java.util.regex.Pattern.compile; import javax.annotation.Nullable; +import com.google.common.collect.Lists; import org.apache.commons.lang.StringUtils; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.CommonConfigurationKeysPublic; @@ -378,6 +377,19 @@ public static int getArchivingLevel(Partition part) throws MetaException { return HadoopThriftAuthBridge.getBridge().getHadoopSaslProperties(conf); } + /** + * Returns currently known class paths as best effort. For system class loader, this may return + * In such cases we will anyway create new child class loader in {@link #addToClassPath(ClassLo + * so all new class paths will be added and next time we will have a URLClassLoader to work wit + */ + private static List getCurrentClassPaths(ClassLoader parentLoader) { + if(parentLoader instanceof URLClassLoader) { + return Lists.newArrayList(((URLClassLoader) parentLoader).getURLs()); + } else { + return Collections.emptyList(); + } + } + /** * Add new elements to the classpath. * @@ -385,8 +397,7 @@ public static int getArchivingLevel(Partition part) throws MetaException { * Array of classpath elements */ public static ClassLoader addToClassPath(ClassLoader cloader, String[] newPaths) throws Exception { - URLClassLoader loader = (URLClassLoader) cloader; - List curPath = Arrays.asList(loader.getURLs()); + List curPath = getCurrentClassPaths(cloader); ArrayList newPath = new ArrayList<>(curPath.size()); // get a list with the current classpath components @@ -402,7 +413,7 @@ public static ClassLoader addToClassPath(ClassLoader cloader, String[] newPaths) } } - return new URLClassLoader(curPath.toArray(new URL[0]), loader); + return new URLClassLoader(curPath.toArray(new URL[0]), cloader); } /**