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
@@ -48,12 +48,12 @@ public int foo() {

public class TestNestHostErrorWithMultiThread {

public static void main(String args[]) {
public static void main(String args[]) throws Throwable {

This comment has been minimized.

@dholmes-ora

dholmes-ora Apr 8, 2021
Member

"throws Throwable" is only necessary if you want unlisted checked exceptions to propagate.

This comment has been minimized.

@Wanghuang-Huawei

Wanghuang-Huawei Apr 23, 2021
Author Contributor

Thank you for your review. throws Throwable will be removed in my next commit.

TestNestHostErrorWithMultiThread t = new TestNestHostErrorWithMultiThread();
t.test();
}

public void test() {
public void test() throws Throwable {

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

@@ -62,19 +62,19 @@ public void test() {
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();
}
} catch (IllegalAccessError expected) {
System.out.println("OK - got expected exception: " + expected);
} catch (InterruptedException e) {}
}).start();

new Thread(() -> {
try {
latch.await();
HostNoNestMember h = new HostNoNestMember();
h.foo();
} catch (Exception e) {
e.printStackTrace();
}
} catch (IllegalAccessError expected) {
System.out.println("OK - got expected exception: " + expected);
} catch (InterruptedException e) {}

This comment has been minimized.

@dholmes-ora

dholmes-ora Apr 28, 2021
Member

Catch block should have:

throw new Error("Unexpected interrupt");

}).start();

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