Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8159244

Partially initialized string object created by C2's string concat optimization may escape

    Details

    • Subcomponent:
    • Resolved In Build:
      b127

      Backports

        Description

        C2's String concatenation optimization replaces a series of StringBuilder.append() calls by creating a single char buffer array (or byte array with Compact Strings) and emitting direct loads/stores to/from this array. The final StringBuilder.toString() call is replaced by a new String allocation which is initialized to the buffer array (see [1] -> [2], CallStaticJava is replaced).
        Depending on the scheduling of instructions, it may happen that a reference to the newly allocated String object escapes before the String.value field is initialized (see [2], '334 StoreP' stores the String object, '514 StoreP' initializes the String.value field). In a highly concurrent setting, another thread may try to dereference String.value from such a partially initialized String object and crash.

        TestStringObjectInitialization.java reproduces this problem with JDK 7, 8 and 9 (see attached hs_err files) in approximately 1 out of 10 runs. I had to disable Indify String Concat, Compressed Oops and G1 to trigger the bug with JDK 9.

        [1] https://bugs.openjdk.java.net/secure/attachment/60305/graph_baseline_before%20SC.png
        [2] https://bugs.openjdk.java.net/secure/attachment/60306/graph_baseline_after_sc.png
        1. baseline.asm
          0.6 kB
          Tobias Hartmann
        2. fix.asm
          0.5 kB
          Tobias Hartmann
        3. JDK7_hs_err_pid17491.log
          139 kB
          Tobias Hartmann
        4. JDK8u_hs_err_pid23015.log
          67 kB
          Tobias Hartmann
        5. JDK9_hs_err_pid383.log
          64 kB
          Tobias Hartmann
        1. graph_baseline_after_sc.png
          28 kB
        2. graph_baseline_before SC.png
          21 kB
        3. graph_fix.png
          54 kB

          Issue Links

            Activity

            Hide
            thartmann Tobias Hartmann added a comment -
            ILW = Crash or wrong Java exceptions, easy to reproduce, -XX:-OptimizeStringConcat = HHM = P1
            Show
            thartmann Tobias Hartmann added a comment - ILW = Crash or wrong Java exceptions, easy to reproduce, -XX:-OptimizeStringConcat = HHM = P1
            Hide
            thartmann Tobias Hartmann added a comment -
            The solution is to add a StoreStore barrier after the String object initialization to prevent subsequent stores to float above (we do the same for the Object.clone intrinsic).

            Prototype fixes:
            http://cr.openjdk.java.net/~thartmann/8159244/webrev.9.00/
            http://cr.openjdk.java.net/~thartmann/8159244/webrev.8u.00/
            Show
            thartmann Tobias Hartmann added a comment - The solution is to add a StoreStore barrier after the String object initialization to prevent subsequent stores to float above (we do the same for the Object.clone intrinsic). Prototype fixes: http://cr.openjdk.java.net/~thartmann/8159244/webrev.9.00/ http://cr.openjdk.java.net/~thartmann/8159244/webrev.8u.00/
            Show
            thartmann Tobias Hartmann added a comment - A StoreStoreBarrier is not sufficient (see [1]). We need to emit a MemBarRelease: http://cr.openjdk.java.net/~thartmann/8159244/webrev.9.01 http://cr.openjdk.java.net/~thartmann/8159244/webrev.8u.01 [1] http://www.hboehm.info/c++mm/no_write_fences.html
            Hide
            hgupdate HG Updates added a comment -
            URL: http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/eadc4ebb7755
            User: thartmann
            Date: 2016-06-15 07:31:25 +0000
            Show
            hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/eadc4ebb7755 User: thartmann Date: 2016-06-15 07:31:25 +0000
            Hide
            ostuart Owen Stuart added a comment -
            Changed label to 8BPR as this is what is needed.
            Show
            ostuart Owen Stuart added a comment - Changed label to 8BPR as this is what is needed.
            Hide
            thartmann Tobias Hartmann added a comment -
            The question came up why this bug did not show up before. Putting the answer here for reference:
            1) It's a concurrency issue that only shows up with a very high load
            2) It requires concurrent String concatenations without any synchronization (not a common code pattern)
            3) It depends on instruction scheduling of the C2 compiler which is kind of nondeterministic
            4) In JDK 9 we have new String features like Compact Strings and Indify String Concat which (depending on the settings) may hide this bug
            Show
            thartmann Tobias Hartmann added a comment - The question came up why this bug did not show up before. Putting the answer here for reference: 1) It's a concurrency issue that only shows up with a very high load 2) It requires concurrent String concatenations without any synchronization (not a common code pattern) 3) It depends on instruction scheduling of the C2 compiler which is kind of nondeterministic 4) In JDK 9 we have new String features like Compact Strings and Indify String Concat which (depending on the settings) may hide this bug
            Hide
            hgupdate HG Updates added a comment -
            URL: http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/eadc4ebb7755
            User: amurillo
            Date: 2016-07-08 20:16:41 +0000
            Show
            hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/eadc4ebb7755 User: amurillo Date: 2016-07-08 20:16:41 +0000

              People

              • Assignee:
                thartmann Tobias Hartmann
                Reporter:
                thartmann Tobias Hartmann
              • Votes:
                0 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: