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
Closed
@@ -1935,8 +1935,11 @@ void SystemDictionary::add_nest_host_error(const constantPoolHandle& pool,
{
MutexLocker ml(Thread::current(), SystemDictionary_lock);
ResolutionErrorEntry* entry = resolution_errors()->find_entry(index, hash, pool, which);
if (entry != NULL) {
assert(entry->nest_host_error() == NULL, "Nest host error message already set!");
if (entry != NULL && entry->nest_host_error() == NULL) {
// An existing entry means we had a true resolution failure (LinkageError) with our nest host, but we
// still want to add the error message for the higher-level access checks to report. We should
// only reach here under the same error condition, so we can ignore the potential race with setting
// the message. If we see it is already set then we can ignore it.
entry->set_nest_host_error(message);

This comment has been minimized.

@dholmes-ora

dholmes-ora Apr 12, 2021
Member

Please add a comment before line 1939:
// An existing entry means we had a true resolution failure (LinkageError) with our nest host, but we
// still want to add the error message for the higher-level access checks to report. We should
// only reach here under the same error condition, so we can ignore the potential race with setting
// the message. If we see it is already set then we can ignore it.

This comment has been minimized.

@Wanghuang-Huawei

Wanghuang-Huawei Apr 23, 2021
Author Contributor

OK. Thank you for your advice. I will add that.

} else {
resolution_errors()->add_entry(index, hash, pool, which, message);
@@ -0,0 +1,43 @@
/*
* 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.
*/

/*
* This class was used to produce a jcod file in which the
* NestMembers attribute was modified to make it empty. Class
* HostNoNestMember$Member has class HostNoNestMember as its
* NestHost, which will trigger an error when resolving.
*
* When compiled, this generates a HostNoNestMember class and
* a HostNoNestMember$Menber class. The former class, HostNoNestMember,
* gets overwritten when HostNoNestMember.jcod gets compiled.
*/

This comment has been minimized.

@hseigel

hseigel Apr 30, 2021
Member

Can you modify the above comment to something such as:

 * This class was used to produce a jcod file in which the
 * NestMembers attribute was modified to make it empty. Class
 * HostNoNestMember$Member has class HostNoNestMember as its
 * NestHost, which will trigger an error when resolving.
 *
 * When compiled, this generates a HostNoNestMember class and
 * a HostNoNestMember$Menber class.  The former class, HostNoNestMember,
 * gets overwritten when HostNoNestMember.jcod gets compiled.

This comment has been minimized.

@Wanghuang-Huawei

Wanghuang-Huawei May 6, 2021
Author Contributor

OK. I will fix that. Thank you.

class HostNoNestMember {
class Member {
private int value;
}

public int test() {
Member m = new Member();
return m.value;
}
}
@@ -0,0 +1,139 @@
/*
* 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.
*/

// NestMembers attribute empty

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 "test"; // #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 44;
}
} // 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 50;
9 51;
}
} // 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,98 @@
/*
* 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
*
* @compile HostNoNestMember.java
* @compile HostNoNestMember.jcod
*
* @run main TestNestHostErrorWithMultiThread
*/

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.

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

import java.util.concurrent.CountDownLatch;

public class TestNestHostErrorWithMultiThread {

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

Runnable test = new Test(runLatch, startLatch);

Thread t1 = new Thread(test);
Thread t2 = new Thread(test);

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

try {
// waiting thread creation
startLatch.await();
runLatch.countDown();

t1.join();
t2.join();
} catch (InterruptedException e) {
throw new Error("Unexpected interrupt");
}
}

static class Test implements Runnable {
private CountDownLatch runLatch;
private CountDownLatch startLatch;

Test(CountDownLatch runLatch, CountDownLatch startLatch) {
this.runLatch = runLatch;
this.startLatch = startLatch;
}

@Override
public void run() {
try {
startLatch.countDown();
// Try to have all threads trigger the nesthost check at the same time
runLatch.await();
HostNoNestMember h = new HostNoNestMember();
h.test();

This comment has been minimized.

@dholmes-ora

dholmes-ora Apr 28, 2021
Member

After the call to test you should have:

throw new Error("IllegalAccessError was not thrown as expected");

This comment has been minimized.

@Wanghuang-Huawei

Wanghuang-Huawei Apr 28, 2021
Author Contributor

Thank you. I will change that.

throw new Error("IllegalAccessError was not thrown as expected");
} catch (IllegalAccessError expected) {
String msg = "current type is not listed as a nest member";
if (!expected.getMessage().contains(msg)) {
throw new Error("Wrong " + expected.getClass().getSimpleName() +": \"" +
expected.getMessage() + "\" does not contain \"" +
msg + "\"", expected);
}
System.out.println("OK - got expected exception: " + expected);
} catch (InterruptedException e) {
throw new Error("Unexpected interrupt");
}
}
}
}
ProTip! Use n and p to navigate between commits in a pull request.