[SPARK-40729][CORE] Make Spark use `UnsafeFieldAccessor` as default for Java 18+ #38190

LuciferYang · 2022-10-10T12:40:50Z

What changes were proposed in this pull request?

This pr add -Djdk.reflect.useDirectMethodHandle=false to JavaModuleOptions and maven/sbt extraJavaTestArgs to make Spark use UnsafeFieldAccessor as default with Java 18/19 to avoid the bad case described in SPARK-40729 .

Why are the changes needed?

After JEP 416: Reimplement Core Reflection with Method Handles, MethodHandleAccessor 'is the default reflection implementation of Java, but in Spark it will cause the bad case mentioned in SPARK-40729, so add -Djdk.reflect.useDirectMethodHandle=false` as a workaround for Java 18/19.

Does this PR introduce any user-facing change?

No, The new option will not affect Java versions below 18

How was this patch tested?

Pass GitHub Actions
Manual test:

run repl module test with Java 18/19

Before

- broadcast vars *** FAILED ***
  isContain was true Interpreter output contained 'Exception':
  Welcome to
        ____              __
       / __/__  ___ _____/ /__
      _\ \/ _ \/ _ `/ __/  '_/
     /___/ .__/\_,_/_/ /_/\_\   version 3.4.0-SNAPSHOT
        /_/
           
  Using Scala version 2.12.17 (OpenJDK 64-Bit Server VM, Java 19)
  Type in expressions to have them evaluated.
  Type :help for more information.
  
  scala> 
  scala> array: Array[Int] = Array(0, 0, 0, 0, 0)
  
  scala> broadcastArray: org.apache.spark.broadcast.Broadcast[Array[Int]] = Broadcast(0)
  
  scala> java.lang.InternalError: java.lang.IllegalAccessException: final field has no write access: $Lambda$3029/0x0000000801d80a30.arg$1/putField, from class java.lang.Object (module java.base)
    at java.base/jdk.internal.reflect.MethodHandleAccessorFactory.newFieldAccessor(MethodHandleAccessorFactory.java:167)
    at java.base/jdk.internal.reflect.ReflectionFactory.newFieldAccessor(ReflectionFactory.java:145)
    at java.base/java.lang.reflect.Field.acquireOverrideFieldAccessor(Field.java:1184)
    at java.base/java.lang.reflect.Field.getOverrideFieldAccessor(Field.java:1153)
    at java.base/java.lang.reflect.Field.set(Field.java:820)
    at org.apache.spark.util.ClosureCleaner$.clean(ClosureCleaner.scala:406)
    at org.apache.spark.util.ClosureCleaner$.clean(ClosureCleaner.scala:163)
    at org.apache.spark.SparkContext.clean(SparkContext.scala:2502)
    at org.apache.spark.rdd.RDD.$anonfun$map$1(RDD.scala:413)
    at org.apache.spark.rdd.RDDOperationScope$.withScope(RDDOperationScope.scala:151)
    at org.apache.spark.rdd.RDDOperationScope$.withScope(RDDOperationScope.scala:112)
    at org.apache.spark.rdd.RDD.withScope(RDD.scala:405)
    at org.apache.spark.rdd.RDD.map(RDD.scala:412)
    ... 93 elided
  Caused by: java.lang.IllegalAccessException: final field has no write access: $Lambda$3029/0x0000000801d80a30.arg$1/putField, from class java.lang.Object (module java.base)
    at java.base/java.lang.invoke.MemberName.makeAccessException(MemberName.java:955)
    at java.base/java.lang.invoke.MethodHandles$Lookup.unreflectField(MethodHandles.java:3511)
    at java.base/java.lang.invoke.MethodHandles$Lookup.unreflectSetter(MethodHandles.java:3502)
    at java.base/java.lang.invoke.MethodHandleImpl$1.unreflectField(MethodHandleImpl.java:1630)
    at java.base/jdk.internal.reflect.MethodHandleAccessorFactory.newFieldAccessor(MethodHandleAccessorFactory.java:145)
    ... 105 more
  
  scala> 
  scala> java.lang.InternalError: java.lang.IllegalAccessException: final field has no write access: $Lambda$3061/0x0000000801e01000.arg$1/putField, from class java.lang.Object (module java.base)
    at java.base/jdk.internal.reflect.MethodHandleAccessorFactory.newFieldAccessor(MethodHandleAccessorFactory.java:167)
    at java.base/jdk.internal.reflect.ReflectionFactory.newFieldAccessor(ReflectionFactory.java:145)
    at java.base/java.lang.reflect.Field.acquireOverrideFieldAccessor(Field.java:1184)
    at java.base/java.lang.reflect.Field.getOverrideFieldAccessor(Field.java:1153)
    at java.base/java.lang.reflect.Field.set(Field.java:820)
    at org.apache.spark.util.ClosureCleaner$.clean(ClosureCleaner.scala:406)
    at org.apache.spark.util.ClosureCleaner$.clean(ClosureCleaner.scala:163)
    at org.apache.spark.SparkContext.clean(SparkContext.scala:2502)
    at org.apache.spark.rdd.RDD.$anonfun$map$1(RDD.scala:413)
    at org.apache.spark.rdd.RDDOperationScope$.withScope(RDDOperationScope.scala:151)
    at org.apache.spark.rdd.RDDOperationScope$.withScope(RDDOperationScope.scala:112)
    at org.apache.spark.rdd.RDD.withScope(RDD.scala:405)
    at org.apache.spark.rdd.RDD.map(RDD.scala:412)
    ... 93 elided
  Caused by: java.lang.IllegalAccessException: final field has no write access: $Lambda$3061/0x0000000801e01000.arg$1/putField, from class java.lang.Object (module java.base)
    at java.base/java.lang.invoke.MemberName.makeAccessException(MemberName.java:955)
    at java.base/java.lang.invoke.MethodHandles$Lookup.unreflectField(MethodHandles.java:3511)
    at java.base/java.lang.invoke.MethodHandles$Lookup.unreflectSetter(MethodHandles.java:3502)
    at java.base/java.lang.invoke.MethodHandleImpl$1.unreflectField(MethodHandleImpl.java:1630)
    at java.base/jdk.internal.reflect.MethodHandleAccessorFactory.newFieldAccessor(MethodHandleAccessorFactory.java:145)
    ... 105 more
  
  scala>      | 
  scala> :quit (ReplSuite.scala:83)

After

Run completed in 1 minute, 12 seconds.
Total number of tests run: 44
Suites: completed 7, aborted 0
Tests: succeeded 44, failed 0, canceled 0, ignored 0, pending 0
All tests passed.

test spark-shell with Java 18/19:

Before

bin/spark-shell --master local
Setting default log level to "WARN".
To adjust logging level use sc.setLogLevel(newLevel). For SparkR, use setLogLevel(newLevel).
22/10/11 19:13:08 WARN NativeCodeLoader: Unable to load native-hadoop library for your platform... using builtin-java classes where applicable
22/10/11 19:13:08 WARN Utils: Service 'SparkUI' could not bind on port 4040. Attempting port 4041.
Spark context Web UI available at http://localhost:4041
Spark context available as 'sc' (master = local, app id = local-1665486788733).
Spark session available as 'spark'.
Welcome to
      ____              __
     / __/__  ___ _____/ /__
    _\ \/ _ \/ _ `/ __/  '_/
   /___/ .__/\_,_/_/ /_/\_\   version 3.4.0-SNAPSHOT
      /_/
         
Using Scala version 2.12.17 (OpenJDK 64-Bit Server VM, Java 18.0.2.1)
Type in expressions to have them evaluated.
Type :help for more information.

scala> :paste
// Entering paste mode (ctrl-D to finish)

var array = new Array[Int](5)
val broadcastArray = sc.broadcast(array)
sc.parallelize(0 to 4).map(x => broadcastArray.value(x)).collect()
array(0) = 5
sc.parallelize(0 to 4).map(x => broadcastArray.value(x)).collect()

// Exiting paste mode, now interpreting.

java.lang.InternalError: java.lang.IllegalAccessException: final field has no write access: $Lambda$2396/0x00000008015b2e70.arg$1/putField, from class java.lang.Object (module java.base)
  at java.base/jdk.internal.reflect.MethodHandleAccessorFactory.newFieldAccessor(MethodHandleAccessorFactory.java:167)
  at java.base/jdk.internal.reflect.ReflectionFactory.newFieldAccessor(ReflectionFactory.java:176)
  at java.base/java.lang.reflect.Field.acquireOverrideFieldAccessor(Field.java:1184)
  at java.base/java.lang.reflect.Field.getOverrideFieldAccessor(Field.java:1153)
  at java.base/java.lang.reflect.Field.set(Field.java:820)
  at org.apache.spark.util.ClosureCleaner$.clean(ClosureCleaner.scala:406)
  at org.apache.spark.util.ClosureCleaner$.clean(ClosureCleaner.scala:163)
  at org.apache.spark.SparkContext.clean(SparkContext.scala:2502)
  at org.apache.spark.rdd.RDD.$anonfun$map$1(RDD.scala:413)
  at org.apache.spark.rdd.RDDOperationScope$.withScope(RDDOperationScope.scala:151)
  at org.apache.spark.rdd.RDDOperationScope$.withScope(RDDOperationScope.scala:112)
  at org.apache.spark.rdd.RDD.withScope(RDD.scala:405)
  at org.apache.spark.rdd.RDD.map(RDD.scala:412)
  ... 43 elided
Caused by: java.lang.IllegalAccessException: final field has no write access: $Lambda$2396/0x00000008015b2e70.arg$1/putField, from class java.lang.Object (module java.base)
  at java.base/java.lang.invoke.MemberName.makeAccessException(MemberName.java:955)
  at java.base/java.lang.invoke.MethodHandles$Lookup.unreflectField(MethodHandles.java:3494)
  at java.base/java.lang.invoke.MethodHandles$Lookup.unreflectSetter(MethodHandles.java:3485)
  at java.base/java.lang.invoke.MethodHandleImpl$1.unreflectField(MethodHandleImpl.java:1637)
  at java.base/jdk.internal.reflect.MethodHandleAccessorFactory.newFieldAccessor(MethodHandleAccessorFactory.jav

After

bin/spark-shell --master local
Setting default log level to "WARN".
To adjust logging level use sc.setLogLevel(newLevel). For SparkR, use setLogLevel(newLevel).
22/10/11 19:11:21 WARN NativeCodeLoader: Unable to load native-hadoop library for your platform... using builtin-java classes where applicable
22/10/11 19:11:21 WARN Utils: Service 'SparkUI' could not bind on port 4040. Attempting port 4041.
Spark context Web UI available at http://localhost:4041
Spark context available as 'sc' (master = local, app id = local-1665486681920).
Spark session available as 'spark'.
Welcome to
      ____              __
     / __/__  ___ _____/ /__
    _\ \/ _ \/ _ `/ __/  '_/
   /___/ .__/\_,_/_/ /_/\_\   version 3.4.0-SNAPSHOT
      /_/
         
Using Scala version 2.12.17 (OpenJDK 64-Bit Server VM, Java 18.0.2.1)
Type in expressions to have them evaluated.
Type :help for more information.

scala> :paste
// Entering paste mode (ctrl-D to finish)

var array = new Array[Int](5)
val broadcastArray = sc.broadcast(array)
sc.parallelize(0 to 4).map(x => broadcastArray.value(x)).collect()
array(0) = 5
sc.parallelize(0 to 4).map(x => broadcastArray.value(x)).collect()

// Exiting paste mode, now interpreting.

array: Array[Int] = Array(5, 0, 0, 0, 0)
broadcastArray: org.apache.spark.broadcast.Broadcast[Array[Int]] = Broadcast(0)
res0: Array[Int] = Array(5, 0, 0, 0, 0)

LuciferYang · 2022-10-11T09:13:09Z

spark/core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala

Lines 397 to 408 in 8e85393

    
           val outerField = func.getClass.getDeclaredField("arg$1") 
        
           // SPARK-37072: When Java 17 is used and `outerField` is read-only, 
        
           // the content of `outerField` cannot be set by reflect api directly. 
        
           // But we can remove the `final` modifier of `outerField` before set value 
        
           // and reset the modifier after set value. 
        
           val modifiersField = getFinalModifiersFieldForJava17(outerField) 
        
           modifiersField 
        
             .foreach(m => m.setInt(outerField, outerField.getModifiers & ~Modifier.FINAL)) 
        
           outerField.setAccessible(true) 
        
           outerField.set(func, clonedOuterThis) 
        
           modifiersField 
        
             .foreach(m => m.setInt(outerField, outerField.getModifiers | Modifier.FINAL))

outerField.set(func, clonedOuterThis) threw java.lang.IllegalAccessException: final field has no write access: $Lambda$3061/0x0000000801e01000.arg$1/putField, from class java.lang.Object (module java.base), although we have deleted FINAL Modifier.

Any suggestions for code level fix? -Djdk.reflect.useDirectMethodHandle=false will be also deleted in the future

LuciferYang · 2022-10-11T09:18:49Z

cc @rednaxelafx

dongjoon-hyun · 2022-10-11T21:07:18Z

pom.xml

            <filereports>SparkTestSuite.txt</filereports>
-            <argLine>-ea -Xmx4g -Xss4m -XX:MaxMetaspaceSize=2g -XX:ReservedCodeCacheSize=${CodeCacheSize} ${extraJavaTestArgs} -Dio.netty.tryReflectionSetAccessible=true</argLine>
+            <argLine>-ea -Xmx4g -Xss4m -XX:MaxMetaspaceSize=2g -XX:ReservedCodeCacheSize=${CodeCacheSize} ${extraJavaTestArgs} -Dio.netty.tryReflectionSetAccessible=true
+              -Djdk.reflect.useDirectMethodHandle=false</argLine>


What about the vanilla Scala REPL? Is this only Apache Spark REPL issue?

The issue seem to be that the ClosureCleaner is trying to set a final field which shouldn't be allowed. This is Spark-specific (and affects all other projects that copied Spark's ClosureCleaner), and doesn't exist in the vanilla Scala REPL.

Yes, only Apache Spark REPL issue?

Got it. Thank you, @rednaxelafx and @LuciferYang .

dongjoon-hyun

If this is required, why don't we put this into JavaModuleOptions like the other Java options?

LuciferYang · 2022-10-12T03:11:37Z

If this is required, why don't we put this into JavaModuleOptions like the other Java options?

Yes, this is a feasible way, let me try it.

LuciferYang · 2022-10-12T03:37:15Z

launcher/src/main/java/org/apache/spark/launcher/JavaModuleOptions.java

      "--add-opens=java.base/sun.util.calendar=ALL-UNNAMED",
-      "--add-opens=java.security.jgss/sun.security.krb5=ALL-UNNAMED"};
+      "--add-opens=java.security.jgss/sun.security.krb5=ALL-UNNAMED",
+      "-Djdk.reflect.useDirectMethodHandle=false"};


I think it is better to rename JavaModuleOptions as JavaExtraOptions. Do you suggest completing the corresponding refactor work in this pr? @dongjoon-hyun

My idea is that this pr solves the issue first if need, and then do the rename work by a separate pr

Ya, I agree with you. Let's postpone the renaming.

Thanks @dongjoon-hyun

dongjoon-hyun

+1, LGTM. Thank you, @LuciferYang and @rednaxelafx .
Merged to master for Apache Spark 3.4.0.

LuciferYang · 2022-10-12T06:34:07Z

thanks @dongjoon-hyun @rednaxelafx

LuciferYang added 4 commits October 10, 2022 20:31

fix

994f23d

fix

dccfd01

add to sbt

ebce0f0

fix comments

e74d457

github-actions bot added BUILD SPARK SHELL labels Oct 10, 2022

LuciferYang marked this pull request as draft October 10, 2022 12:57

fix scala test

1da9f8b

LuciferYang closed this Oct 10, 2022

fix sbt

ba464ee

LuciferYang reopened this Oct 11, 2022

dongjoon-hyun reviewed Oct 11, 2022

View reviewed changes

add -Djdk.reflect.useDirectMethodHandle=false as default

1ae2874

LuciferYang changed the title ~~[WIP][SPARK-40729][BUILD][SHELL] Use -Djdk.reflect.useDirectMethodHandle=false to make Java18/19 use UnsafeFieldAccessor in Spark-Shell~~ [WIP][SPARK-40729][CORE] Make Spark use UnsafeFieldAccessor as default with Java18/19 Oct 12, 2022

LuciferYang commented Oct 12, 2022

View reviewed changes

LuciferYang marked this pull request as ready for review October 12, 2022 05:26

LuciferYang changed the title ~~[WIP][SPARK-40729][CORE] Make Spark use UnsafeFieldAccessor as default with Java18/19~~ [SPARK-40729][CORE] Make Spark use UnsafeFieldAccessor as default with Java18/19 Oct 12, 2022

dongjoon-hyun approved these changes Oct 12, 2022

View reviewed changes

dongjoon-hyun changed the title ~~[SPARK-40729][CORE] Make Spark use UnsafeFieldAccessor as default with Java18/19~~ [SPARK-40729][CORE] Make Spark use UnsafeFieldAccessor as default for Java 18+ Oct 12, 2022

dongjoon-hyun closed this in 290e8a7 Oct 12, 2022

[SPARK-40729][CORE] Make Spark use UnsafeFieldAccessor as default for Java 18+ #38190

[SPARK-40729][CORE] Make Spark use UnsafeFieldAccessor as default for Java 18+ #38190

Uh oh!

Conversation

LuciferYang commented Oct 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

LuciferYang commented Oct 11, 2022

Uh oh!

LuciferYang commented Oct 11, 2022

Uh oh!

dongjoon-hyun Oct 11, 2022

Choose a reason for hiding this comment

Uh oh!

rednaxelafx Oct 11, 2022

Choose a reason for hiding this comment

Uh oh!

LuciferYang Oct 12, 2022

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Oct 12, 2022

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LuciferYang commented Oct 12, 2022

Uh oh!

LuciferYang Oct 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LuciferYang Oct 12, 2022

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Oct 12, 2022

Choose a reason for hiding this comment

Uh oh!

LuciferYang Oct 12, 2022

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

LuciferYang commented Oct 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[SPARK-40729][CORE] Make Spark use `UnsafeFieldAccessor` as default for Java 18+ #38190

[SPARK-40729][CORE] Make Spark use `UnsafeFieldAccessor` as default for Java 18+ #38190

LuciferYang commented Oct 10, 2022 •

edited

Loading

dongjoon-hyun left a comment •

edited

Loading

LuciferYang Oct 12, 2022 •

edited

Loading