You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/09/10 03:21:20 UTC
[GitHub] [spark] mridulm commented on a change in pull request #33947: [SPARK-36704][CORE] Expand exception handling to more Java 9 cases where reflection is limited at runtime, when reflecting to manage DirectByteBuffer settings
mridulm commented on a change in pull request #33947:
URL: https://github.com/apache/spark/pull/33947#discussion_r705866414
##########
File path: common/unsafe/src/main/java/org/apache/spark/unsafe/Platform.java
##########
@@ -77,28 +67,52 @@
cleanerClassName = "jdk.internal.ref.Cleaner";
}
try {
- Class<?> cleanerClass = Class.forName(cleanerClassName);
- Method createMethod = cleanerClass.getMethod("create", Object.class, Runnable.class);
- // Accessing jdk.internal.ref.Cleaner should actually fail by default in JDK 9+,
- // unfortunately, unless the user has allowed access with something like
- // --add-opens java.base/java.lang=ALL-UNNAMED If not, we can't really use the Cleaner
- // hack below. It doesn't break, just means the user might run into the default JVM limit
- // on off-heap memory and increase it or set the flag above. This tests whether it's
- // available:
+ Class<?> cls = Class.forName("java.nio.DirectByteBuffer");
+ Constructor<?> constructor = cls.getDeclaredConstructor(Long.TYPE, Integer.TYPE);
+ Field cleanerField = cls.getDeclaredField("cleaner");
try {
- createMethod.invoke(null, null, null);
- } catch (IllegalAccessException e) {
- // Don't throw an exception, but can't log here?
- createMethod = null;
- } catch (InvocationTargetException ite) {
- // shouldn't happen; report it
- throw new IllegalStateException(ite);
+ constructor.setAccessible(true);
+ cleanerField.setAccessible(true);
+ } catch (RuntimeException re) {
+ // This is a Java 9+ exception, so needs to be handled without importing it
+ if ("InaccessibleObjectException".equals(re.getClass().getSimpleName())) {
+ // Continue, but the constructor/field are not available
+ // See comment below for more context
+ constructor = null;
+ cleanerField = null;
Review comment:
Given the `throw`, why do we need this ? Class wont be initialized right ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org