Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8264760: JVM crashes when two threads encounter the same resolution error #3392

Closed
wants to merge 7 commits into from
@@ -0,0 +1,114 @@
class HostNoNestMember {

This comment has been minimized.

@dholmes-ora

dholmes-ora Apr 8, 2021
Member

This new file needs a copyright notice and GPL header.

Also please add a comment stating what this jcod file describes - see other jcod files in the directory for example.

This comment has been minimized.

@Wanghuang-Huawei

Wanghuang-Huawei Apr 23, 2021
Author Contributor

OK. I will fix that.

0xCAFEBABE;
0; // minor version
60; // version
[] { // Constant Pool
; // first element is empty
Method #2 #3; // #1
class #4; // #2
NameAndType #5 #6; // #3
Utf8 "java/lang/Object"; // #4
Utf8 "<init>"; // #5
Utf8 "()V"; // #6
class #8; // #7
Utf8 "HostNoNestMember$Member"; // #8
Method #7 #10; // #9
NameAndType #5 #11; // #10
Utf8 "(LHostNoNestMember;)V"; // #11
Field #7 #13; // #12
NameAndType #14 #15; // #13
Utf8 "value"; // #14
Utf8 "I"; // #15
class #17; // #16
Utf8 "HostNoNestMember"; // #17
Utf8 "Code"; // #18
Utf8 "LineNumberTable"; // #19
Utf8 "foo"; // #20
Utf8 "()I"; // #21
Utf8 "SourceFile"; // #22
Utf8 "TestNestHostErrorWithMultiThread.java"; // #23
Utf8 "NestMembers"; // #24
Utf8 "InnerClasses"; // #25
Utf8 "Member"; // #26
} // Constant Pool

0x0020; // access
#16;// this_cpx
#2;// super_cpx

[] { // Interfaces
} // Interfaces

[] { // Fields
} // Fields

[] { // Methods
{ // method
0x0000; // access
#5; // name_index
#6; // descriptor_index
[] { // Attributes
Attr(#18) { // Code
1; // max_stack
1; // max_locals
Bytes[]{
0x2AB70001B1;
}
[] { // Traps
} // end Traps
[] { // Attributes
Attr(#19) { // LineNumberTable
[] { // line_number_table
0 3;
}
} // end LineNumberTable
} // Attributes
} // end Code
} // Attributes
}
;
{ // method
0x0001; // access
#20; // name_index
#21; // descriptor_index
[] { // Attributes
Attr(#18) { // Code
3; // max_stack
2; // max_locals
Bytes[]{
0xBB0007592AB70009;
0x4C2BB4000CAC;
}
[] { // Traps
} // end Traps
[] { // Attributes
Attr(#19) { // LineNumberTable
[] { // line_number_table
0 9;
9 10;
}
} // end LineNumberTable
} // Attributes
} // end Code
} // Attributes
}
} // Methods

[] { // Attributes
Attr(#22) { // SourceFile
#23;
} // end SourceFile
;
Attr(#24) { // NestMembers
[] { // classes
// #7; delete NestMember
}
} // end NestMembers
;
Attr(#25) { // InnerClasses
[] { // classes
#7 #16 #26 0;
}
} // end InnerClasses
} // Attributes
} // end class HostNoNestMember
@@ -0,0 +1,82 @@
/*
* Copyright (c) 2021, Huawei Technologies Co., Ltd. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/*
* @test
* @bug 8264760
* @summary JVM crashes when two threads encounter the same resolution error
*
* @library /test/lib

This comment has been minimized.

@dholmes-ora

dholmes-ora Apr 28, 2021
Member

You don't use the test library

This comment has been minimized.

@Wanghuang-Huawei

Wanghuang-Huawei Apr 28, 2021
Author Contributor

Yes. Thank you for your review.

* @compile TestNestHostErrorWithMultiThread.java
* @compile HostNoNestMember.jcod
*
* @run main/othervm TestNestHostErrorWithMultiThread

This comment has been minimized.

@dholmes-ora

dholmes-ora Apr 28, 2021
Member

No need for othervm in this test.

This comment has been minimized.

@Wanghuang-Huawei

Wanghuang-Huawei Apr 28, 2021
Author Contributor

Sure. I will do that.

*/

This comment has been minimized.

@hseigel

hseigel Apr 30, 2021
Member

Please add a comment here that HostNoNestMember.jcod must be compiled after HostNoNestMember.java because the class file from the jcod file must replace the HostNoNestMember class file generated from HostNoNestMember.java.

This comment has been minimized.

@Wanghuang-Huawei

Wanghuang-Huawei May 6, 2021
Author Contributor

Thank you for your review. I will add that.

import java.util.concurrent.CountDownLatch;

class HostNoNestMember {

This comment has been minimized.

@dholmes-ora

dholmes-ora Apr 8, 2021
Member

Please add a comment describing how this class is replaced with the jcode version, and the difference in that version.

This comment has been minimized.

@Wanghuang-Huawei

Wanghuang-Huawei Apr 23, 2021
Author Contributor

I split this class into a single file HostNoNestMember.java as @hseigel suggested and add a comment.

class Member {
private int value;
}

public int foo() {

This comment has been minimized.

@dholmes-ora

dholmes-ora Apr 8, 2021
Member

Please call this "test".

This comment has been minimized.

@Wanghuang-Huawei

Wanghuang-Huawei Apr 23, 2021
Author Contributor

Sure. Thank you.

Member m = new Member();
return m.value;
}
}

public class TestNestHostErrorWithMultiThread {

public static void main(String args[]) {
TestNestHostErrorWithMultiThread t = new TestNestHostErrorWithMultiThread();
t.test();
Comment on lines +52 to +53

This comment has been minimized.

@dholmes-ora

dholmes-ora Apr 8, 2021
Member

There is no need to create an instance - just use a static test method.

The test code could be placed directly in main in this case.

This comment has been minimized.

@Wanghuang-Huawei

Wanghuang-Huawei Apr 23, 2021
Author Contributor

Thank you. It will be deleted in next commit.

}

public void test() {

CountDownLatch latch = new CountDownLatch(1);

This comment has been minimized.

@dholmes-ora

dholmes-ora Apr 8, 2021
Member

The use of CDL doesn't guarantee that both threads have reached the await() before the main thread does the countDown(). If you use a CyclicBarrier instead it will trip when the second thread arrives.

This comment has been minimized.

@Wanghuang-Huawei

Wanghuang-Huawei Apr 14, 2021
Author Contributor

I have tested CyclicBarrier here and found that CountDownLatch could trigger this bug fast while CyclicBarrier couldn't always trigger this bug.

This comment has been minimized.

This comment has been minimized.

@Wanghuang-Huawei

Wanghuang-Huawei Apr 19, 2021
Author Contributor

The use of CDL doesn't guarantee that both threads have reached the await() before the main thread does the countDown(). If you use a CyclicBarrier instead it will trip when the second thread arrives.

I have tested a small case the CyclicBarrier version and CountDownLatch version on JDK17 & JDK11. It seems that :

  • For JDK17, the wake-up time between two threads in the CyclicBarrier version is bigger than the CountDownLatch version. In other words, the CountDownLatch version can trigger this bug more easily .
  • the CyclicBarrier version and the CountDownLatch version running under JDK11 have the same performance.
// CountDownLatch version 
public class TestCountDownLatch {

  public static void main(String args[]) {
    CountDownLatch latch1 = new CountDownLatch(1);
    CountDownLatch latch2 = new CountDownLatch(2);

    MyThread t1 = new MyThread(latch1, latch2);
    MyThread t2 = new MyThread(latch1, latch2);

    t1.start();
    t2.start();

    try {
      // waiting thread creation
      latch2.await();
      latch1.countDown();

      t1.join();
      t2.join();
    } catch (InterruptedException e) {}   

    System.out.println(Math.abs(t1.getAwakeTime() - t2.getAwakeTime()));
  }

  static class MyThread extends Thread {
    private CountDownLatch latch1;
    private CountDownLatch latch2;
		private long awaketime;

    MyThread(CountDownLatch latch1, CountDownLatch latch2) {
      this.latch1 = latch1;
      this.latch2 = latch2;
    }

    @Override
    public void run() {
      try {
        latch2.countDown();
        // Try to have all threads trigger the nesthost check at the same time
        latch1.await();
        awaketime = System.nanoTime();
      } catch (InterruptedException e) {}
    }

    public long getAwakeTime() {
      return awaketime; 
    }
  }
}
// CyclicBarrier version 
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.CyclicBarrier;

public class TestCyclicBarrier {

  public static void main(String args[]) {
    CyclicBarrier barrier = new CyclicBarrier(2);

    MyThread t1 = new MyThread(barrier);
    MyThread t2 = new MyThread(barrier);

    t1.start();
    t2.start();

    try {
      t1.join();
      t2.join();
    } catch (InterruptedException e) {}   

    System.out.println(Math.abs(t1.getAwakeTime() - t2.getAwakeTime()));
  }

  static class MyThread extends Thread {
    private CyclicBarrier barrier; 
		private long awaketime;

    MyThread(CyclicBarrier barrier) {
      this.barrier = barrier;
    }

    @Override
    public void run() {
      try {
        // Try to have all threads trigger the nesthost check at the same time
        barrier.await();
        awaketime = System.nanoTime();
      } catch (Exception e) {}
    }

    public long getAwakeTime() {
      return awaketime; 
    }
  }
}
  • results:
  • CountDownLatch against CyclicBarrier
 CountDownLatch: 3960
 CyclicBarrier: 3361280
 CountDownLatch: 6120
 CyclicBarrier: 3337540
 CountDownLatch: 6160
 CyclicBarrier: 3462920
 CountDownLatch: 9150
 CyclicBarrier: 3328090
 CountDownLatch: 6580
 CyclicBarrier: 3345450
 CountDownLatch: 6340
 CyclicBarrier: 3342900
 CountDownLatch: 1330
 CyclicBarrier: 3379210
 CountDownLatch: 7780
 CyclicBarrier: 3219020
 CountDownLatch: 2460
 CyclicBarrier: 3297020
 CountDownLatch: 7320
 CyclicBarrier: 3332770
  • JDK17 against CyclicBarrier
CyclicBarrier jdk17: 3188590
CyclicBarrier jdk11: 49090
CyclicBarrier jdk17: 3123340
CyclicBarrier jdk11: 14680
CyclicBarrier jdk17: 3107910
CyclicBarrier jdk11: 780
CyclicBarrier jdk17: 3072600
CyclicBarrier jdk11: 1720
CyclicBarrier jdk17: 3164340
CyclicBarrier jdk11: 41020
CyclicBarrier jdk17: 3098490
CyclicBarrier jdk11: 7060
CyclicBarrier jdk17: 3058220
CyclicBarrier jdk11: 14750
CyclicBarrier jdk17: 3052460
CyclicBarrier jdk11: 660
CyclicBarrier jdk17: 3083650
CyclicBarrier jdk11: 14670
CyclicBarrier jdk17: 3116260
CyclicBarrier jdk11: 850

new Thread(() -> {
try {
latch.await();

This comment has been minimized.

@dholmes-ora

dholmes-ora Apr 8, 2021
Member

Please add a comment:

// Try to have all threads trigger the nesthost check at the same time

This comment has been minimized.

@Wanghuang-Huawei

Wanghuang-Huawei Apr 23, 2021
Author Contributor

Thank you. I will add that in next commit.

HostNoNestMember h = new HostNoNestMember();
h.foo();

This comment has been minimized.

@dholmes-ora

dholmes-ora Apr 8, 2021
Member

Please follow this by:
throw new RuntimeException("Did not get expected IllegalAccessError");

} catch (Exception e) {
e.printStackTrace();
}
}).start();

new Thread(() -> {
try {
latch.await();
HostNoNestMember h = new HostNoNestMember();
h.foo();
} catch (Exception e) {
e.printStackTrace();
}
}).start();

latch.countDown();
}
}
ProTip! Use n and p to navigate between commits in a pull request.