On Thu, 27 Feb 2025 17:49:48 GMT, Mikhail Yankelevich <myankelev...@openjdk.org> wrote:
>> Refactor the following to run fully in java: >> test/java/security//Security/ClassLoaderDeadlock/ClassLoaderDeadlock.sh >> test/java/security//Security/ClassLoaderDeadlock/Deadlock.sh > > Mikhail Yankelevich has updated the pull request with a new target base due > to a merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - typo > - Merge branch 'master' into JDK-8349348 > - JDK-8349348: Refactor ClassLoaderDeadlock.sh and Deadlock.sh to run fully > in java test/jdk/java/security/Security/ClassLoaderDeadlock/ClassLoaderDeadlock.java line 28: > 26: * @bug 5094825 > 27: * @summary verify no deadlock if crypto provider in other classloader is > used to verify signed jars > 28: * @modules java.base/java.security Hello Mikhail, `java.security` package is an exported package from `java.base`. So you don't need this `@modules` declaration. test/jdk/java/security/Security/ClassLoaderDeadlock/ClassLoaderDeadlock.java line 30: > 28: * @modules java.base/java.security > 29: * @library ./Deadlock.jar > 30: * @compile -g provider/HashProvider.java Is the `-g` needed? test/jdk/java/security/Security/ClassLoaderDeadlock/ClassLoaderDeadlock.java line 31: > 29: * @library ./Deadlock.jar > 30: * @compile -g provider/HashProvider.java > 31: * @run main/othervm/timeout=30 -Djava.awt.headless=true > ClassLoaderDeadlock The `-Djava.awt.headless=true` is likely not needed but since it was used in the original test script, I think it's OK to carry it over. test/jdk/java/security/Security/ClassLoaderDeadlock/Deadlock.java line 29: > 27: * @summary make sure we do not deadlock loading signed JAR with > getInstance() > 28: * @library ./Deadlock.jar > 29: * @build Deadlock `Deadlock` here is the class name of the test itself. So you don't need to `@build` it explicitly. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23440#discussion_r1983175334 PR Review Comment: https://git.openjdk.org/jdk/pull/23440#discussion_r1983177523 PR Review Comment: https://git.openjdk.org/jdk/pull/23440#discussion_r1983177327 PR Review Comment: https://git.openjdk.org/jdk/pull/23440#discussion_r1983179883