You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by sr...@apache.org on 2021/10/08 12:12:43 UTC

[spark] branch branch-3.2 updated: [SPARK-36717][CORE] Incorrect order of variable initialization may lead incorrect behavior

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 8e60d6e  [SPARK-36717][CORE] Incorrect order of variable initialization may lead incorrect behavior
8e60d6e is described below

commit 8e60d6e7a9dcc9712deffd519709592e40771086
Author: daugraph <da...@qq.com>
AuthorDate: Fri Oct 8 07:11:26 2021 -0500

    [SPARK-36717][CORE] Incorrect order of variable initialization may lead incorrect behavior
    
    ### What changes were proposed in this pull request?
    Incorrect order of variable initialization may lead to incorrect behavior, related code: TorrentBroadcast.scala , TorrentBroadCast will get wrong checksumEnabled value after initialization, this may not be what we need, we can move L94 front of setConf(SparkEnv.get.conf) to avoid this.
    
    Supplement:
    Snippet 1
    ```scala
    class Broadcast {
      def setConf(): Unit = {
        checksumEnabled = true
      }
      setConf()
      var checksumEnabled = false
    }
    println(new Broadcast().checksumEnabled)
    ```
    output:
    ```scala
    false
    ```
     Snippet 2
    ```scala
    class Broadcast {
      var checksumEnabled = false
      def setConf(): Unit = {
        checksumEnabled = true
      }
      setConf()
    }
    println(new Broadcast().checksumEnabled)
    ```
    output:
    ```scala
    true
    ```
    
    ### Why are the changes needed?
    we can move L94 front of setConf(SparkEnv.get.conf) to avoid this.
    
    ### Does this PR introduce _any_ user-facing change?
    No
    
    ### How was this patch tested?
    No
    
    Closes #33957 from daugraph/branch0.
    
    Authored-by: daugraph <da...@qq.com>
    Signed-off-by: Sean Owen <sr...@gmail.com>
    (cherry picked from commit 65f6a7c1ecdcf7d6df798e30c9fc03a5dbe0b047)
    Signed-off-by: Sean Owen <sr...@gmail.com>
---
 .../main/scala/org/apache/spark/broadcast/TorrentBroadcast.scala    | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/core/src/main/scala/org/apache/spark/broadcast/TorrentBroadcast.scala b/core/src/main/scala/org/apache/spark/broadcast/TorrentBroadcast.scala
index 1024d9b..e35a079 100644
--- a/core/src/main/scala/org/apache/spark/broadcast/TorrentBroadcast.scala
+++ b/core/src/main/scala/org/apache/spark/broadcast/TorrentBroadcast.scala
@@ -73,6 +73,10 @@ private[spark] class TorrentBroadcast[T: ClassTag](obj: T, id: Long)
   /** Size of each block. Default value is 4MB.  This value is only read by the broadcaster. */
   @transient private var blockSize: Int = _
 
+
+  /** Whether to generate checksum for blocks or not. */
+  private var checksumEnabled: Boolean = false
+
   private def setConf(conf: SparkConf): Unit = {
     compressionCodec = if (conf.get(config.BROADCAST_COMPRESS)) {
       Some(CompressionCodec.createCodec(conf))
@@ -90,8 +94,6 @@ private[spark] class TorrentBroadcast[T: ClassTag](obj: T, id: Long)
   /** Total number of blocks this broadcast variable contains. */
   private val numBlocks: Int = writeBlocks(obj)
 
-  /** Whether to generate checksum for blocks or not. */
-  private var checksumEnabled: Boolean = false
   /** The checksum for all the blocks. */
   private var checksums: Array[Int] = _
 

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org