Re: RFR: 8270380: Change the default value of the java.security.manager system property to disallow
On 23/08/2021 05:45, David Holmes wrote: : @wangweij there are many tests that can call setSecurityManager() and will presumably need to be fixed before this switch can be applied. And all testing will need to be updated to require jtreg 6.1 (which no longer uses the SM) once it is released. Most of the preparation work for the tests was done via JDK-8267184 in JDK 17. JDK-8270380 isn't going to be integrated until after the upgrade the jtreg 6.1 (which I think is one part of Jai's mail). -Alan.
Re: RFR: 8270380: Change the default value of the java.security.manager system property to disallow
On Fri, 20 Aug 2021 22:44:34 GMT, Weijun Wang wrote: > This change modifies the default value of the `java.security.manager` system > property from "allow" to "disallow". This means unless it's explicitly set to > "allow", any call to `System.setSecurityManager()` would throw an UOE. > > The `AllowSecurityManager.java` and `SecurityManagerWarnings.java` tests are > updated to confirm this behavior change. Two other tests are updated because > they were added after JDK-8267184 and do not have > `-Djava.security.manager=allow` on its `@run` line even it they need to > install a `SecurityManager` at runtime. @wangweij there are many tests that can call setSecurityManager() and will presumably need to be fixed before this switch can be applied. And all testing will need to be updated to require jtreg 6.1 (which no longer uses the SM) once it is released. Thanks, David - PR: https://git.openjdk.java.net/jdk/pull/5204
Re: RFR: 8270380: Change the default value of the java.security.manager system property to disallow
On Fri, 20 Aug 2021 22:44:34 GMT, Weijun Wang wrote: > This change modifies the default value of the `java.security.manager` system > property from "allow" to "disallow". This means unless it's explicitly set to > "allow", any call to `System.setSecurityManager()` would throw an UOE. > > The `AllowSecurityManager.java` and `SecurityManagerWarnings.java` tests are > updated to confirm this behavior change. Two other tests are updated because > they were added after JDK-8267184 and do not have > `-Djava.security.manager=allow` on its `@run` line even it they need to > install a `SecurityManager` at runtime. It looks to me that the failures reported in the GitHub jobs are genuine and related to this change. It looks like the `jtreg` framework itself is impacted by this change because it calls the `System.setSecurityManager()` while launching the tests. 2021-08-21T01:41:42.5731927Z stderr: 2021-08-21T01:41:42.5733295Z java.lang.UnsupportedOperationException: The Security Manager is deprecated and will be removed in a future release 2021-08-21T01:41:42.5734843Zat java.base/java.lang.System.setSecurityManager(System.java:409) 2021-08-21T01:41:42.5736946Zat com.sun.javatest.regtest.agent.RegressionSecurityManager.install(RegressionSecurityManager.java:56) 2021-08-21T01:41:42.5739224Zat com.sun.javatest.regtest.agent.AgentServer.(AgentServer.java:211) 2021-08-21T01:41:42.5740879Zat com.sun.javatest.regtest.agent.AgentServer.main(AgentServer.java:70) 2021-08-21T01:41:42.5741805Z 2021-08-21T01:41:42.5743348Z TEST RESULT: Error. Agent communication error: java.net.SocketException: Connection reset; check console log for any additional details 2021-08-21T01:41:42.5745030Z -- 2021-08-21T01:41:51.2539413Z Test results: passed: 5; error: 879 2021-08-21T01:41:59.0887042Z Report written to /home/runner/work/jdk/jdk/build/run-test-prebuilt/test-results/jtreg_test_jdk_tier1_part1/html/report.html 2021-08-21T01:41:59.0892404Z Results written to /home/runner/work/jdk/jdk/build/run-test-prebuilt/test-support/jtreg_test_jdk_tier1_part1 2021-08-21T01:41:59.0895498Z Error: Some tests failed or other problems occurred. 2021-08-21T01:41:59.1235748Z Finished running test 'jtreg:test/jdk:tier1_part1' 2021-08-21T01:41:59.1237631Z Test report is stored in build/run-test-prebuilt/test-results/jtreg_test_jdk_tier1_part1 2021-08-21T01:41:59.1650100Z 2021-08-21T01:41:59.1667939Z == 2021-08-21T01:41:59.1668727Z Test summary 2021-08-21T01:41:59.1669635Z == 2021-08-21T01:41:59.1670187ZTEST TOTAL PASS FAIL ERROR 2021-08-21T01:41:59.1670795Z >> jtreg:test/jdk:tier1_part1 884 5 0 879 << - PR: https://git.openjdk.java.net/jdk/pull/5204
Re: RFR: 8270380: Change the default value of the java.security.manager system property to disallow
On Fri, 20 Aug 2021 22:44:34 GMT, Weijun Wang wrote:
> This change modifies the default value of the `java.security.manager` system
> property from "allow" to "disallow". This means unless it's explicitly set to
> "allow", any call to `System.setSecurityManager()` would throw an UOE.
>
> The `AllowSecurityManager.java` and `SecurityManagerWarnings.java` tests are
> updated to confirm this behavior change. Two other tests are updated because
> they were added after JDK-8267184 and do not have
> `-Djava.security.manager=allow` on its `@run` line even it they need to
> install a `SecurityManager` at runtime.
A somewhat broader question - I looked at the javadocs of this latest update to
`SecurityManager` in this PR. One thing I'm unclear about is, consider the case
where the `java.security.manager` is _not_ set to anything at the command line.
Then in some application code, let's say we have this:
String oldVal = System.getProperty("java.security.manager");
try {
System.setProperty("java.security.manager", "allow");
System.setSecurityManager(someSecurityManager);
// do something
} finally {
System.setProperty("java.security.manager", oldVal);
}
Would this then allow the security manager to be used? In other words, can the
value of `java.security.manager` be changed dynamically at runtime or is it
restricted to be set only at launch time (via command line arugment
`-Djava.security.manager`)?
-
PR: https://git.openjdk.java.net/jdk/pull/5204
Re: Implementing MethodHandleProxies#asInterfaceInstance with hidden classes
This was an early attempt at the functionality provided by LambdaMetafactory. It could probably be reimplemented on top of that, but probably could be deprecated in favor of LMF as well. Sent from my iPad > On Aug 22, 2021, at 10:08 PM, liangchenb...@gmail.com wrote: > > Currently, java.lang.invoke.MethodHandleProxies#asInterfaceInstance [1] is > implemented with java.lang.reflect.Proxy. After looking at its public API, > including Javadoc, it seems that MethodHandleProxies would benefit from a > hidden class implementation without changing its public API definition > (including Javadocs). > > Recently, there is JEP 416 [2] for reimplementing reflection based on > method handles. This implementation utilizes hidden classes with method > handles passed in classdata and retrieved to condy, which allows generic > method handles (beyond regular constable ones) to be optimized in method > calls. Similarly, for MethodHandleProxies, hidden classes allow the > implementations to detach from classloaders and be freely recyclable; they > can use class data to store the adapted method handles (which can > significantly speed up invocations); and it can allow it to support > implementing single-abstract-method abstract classes in the future (as > discussed in its Javadoc) as well, with only minor tweaks. > > Does this sound feasible? I want to ensure it is a good idea before any > implementation is attempted. If there is any issue with this vision, please > don't hesitate to point it out. Feel free to comment, too! If this looks > good, I hope an issue can be created for it. > > Best > > [1] > https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/lang/invoke/MethodHandleProxies.html > [2] https://openjdk.java.net/jeps/416
Implementing MethodHandleProxies#asInterfaceInstance with hidden classes
Currently, java.lang.invoke.MethodHandleProxies#asInterfaceInstance [1] is implemented with java.lang.reflect.Proxy. After looking at its public API, including Javadoc, it seems that MethodHandleProxies would benefit from a hidden class implementation without changing its public API definition (including Javadocs). Recently, there is JEP 416 [2] for reimplementing reflection based on method handles. This implementation utilizes hidden classes with method handles passed in classdata and retrieved to condy, which allows generic method handles (beyond regular constable ones) to be optimized in method calls. Similarly, for MethodHandleProxies, hidden classes allow the implementations to detach from classloaders and be freely recyclable; they can use class data to store the adapted method handles (which can significantly speed up invocations); and it can allow it to support implementing single-abstract-method abstract classes in the future (as discussed in its Javadoc) as well, with only minor tweaks. Does this sound feasible? I want to ensure it is a good idea before any implementation is attempted. If there is any issue with this vision, please don't hesitate to point it out. Feel free to comment, too! If this looks good, I hope an issue can be created for it. Best [1] https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/lang/invoke/MethodHandleProxies.html [2] https://openjdk.java.net/jeps/416
Re: RFR: 8269373: some tests in jdk/tools/launcher/ fails on localized Windows platform [v2]
On Wed, 4 Aug 2021 07:25:06 GMT, Masanori Yano wrote: >> Hi all, >> >> Could you please review the 8269373 bug fixes? >> >> These tests call java.lang.ProcessBuilder in direct, so not used jtreg >> command option. To run non-localized tests, -Duser.language=en and >> -Duser.country=US options should be added in ProcessBuilder. > > Masanori Yano has updated the pull request incrementally with one additional > commit since the last revision: > > 8269373: use test opts for process arguments IMHO, this kind of test that sets the locale forcefully sometimes gives us false positives. I'd rather eliminate that possibility than run tests in different locales - PR: https://git.openjdk.java.net/jdk/pull/4594
Re: RFR: 8271820: Implementation of JEP 416: Reimplement Core Reflection with Method Handle [v3]
On Sun, 22 Aug 2021 01:53:40 GMT, liach wrote: >> Mandy Chung has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Remove separate accessor for static vs instance method >> >> There is no effective difference when using MethodHandle::dropArgument for >> static method. Removing Static*Accessor and Instance*Accessor simplifies >> the implementation. > > src/java.base/share/classes/jdk/internal/reflect/DirectConstructorAccessorImpl.java > line 46: > >> 44: // fast invoker >> 45: var mhInvoker = newMethodHandleInvoker(ctor, target); >> 46: return new DirectConstructorAccessorImpl(ctor, target); > > The created fast invoker should be passed to the ctor than discarded. Good catch. Will fix. - PR: https://git.openjdk.java.net/jdk/pull/5027
Re: RFR: 8272805: Avoid looking up standard charsets
On Sun, 22 Aug 2021 02:53:44 GMT, Sergey Bylokhov wrote:
> This is the continuation of JDK-8233884, JDK-8271456, and JDK-8272120.
>
> In many places standard charsets are looked up via their names, for example:
> absolutePath.getBytes("UTF-8");
>
> This could be done more efficiently(up to x20 time faster) with use of
> java.nio.charset.StandardCharsets:
> absolutePath.getBytes(StandardCharsets.UTF_8);
>
> The later variant also makes the code cleaner, as it is known not to throw
> UnsupportedEncodingException in contrary to the former variant.
>
> This change includes:
> * demo/utils
> * jdk.xx packages
> * Some places were missed in the previous changes. I have found it by
> tracing the calls to the Charset.forName() by executing tier1,2,3 and desktop
> tests.
>
> Some performance discussion: https://github.com/openjdk/jdk/pull/5063
>
> Code excluded in this fix: the Xerces library(should be fixed upstream),
> J2DBench(should be compatible to 1.4), some code in the network(the change
> there are not straightforward, will do it later).
>
> Tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS.
The security related change looks fine to me.
-
Marked as reviewed by weijun (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/5210
RFR: 8272805: Avoid looking up standard charsets
This is the continuation of JDK-8233884, JDK-8271456, and JDK-8272120.
See https://github.com/openjdk/jdk/pull/5063 and
https://github.com/openjdk/jdk/pull/4951
In many places standard charsets are looked up via their names, for example:
absolutePath.getBytes("UTF-8");
This could be done more efficiently(up to x20 time faster) with use of
java.nio.charset.StandardCharsets:
absolutePath.getBytes(StandardCharsets.UTF_8);
The later variant also makes the code cleaner, as it is known not to throw
UnsupportedEncodingException in contrary to the former variant.
This change includes:
* demo/utils
* jdk.xx packages
* Some places were missed in the previous changes. I have found it by tracing
the calls to the Charset.forName() by executing tier1,2,3 and desktop tests.
Also checked for "aliases" usage.
Some performance discussion: https://github.com/openjdk/jdk/pull/5063
Code excluded in this fix: the Xerces library(should be fixed upstream),
J2DBench(should be compatible to 1.4), some code in the "java.naming"(the
change there is not straightforward, will do it later).
Tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS.
-
Commit messages:
- Fix related imports
- Merge branch 'master' into standard-encodings-in-non-public-modules
- Cleanup UnsupportedEncodingException
- Update PacketStream.java
- Rollback TextTests, should be compatible with jdk1.4
- Rollback TextRenderTests, should be compatible with jdk1.4
- Cleanup the UnsupportedEncodingException
- aliases for ISO_8859_1
- Update imageio
- Initial fix
Changes: https://git.openjdk.java.net/jdk/pull/5210/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5210=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8272805
Stats: 333 lines in 48 files changed: 91 ins; 123 del; 119 mod
Patch: https://git.openjdk.java.net/jdk/pull/5210.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/5210/head:pull/5210
PR: https://git.openjdk.java.net/jdk/pull/5210
