8267796: vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002/TestDescription.java fails with NoClassDefFoundError #7607

alexmenkov · 2022-02-24T12:44:18Z

The fix adds workaround in hs201t001a class like we have in hs201t001 test to avoid class loading while the test do single stepping/pop frame.
Also fixed a number of issues in the test.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-8267796: vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002/TestDescription.java fails with NoClassDefFoundError

Reviewers

Kevin Walls (@kevinjwalls - Committer) ⚠️ Review applies to 404d421
Chris Plummer (@plummercj - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/7607/head:pull/7607
$ git checkout pull/7607

Update a local copy of the PR:
$ git checkout pull/7607
$ git pull https://git.openjdk.java.net/jdk pull/7607/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 7607

View PR using the GUI difftool:
$ git pr show -t 7607

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/7607.diff

bridgekeeper · 2022-02-24T12:44:29Z

👋 Welcome back amenkov! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2022-02-24T12:46:43Z

@alexmenkov The following label will be automatically applied to this pull request:

serviceability

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

alexmenkov · 2022-02-24T12:47:08Z

test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002/TestDescription.java

@@ -71,7 +71,7 @@
 *      newclass
 *
 * @run main/othervm/native
- *      -agentlib:hs201t002=pathToNewByteCode=./bin,-waittime=5
- *      nsk.jvmti.scenarios.hotswap.HS201.hs201t002
+ *      -agentlib:hs201t002=pathToNewByteCode=./bin,-waittime=5,-verbose


Adds verbose output for the test and agent

alexmenkov · 2022-02-24T12:48:06Z

test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002/hs201t002.cpp

@@ -45,16 +45,12 @@ static jlong timeout = 0;
 #define PATH_TO_NEW_BYTECODE "pathToNewByteCode"

 static jint testStep;
-static int redefineNumber;


alexmenkov · 2022-02-24T12:49:39Z

test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002/hs201t002.cpp

    }

    if (!NSK_JVMTI_VERIFY(jvmti_env->Deallocate((unsigned char*)className))) {
        nsk_jvmti_setFailStatus();
    }
+    if (!NSK_JVMTI_VERIFY(jvmti_env->Deallocate(newClassBytes))) {


proper resource deallocation

alexmenkov · 2022-02-24T12:53:02Z

test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002/hs201t002.cpp

        jclass klass;

-        NSK_DISPLAY2("\n\n>>>> Exception %s in thread - %s\n",


getClassName() and getThreadName() used the same global buffer to return results. So thread name overrides class name and in log we see:

Exception hs201t002Thread in thread - hs201t002Thread
instead of
Exception Lnsk/jvmti/scenarios/hotswap/HS201/hs201t002a; in thread - hs201t002Thread

alexmenkov · 2022-02-24T12:54:27Z

test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002/hs201t002.cpp

@@ -377,7 +379,7 @@ callbackExceptionCatch(jvmtiEnv *jvmti_env, JNIEnv* jni_env, jthread thread,

 /* ============================================================================= */

-int readNewBytecode(jvmtiEnv* jvmti) {
+int readNewBytecode(jvmtiEnv* jvmti, jint &newClassSize, unsigned char* &newClassBytes) {


The function returns loaded data instead of using global variable for correct resource management

alexmenkov · 2022-02-24T12:55:52Z

test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002/hs201t002.cpp

@@ -422,68 +426,61 @@ int readNewBytecode(jvmtiEnv* jvmti) {

 /* ============================================================================= */

-const char* getThreadName(JNIEnv* jni_env, jthread thread) {
+const char* getThreadName(jvmtiEnv* jvmti_env, JNIEnv* jni_env, jthread thread) {


The function now returns JVMTI allocated buffer instead of global buffer

alexmenkov · 2022-02-24T12:56:08Z

test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002/hs201t002.cpp


    jni_env->ReleaseStringUTFChars(jthreadName, threadName);

-    return chbuffer;
+    return (char*)result;
 }

 /* ============================================================================= */

 const char* getClassName(jvmtiEnv *jvmti_env, JNIEnv* jni_env, jobject object) {


The function now returns JVMTI allocated buffer instead of global buffer

alexmenkov · 2022-02-24T12:58:21Z

test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002/hs201t002.cpp

@@ -665,11 +666,6 @@ jint Agent_Initialize(JavaVM *jvm, char *options, void *reserved) {

    NSK_DISPLAY0("Enable events\n");

-    enableEvent(jvmti, JVMTI_EVENT_CLASS_LOAD, testedThread);


This code executed on agent initialization, the test has no chance to set testThread.
Moved to hs201t002.setThread()

alexmenkov · 2022-02-24T13:15:25Z

...otspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002/newclass/hs201t002a.java

@@ -26,6 +26,7 @@
 public class hs201t002a extends Exception {

    public hs201t002a () {
+        System.out.println("Current step: " + hs201t002.currentStep); // Avoid calling classloader to find hs201t002 in doInit()


This is the fix for the failure, the rest are test improvements

mlbridge · 2022-02-24T13:28:24Z

Webrevs

01: Full - Incremental (304624b3)
00: Full (404d421a)

test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002.java

test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002/hs201t002.cpp

test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002/TestDescription.java

kevinjwalls

Looks like a good improvement.

openjdk · 2022-03-01T20:14:15Z

@alexmenkov This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8267796: vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002/TestDescription.java fails with NoClassDefFoundError

Reviewed-by: kevinw, cjplummer

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 267 new commits pushed to the master branch:

732d891: 8282411: Add useful predicates to ElementKind
eac8021: 8282320: Remove case conversion for debugging log in SSLCipher
b86a8c0: 8282150: Drop redundant
elements from tables in java.desktop HTML files
941e97c: 8281738: Create a regression test for checking the 'Space' key activation of focused Button
8fec7b8: 8281548: Add escape analysis tracing flag
b03d66c: 8282452: Use of Preview API in preview methods should not trigger preview warning for the enclosing class
76398c8: 8279573: compiler/codecache/CodeCacheFullCountTest.java fails with "RuntimeException: the value of full_count is wrong."
31b61f9: 8282311: Fix a typo in javax.lang.model.type.NullType
2c5d266: 8282045: When loop strip mining fails, safepoints are removed from loop anyway
341c8bd: 8267834: Refactor G1CardSetAllocator and BufferNode::Allocator to use a common base class
... and 257 more: https://git.openjdk.java.net/jdk/compare/46c6c6f308b5ec0ec3b762df4b76de555287474c...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

alexmenkov · 2022-03-02T10:52:56Z

/integrate

openjdk · 2022-03-02T10:54:34Z

Going to push as commit f12200c.
Since your change was applied there have been 270 commits pushed to the master branch:

12a822a: 8282381: Parallel: Remove unnecessary PCReferenceProcessor
ed3496e: 8282480: IGV: Use description instead of enum name for phases
fc52a21: 8282143: Objects.requireNonNull should be ForceInline
732d891: 8282411: Add useful predicates to ElementKind
eac8021: 8282320: Remove case conversion for debugging log in SSLCipher
b86a8c0: 8282150: Drop redundant
elements from tables in java.desktop HTML files
941e97c: 8281738: Create a regression test for checking the 'Space' key activation of focused Button
8fec7b8: 8281548: Add escape analysis tracing flag
b03d66c: 8282452: Use of Preview API in preview methods should not trigger preview warning for the enclosing class
76398c8: 8279573: compiler/codecache/CodeCacheFullCountTest.java fails with "RuntimeException: the value of full_count is wrong."
... and 260 more: https://git.openjdk.java.net/jdk/compare/46c6c6f308b5ec0ec3b762df4b76de555287474c...master

Your commit was automatically rebased without conflicts.

openjdk · 2022-03-02T10:54:47Z

@alexmenkov Pushed as commit f12200c.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

JDK-8267796

039c93d

openjdk bot added the serviceability serviceability-dev@openjdk.org label Feb 24, 2022

alexmenkov commented Feb 24, 2022

View reviewed changes

trailing whitespace

404d421

alexmenkov commented Feb 24, 2022

View reviewed changes

alexmenkov marked this pull request as ready for review February 24, 2022 13:22

openjdk bot added the rfr Pull request is ready for review label Feb 24, 2022

plummercj reviewed Feb 28, 2022

View reviewed changes

kevinjwalls reviewed Mar 1, 2022

View reviewed changes

test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002/TestDescription.java Show resolved Hide resolved

kevinjwalls approved these changes Mar 1, 2022

View reviewed changes

Chris's feedback

304624b

plummercj approved these changes Mar 1, 2022

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Mar 1, 2022

openjdk bot added the integrated Pull request has been integrated label Mar 2, 2022

openjdk bot closed this Mar 2, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Mar 2, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8267796: vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002/TestDescription.java fails with NoClassDefFoundError #7607

8267796: vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002/TestDescription.java fails with NoClassDefFoundError #7607

alexmenkov commented Feb 24, 2022 •

edited by openjdk bot

Loading

bridgekeeper bot commented Feb 24, 2022

openjdk bot commented Feb 24, 2022

alexmenkov Feb 24, 2022

alexmenkov Feb 24, 2022

alexmenkov Feb 24, 2022

alexmenkov Feb 24, 2022

alexmenkov Feb 24, 2022

alexmenkov Feb 24, 2022

alexmenkov Feb 24, 2022

alexmenkov Feb 24, 2022

alexmenkov Feb 24, 2022

mlbridge bot commented Feb 24, 2022 •

edited

Loading

kevinjwalls left a comment

openjdk bot commented Mar 1, 2022

alexmenkov commented Mar 2, 2022

openjdk bot commented Mar 2, 2022

openjdk bot commented Mar 2, 2022

		jclass klass;

		NSK_DISPLAY2("\n\n>>>> Exception %s in thread - %s\n",

		@@ -665,11 +666,6 @@ jint Agent_Initialize(JavaVM jvm, char options, void *reserved) {

		NSK_DISPLAY0("Enable events\n");

		enableEvent(jvmti, JVMTI_EVENT_CLASS_LOAD, testedThread);

8267796: vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002/TestDescription.java fails with NoClassDefFoundError #7607

8267796: vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002/TestDescription.java fails with NoClassDefFoundError #7607

Conversation

alexmenkov commented Feb 24, 2022 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Feb 24, 2022

openjdk bot commented Feb 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mlbridge bot commented Feb 24, 2022 • edited Loading

Webrevs

kevinjwalls left a comment

Choose a reason for hiding this comment

openjdk bot commented Mar 1, 2022

alexmenkov commented Mar 2, 2022

openjdk bot commented Mar 2, 2022

openjdk bot commented Mar 2, 2022

alexmenkov commented Feb 24, 2022 •

edited by openjdk bot

Loading

mlbridge bot commented Feb 24, 2022 •

edited

Loading