This is an automated email from the ASF dual-hosted git repository.
srowen pushed a commit to branch branch-3.2
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-3.2 by this push:
new 043dd53 [SPARK-36704][CORE] Expand exception handling to more Java 9
cases where reflection is limited at runtime, when reflecting to manage
DirectByteBuffer settings
043dd53 is described below
commit 043dd531a0a23b836d6f3795e4e5e949d4cdee7c
Author: Sean Owen <sro...@gmail.com>
AuthorDate: Sat Sep 11 13:38:10 2021 -0500
[SPARK-36704][CORE] Expand exception handling to more Java 9 cases where
reflection is limited at runtime, when reflecting to manage DirectByteBuffer
settings
### What changes were proposed in this pull request?
Improve exception handling in the Platform initialization, where it
attempts to assess whether reflection is possible to modify DirectByteBuffer.
This can apparently fail in more cases on Java 9+ than are currently handled,
whereas Spark can continue without reflection if needed.
More detailed comments on the change inline.
### Why are the changes needed?
This exception seems to be possible and fails startup:
```
Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make
private java.nio.DirectByteBuffer(long,int) accessible: module java.base does
not "opens java.nio" to unnamed module 71e9ddb4
at
java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:357)
at
java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
at
java.base/java.lang.reflect.Constructor.checkCanSetAccessible(Constructor.java:188)
at
java.base/java.lang.reflect.Constructor.setAccessible(Constructor.java:181)
at org.apache.spark.unsafe.Platform.<clinit>(Platform.java:56)
```
### Does this PR introduce _any_ user-facing change?
Should strictly allow Spark to continue in more cases.
### How was this patch tested?
Existing tests.
Closes #33947 from srowen/SPARK-36704.
Authored-by: Sean Owen <sro...@gmail.com>
Signed-off-by: Sean Owen <sro...@gmail.com>
(cherry picked from commit e5283f5ed5efa5bf3652c3959166f59dc5b5daaa)
Signed-off-by: Sean Owen <sro...@gmail.com>
---
.../java/org/apache/spark/unsafe/Platform.java | 87 +++++++++++++---------
1 file changed, 51 insertions(+), 36 deletions(-)
diff --git a/common/unsafe/src/main/java/org/apache/spark/unsafe/Platform.java
b/common/unsafe/src/main/java/org/apache/spark/unsafe/Platform.java
index dc8d6e3..1286762 100644
--- a/common/unsafe/src/main/java/org/apache/spark/unsafe/Platform.java
+++ b/common/unsafe/src/main/java/org/apache/spark/unsafe/Platform.java
@@ -45,30 +45,20 @@ public final class Platform {
private static final boolean unaligned;
- // Access fields and constructors once and store them, for performance:
-
- private static final Constructor<?> DBB_CONSTRUCTOR;
- private static final Field DBB_CLEANER_FIELD;
- static {
- try {
- Class<?> cls = Class.forName("java.nio.DirectByteBuffer");
- Constructor<?> constructor = cls.getDeclaredConstructor(Long.TYPE,
Integer.TYPE);
- constructor.setAccessible(true);
- Field cleanerField = cls.getDeclaredField("cleaner");
- cleanerField.setAccessible(true);
- DBB_CONSTRUCTOR = constructor;
- DBB_CLEANER_FIELD = cleanerField;
- } catch (ClassNotFoundException | NoSuchMethodException |
NoSuchFieldException e) {
- throw new IllegalStateException(e);
- }
- }
-
// Split java.version on non-digit chars:
private static final int majorVersion =
Integer.parseInt(System.getProperty("java.version").split("\\D+")[0]);
+ // Access fields and constructors once and store them, for performance:
+ private static final Constructor<?> DBB_CONSTRUCTOR;
+ private static final Field DBB_CLEANER_FIELD;
private static final Method CLEANER_CREATE_METHOD;
+
static {
+ // At the end of this block, CLEANER_CREATE_METHOD should be non-null iff
it's possible to use
+ // reflection to invoke it, which is not necessarily possible by default
in Java 9+.
+ // Code below can test for null to see whether to use it.
+
// The implementation of Cleaner changed from JDK 8 to 9
String cleanerClassName;
if (majorVersion < 9) {
@@ -77,28 +67,53 @@ public final class Platform {
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;
+ } else {
+ throw re;
+ }
}
- CLEANER_CREATE_METHOD = createMethod;
- } catch (ClassNotFoundException | NoSuchMethodException e) {
+ // Have to set these values no matter what:
+ DBB_CONSTRUCTOR = constructor;
+ DBB_CLEANER_FIELD = cleanerField;
+
+ // no point continuing if the above failed:
+ if (DBB_CONSTRUCTOR != null && DBB_CLEANER_FIELD != null) {
+ 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:
+ try {
+ createMethod.invoke(null, null, null);
+ } catch (IllegalAccessException e) {
+ // Don't throw an exception, but can't log here?
+ createMethod = null;
+ }
+ CLEANER_CREATE_METHOD = createMethod;
+ } else {
+ CLEANER_CREATE_METHOD = null;
+ }
+ } catch (ClassNotFoundException | NoSuchMethodException |
NoSuchFieldException e) {
+ // These are all fatal in any Java version - rethrow (have to wrap as
this is a static block)
throw new IllegalStateException(e);
+ } catch (InvocationTargetException ite) {
+ throw new IllegalStateException(ite.getCause());
}
-
}
/**
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org
