8291508: Fix some tests with "requires vm.jvmti & vm.continuations" #9677

theaoqi · 2022-07-28T16:11:12Z

vmTestbase/nsk/jvmti/GetThreadInfo/thrinfo001/TestDescription.java and vmTestbase/nsk/jvmti/RedefineClasses/StressRedefineVirtual/StressRedefineVirtual.java are added or modified by JDK-8284161, and they are failed if Loom or JVMTI is not supported.

The issue could be reproduced by zero.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8291508: Fix some tests with "requires vm.jvmti & vm.continuations"

Reviewers

Serguei Spitsyn (@sspitsyn - Reviewer)
Leonid Mesnik (@lmesnik - Reviewer) ⚠️ Review applies to 1e00cd83
Alan Bateman (@AlanBateman - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/9677/head:pull/9677
$ git checkout pull/9677

Update a local copy of the PR:
$ git checkout pull/9677
$ git pull https://git.openjdk.org/jdk pull/9677/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 9677

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/9677.diff

bridgekeeper · 2022-07-28T16:11:55Z

👋 Welcome back aoqi! 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-07-28T16:15:15Z

@theaoqi The following labels will be automatically applied to this pull request:

client
serviceability

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

mlbridge · 2022-07-28T16:18:43Z

Webrevs

theaoqi · 2022-07-28T16:24:56Z

/label core-libs

openjdk · 2022-07-28T16:25:28Z

@theaoqi
The core-libs label was successfully added.

theaoqi · 2022-07-28T16:25:29Z

/label remove client,serviceability

openjdk · 2022-07-28T16:28:17Z

@theaoqi
The client label was successfully removed.

The serviceability label was successfully removed.

AlanBateman · 2022-08-01T07:15:59Z

/label add client
/label add serviceability

openjdk · 2022-08-01T07:16:36Z

@AlanBateman
The client label was successfully added.

openjdk · 2022-08-01T07:18:31Z

@AlanBateman
The serviceability label was successfully added.

AlanBateman · 2022-08-01T07:18:52Z

For the XMLDecoder tests then I think the issue is that it's using the 1-arg Class.forName rather than the 3-arg Class.forName. Can you try this:

diff --git a/test/jdk/java/beans/XMLDecoder/8028054/Task.java b/test/jdk/java/beans/XMLDecoder/8028054/Task.java
index 1d55fa8e358..add48336d16 100644
--- a/test/jdk/java/beans/XMLDecoder/8028054/Task.java
+++ b/test/jdk/java/beans/XMLDecoder/8028054/Task.java
@@ -130,8 +130,9 @@ abstract class Task<T> implements Runnable {
                 .map(s -> s.substring(s.indexOf("java")))
                 .collect(Collectors.toList());
 
+        ClassLoader scl = ClassLoader.getSystemClassLoader();
         for (String name : fileNames) {
-            classes.add(Class.forName(name));
+            classes.add(Class.forName(name, false, scl));
             if (count == classes.size()) {
                 break;
             }

Co-authored-by: Alan Bateman <alanb@openjdk.org>

theaoqi · 2022-08-01T12:02:19Z

Hi @AlanBateman , your method works. It is a much better way. Thank you very much!

Since these two kinds (jvmti and XMLDecoder tests) of the fix are a little different, should I split this issue into two?

theaoqi · 2022-08-01T12:02:38Z

/contributor add alanb

openjdk · 2022-08-01T12:02:46Z

@theaoqi
Contributor Alan Bateman <alanb@openjdk.org> successfully added.

AlanBateman · 2022-08-01T15:54:47Z

Since these two kinds (jvmti and XMLDecoder tests) of the fix are a little different, should I split this issue into two?

I think it would be better to split them.

theaoqi · 2022-08-01T16:29:01Z

/contributor remove alanb

openjdk · 2022-08-01T16:29:15Z

@theaoqi
Contributor Alan Bateman <alanb@openjdk.org> successfully removed.

theaoqi · 2022-08-01T16:30:33Z

Since these two kinds (jvmti and XMLDecoder tests) of the fix are a little different, should I split this issue into two?

I think it would be better to split them.

XMLDecoder tests are filed: https://bugs.openjdk.org/browse/JDK-8291640. This issue was also updated.

sspitsyn

I'm surprised these two tests have the --enable-preview flag.
@lmesnik Could you, please, take a look at these two tests?
Otherwise, the fix looks good to me.
Thanks,
Serguei

openjdk · 2022-08-02T07:01:26Z

@theaoqi 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:

8291508: Fix some tests with "requires vm.jvmti & vm.continuations"

Reviewed-by: sspitsyn, lmesnik, alanb

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 14 new commits pushed to the master branch:

c89556f: 8291633: Build failures with GCC 11, Alpine 3 due to incompatible casts from nullptr
4d6e6f6: 8291822: ARM32: Build errors with GCC 11 in frame::saved_oop_result
e52a340: 8290488: IBM864 character encoding implementation bug
af529be: 8285836: sun/net/www/http/KeepAliveCache/KeepAliveProperty.java failed with "RuntimeException: Failed in server"
b7d2bde: 8291733: Remove JFR events that expose hashtable
0971d34: Merge
54c093a: 8291524: jdk/jfr/event/runtime/TestClassLoaderStatsEvent.java Value not equal to 2, field='hiddenClassCount', value='0'
1d16c91: 8291512: Snippetize modules API examples
0ae8341: 8290908: misc tests fail: assert(!thread->owns_locks()) failed: must release all locks when leaving VM
5acf2d7: 8291578: Remove JMX related tests from ProblemList-svc-vthreads.txt
... and 4 more: https://git.openjdk.org/jdk/compare/554f44ecb1134acff3eaf02e2e1c0e01158ab7e5...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@sspitsyn, @lmesnik, @AlanBateman) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

AlanBateman · 2022-08-02T17:25:19Z

Just an FYI that the ORDER OF TAGS section of the tag language spec recommends putting the @requires before the @modules and @library tags.

theaoqi · 2022-08-03T02:33:27Z

Just an FYI that the ORDER OF TAGS section of the tag language spec recommends putting the @requires before the @modules and @library tags.

Done. Didn't know that. Thank you for the reminder.

theaoqi · 2022-08-03T11:33:55Z

/integrate

openjdk · 2022-08-03T11:35:54Z

@theaoqi
Your change (at version 2ea9e1a) is now ready to be sponsored by a Committer.

sspitsyn · 2022-08-03T17:29:47Z

/sponsor

openjdk · 2022-08-03T17:33:27Z

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

c89556f: 8291633: Build failures with GCC 11, Alpine 3 due to incompatible casts from nullptr
4d6e6f6: 8291822: ARM32: Build errors with GCC 11 in frame::saved_oop_result
e52a340: 8290488: IBM864 character encoding implementation bug
af529be: 8285836: sun/net/www/http/KeepAliveCache/KeepAliveProperty.java failed with "RuntimeException: Failed in server"
b7d2bde: 8291733: Remove JFR events that expose hashtable
0971d34: Merge
54c093a: 8291524: jdk/jfr/event/runtime/TestClassLoaderStatsEvent.java Value not equal to 2, field='hiddenClassCount', value='0'
1d16c91: 8291512: Snippetize modules API examples
0ae8341: 8290908: misc tests fail: assert(!thread->owns_locks()) failed: must release all locks when leaving VM
5acf2d7: 8291578: Remove JMX related tests from ProblemList-svc-vthreads.txt
... and 4 more: https://git.openjdk.org/jdk/compare/554f44ecb1134acff3eaf02e2e1c0e01158ab7e5...master

Your commit was automatically rebased without conflicts.

openjdk · 2022-08-03T17:33:43Z

@sspitsyn @theaoqi Pushed as commit 0cc49fd.

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

8291508: Fix some tests with "requires vm.continuations"

c16a0b8

openjdk bot added the rfr Pull request is ready for review label Jul 28, 2022

openjdk bot added serviceability serviceability-dev@openjdk.org client client-libs-dev@openjdk.org labels Jul 28, 2022

openjdk bot added the core-libs core-libs-dev@openjdk.org label Jul 28, 2022

openjdk bot removed client client-libs-dev@openjdk.org serviceability serviceability-dev@openjdk.org labels Jul 28, 2022

openjdk bot added the client client-libs-dev@openjdk.org label Aug 1, 2022

openjdk bot added the serviceability serviceability-dev@openjdk.org label Aug 1, 2022

XMLDecoder update

623e372

Co-authored-by: Alan Bateman <alanb@openjdk.org>

XMLDecoder tests are filed in JDK-8291640

97c38a3

theaoqi mentioned this pull request Aug 1, 2022

8291640: java/beans/XMLDecoder/8028054/Task.java should use the 3-arg Class.forName #9704

Closed

3 tasks

theaoqi changed the title ~~8291508: Fix some tests with "requires vm.continuations"~~ 8291508: Fix some tests with "requires vm.jvmti & vm.continuations" Aug 1, 2022

Merge branch 'master' into 8291508

1e00cd8

sspitsyn approved these changes Aug 2, 2022

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Aug 2, 2022

lmesnik approved these changes Aug 2, 2022

View reviewed changes

update tags order

2ea9e1a

AlanBateman approved these changes Aug 3, 2022

View reviewed changes

openjdk bot added the sponsor Pull request is ready to be sponsored label Aug 3, 2022

sspitsyn approved these changes Aug 3, 2022

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Aug 3, 2022

openjdk bot closed this Aug 3, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Aug 3, 2022

theaoqi deleted the 8291508 branch August 4, 2022 03:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8291508: Fix some tests with "requires vm.jvmti & vm.continuations" #9677

8291508: Fix some tests with "requires vm.jvmti & vm.continuations" #9677

theaoqi commented Jul 28, 2022 •

edited by openjdk bot

Loading

bridgekeeper bot commented Jul 28, 2022

openjdk bot commented Jul 28, 2022

mlbridge bot commented Jul 28, 2022 •

edited

Loading

theaoqi commented Jul 28, 2022

openjdk bot commented Jul 28, 2022

theaoqi commented Jul 28, 2022

openjdk bot commented Jul 28, 2022

AlanBateman commented Aug 1, 2022

openjdk bot commented Aug 1, 2022

openjdk bot commented Aug 1, 2022

AlanBateman commented Aug 1, 2022

theaoqi commented Aug 1, 2022

theaoqi commented Aug 1, 2022

openjdk bot commented Aug 1, 2022

AlanBateman commented Aug 1, 2022

theaoqi commented Aug 1, 2022

openjdk bot commented Aug 1, 2022

theaoqi commented Aug 1, 2022 •

edited

Loading

sspitsyn left a comment

openjdk bot commented Aug 2, 2022 •

edited

Loading

AlanBateman commented Aug 2, 2022

theaoqi commented Aug 3, 2022

theaoqi commented Aug 3, 2022

openjdk bot commented Aug 3, 2022

sspitsyn commented Aug 3, 2022

openjdk bot commented Aug 3, 2022

openjdk bot commented Aug 3, 2022

8291508: Fix some tests with "requires vm.jvmti & vm.continuations" #9677

8291508: Fix some tests with "requires vm.jvmti & vm.continuations" #9677

Conversation

theaoqi commented Jul 28, 2022 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Jul 28, 2022

openjdk bot commented Jul 28, 2022

mlbridge bot commented Jul 28, 2022 • edited Loading

Webrevs

theaoqi commented Jul 28, 2022

openjdk bot commented Jul 28, 2022

theaoqi commented Jul 28, 2022

openjdk bot commented Jul 28, 2022

AlanBateman commented Aug 1, 2022

openjdk bot commented Aug 1, 2022

openjdk bot commented Aug 1, 2022

AlanBateman commented Aug 1, 2022

theaoqi commented Aug 1, 2022

theaoqi commented Aug 1, 2022

openjdk bot commented Aug 1, 2022

AlanBateman commented Aug 1, 2022

theaoqi commented Aug 1, 2022

openjdk bot commented Aug 1, 2022

theaoqi commented Aug 1, 2022 • edited Loading

sspitsyn left a comment

Choose a reason for hiding this comment

openjdk bot commented Aug 2, 2022 • edited Loading

AlanBateman commented Aug 2, 2022

theaoqi commented Aug 3, 2022

theaoqi commented Aug 3, 2022

openjdk bot commented Aug 3, 2022

sspitsyn commented Aug 3, 2022

openjdk bot commented Aug 3, 2022

openjdk bot commented Aug 3, 2022

theaoqi commented Jul 28, 2022 •

edited by openjdk bot

Loading

mlbridge bot commented Jul 28, 2022 •

edited

Loading

theaoqi commented Aug 1, 2022 •

edited

Loading

openjdk bot commented Aug 2, 2022 •

edited

Loading