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
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/hotspot/share/classfile/systemDictionary.cpp
Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

} else {
resolution_errors()->add_entry(index, hash, pool, which, message);
Expand Down
@@ -0,0 +1,39 @@
/*
* 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 is used to produce a jcod file in which we modify the
* NestMembers attribute to make it empty. The class
* HostNoNestMember$Member has a class HostNoNestMember as its
* NestHost, which will trigger an error when resolving .
*/
Copy link
Member

@hseigel hseigel Apr 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I will fix that. Thank you.

class HostNoNestMember {
class Member {
private int value;
}

public int test() {
Member m = new Member();
return m.value;
}
}
139 changes: 139 additions & 0 deletions test/hotspot/jtreg/runtime/Nestmates/membership/HostNoNestMember.jcod
@@ -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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,88 @@
/*
* 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
*/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your review. I will add that.

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);

new Thread(test).start();
new Thread(test).start();

try {
// waiting thread creation
startLatch.await();
runLatch.countDown();
} 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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the call to test you should have:

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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");
}
}
}
}