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

8248330: [lworld] [lw3] test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetImplementedInterfaces/getintrf007/TestDescription.java fails because of IdentityObject #100

Closed
wants to merge 2 commits into from

Conversation

@fparain
Copy link
Collaborator

@fparain fparain commented Jun 25, 2020

Please review this small fix in JVMTI.

The problem is that GetImplementedInterfaces doesn't return the expected set of interfaces because of the injection of the java.lang.IdentityObject interface by the JVM. The fix simply filters out this interface if it has been injected by the JVM, providing the same behavior as for reflection.

Thank you,

Fred


Progress

  • Change must not contain extraneous whitespace

Issue

  • JDK-8248330: [lworld] [lw3] test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetImplementedInterfaces/getintrf007/TestDescription.java fails because of IdentityObject

Reviewers

  • Harold Seigel (hseigel - Committer) ⚠️ Review applies to 9846fd0

Download

$ git fetch https://git.openjdk.java.net/valhalla pull/100/head:pull/100
$ git checkout pull/100

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Jun 25, 2020

👋 Welcome back fparain! A progress list of the required criteria for merging this PR into lworld will be added to the body of your pull request.

@openjdk
Copy link

@openjdk openjdk bot commented Jun 25, 2020

@fparain This change now passes all automated pre-integration checks, type /integrate in a new comment to proceed. After integration, the commit message will be:

8248330: [lworld] [lw3] test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetImplementedInterfaces/getintrf007/TestDescription.java fails because of IdentityObject

Reviewed-by: hseigel
  • If you would like to add a summary, use the /summary command.
  • To credit additional contributors, use the /contributor command.
  • To add additional solved issues, use the /issue command.

There are currently no new commits on the lworld branch since the last update of the source branch of this PR. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you would like to avoid potential automatic rebasing, specify the current head hash when integrating, like this: /integrate 9ba8bbe151f08445260c08362510304615b6d993.

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

@mlbridge
Copy link

@mlbridge mlbridge bot commented Jun 25, 2020

Webrevs

Copy link
Member

@hseigel hseigel left a comment

The changes look good. Does this also fix the test failures listed in JDK-8247491?

@fparain
Copy link
Collaborator Author

@fparain fparain commented Jun 25, 2020

Harold,

Thank you for the review.

I've tested the fix with the tests listed in JDK-8247491:
vmTestbase/nsk/jdi/ClassType/allInterfaces/allinterfaces001/TestDescription.java
vmTestbase/nsk/jdi/ClassType/interfaces/interfaces001/TestDescription.java
vmTestbase/nsk/jdi/InterfaceType/implementors/implementors001/TestDescription.java
vmTestbase/nsk/jdi/InterfaceType/subinterfaces/subinterfaces001/TestDescription.java
vmTestbase/nsk/jdi/InterfaceType/superinterfaces/superinterfaces001/TestDescription.java
vmTestbase/nsk/jvmti/ClassPrepare/classprep001/TestDescription.java

They all pass now.

Fred

@mlchung
Copy link
Member

@mlchung mlchung commented Jun 25, 2020

Hi Fred,

With GetImplementationInterfaces filtering java.lang.IdentityObject, the temporary fix [1] should be reverted. Otherwise looks good.

Mandy
[1] 806fff4

@hseigel
Copy link
Member

@hseigel hseigel commented Jun 25, 2020

Fred,
Thanks for checking the other tests.
Harold

@mlbridge
Copy link

@mlbridge mlbridge bot commented Jun 25, 2020

Mailing list message from Frederic Parain on valhalla-dev:

Thank you Mandy, I?ll revert the temporary fix.

Fred

@fparain
Copy link
Collaborator Author

@fparain fparain commented Jun 25, 2020

/integrate

@openjdk openjdk bot closed this Jun 25, 2020
@openjdk openjdk bot added the integrated label Jun 25, 2020
@openjdk
Copy link

@openjdk openjdk bot commented Jun 25, 2020

@fparain
Pushed as commit d39d420.

@fparain fparain deleted the jvmti_identityObject branch Aug 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants