[GitHub] [spark] gerashegalov commented on a change in pull request #31540: [SPARK-20977][CORE] Use a non-final field for the state of CollectionAccumulator

[GitBox <gi...@apache.org>]

gerashegalov commented on a change in pull request #31540:
URL: https://github.com/apache/spark/pull/31540#discussion_r578782284



##########
File path: core/src/main/scala/org/apache/spark/util/AccumulatorV2.scala
##########
@@ -449,39 +449,46 @@ class DoubleAccumulator extends AccumulatorV2[jl.Double, jl.Double] {
  * @since 2.0.0
  */
 class CollectionAccumulator[T] extends AccumulatorV2[T, java.util.List[T]] {
-  private val _list: java.util.List[T] = Collections.synchronizedList(new ArrayList[T]())
+  private var _list: java.util.List[T] = _

Review comment:
       `lazy val` was the first thing I tried before embarking on this PR. It exacerbates the situation by adding another `final` field for checking set/not set as another source of NPE that I saw with my repro.
   
   We can use the code from the [filed modifiers java tutorial](https://docs.oracle.com/javase/tutorial/reflect/member/fieldModifiers.html) to demonstrate the issue:
   
   Before the PR one final field:
   ```bash
   $ ./bin/spark-submit ~/gits/FieldModifierSpy/fieldmodifier.jar org.apache.spark.util.CollectionAccumulator final
   Fields in Class 'org.apache.spark.util.CollectionAccumulator' containing modifiers:  final
   _list    [ synthetic=false enum_constant=false ]
   ```
   
   If we replace _list to be a `lazy val` we'll have 
   ```
   Fields in Class 'org.apache.spark.util.CollectionAccumulator' containing modifiers:  
   _list    [ synthetic=false enum_constant=false ]
   bitmap$0 [ synthetic=false enum_constant=false ]
   ```
   
   In the PR branch there are no final fields:
   ```
   $ ./bin/spark-submit ~/gits/FieldModifierSpy/fieldmodifier.jar org.apache.spark.util.CollectionAccumulator final
   Fields in Class 'org.apache.spark.util.CollectionAccumulator' containing modifiers:  final
   No matching fields
   ```
    




----------------------------------------------------------------
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.

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