Re: RFR: 8191441: (Process) add Readers and Writer access to java.lang.Process streams [v4]
On Wed, 26 May 2021 17:58:06 GMT, Roger Riggs wrote: > Process is abstract. Is there any use for these new methods to be overridden? > Perhaps they should be final. It's not clear to me that it is useful to extend Process outside of the JDK. Testing, wrapping, ...? It feels like this class wants to be sealed. Maybe we should look at deprecating the no-arg constructor, like Joe did recently with AccessibleObject. - PR: https://git.openjdk.java.net/jdk/pull/4134
Re: RFR: 8265029: Preserve SIZED characteristics on slice operations (skip, limit) [v8]
> With the introduction of `toList()`, preserving the SIZED characteristics in > more cases becomes more important. This patch preserves SIZED on `skip()` and > `limit()` operations, so now every combination of > `map/mapToX/boxed/asXyzStream/skip/limit/sorted` preserves size, and > `toList()`, `toArray()` and `count()` may benefit from this. E. g., > `LongStream.range(0, 10_000_000_000L).skip(1).count()` returns result > instantly with this patch. > > Some microbenchmarks added that confirm the reduced memory allocation in > `toList()` and `toArray()` cases. Before patch: > > ref.SliceToList.seq_baseline:·gc.alloc.rate.norm1 > thrpt 10 40235,534 ± 0,984B/op > ref.SliceToList.seq_limit:·gc.alloc.rate.norm 1 > thrpt 10 106431,101 ± 0,198B/op > ref.SliceToList.seq_skipLimit:·gc.alloc.rate.norm 1 > thrpt 10 106544,977 ± 1,983B/op > value.SliceToArray.seq_baseline:·gc.alloc.rate.norm 1 > thrpt 10 40121,878 ± 0,247B/op > value.SliceToArray.seq_limit:·gc.alloc.rate.norm1 > thrpt 10 106317,693 ± 1,083B/op > value.SliceToArray.seq_skipLimit:·gc.alloc.rate.norm1 > thrpt 10 106430,954 ± 0,136B/op > > > After patch: > > ref.SliceToList.seq_baseline:·gc.alloc.rate.norm1 > thrpt 10 40235,648 ± 1,354B/op > ref.SliceToList.seq_limit:·gc.alloc.rate.norm 1 > thrpt 10 40355,784 ± 1,288B/op > ref.SliceToList.seq_skipLimit:·gc.alloc.rate.norm 1 > thrpt 10 40476,032 ± 2,855B/op > value.SliceToArray.seq_baseline:·gc.alloc.rate.norm 1 > thrpt 10 40121,830 ± 0,308B/op > value.SliceToArray.seq_limit:·gc.alloc.rate.norm1 > thrpt 10 40242,554 ± 0,443B/op > value.SliceToArray.seq_skipLimit:·gc.alloc.rate.norm1 > thrpt 10 40363,674 ± 1,576B/op > > > Time improvements are less exciting. It's likely that inlining and > vectorizing dominate in these tests over array allocations and unnecessary > copying. Still, I notice a significant improvement in SliceToArray.seq_limit > case (2x) and mild improvement (+12..16%) in other slice tests. No > significant change in parallel execution time, though its performance is much > less stable and I didn't run enough tests. > > Before patch: > > Benchmark (size) Mode Cnt Score Error > Units > ref.SliceToList.par_baseline 1 thrpt 30 14876,723 ± 99,770 > ops/s > ref.SliceToList.par_limit 1 thrpt 30 14856,841 ± 215,089 > ops/s > ref.SliceToList.par_skipLimit 1 thrpt 30 9555,818 ± 991,335 > ops/s > ref.SliceToList.seq_baseline 1 thrpt 30 23732,290 ± 444,162 > ops/s > ref.SliceToList.seq_limit 1 thrpt 30 14894,040 ± 176,496 > ops/s > ref.SliceToList.seq_skipLimit 1 thrpt 30 10646,929 ± 36,469 > ops/s > value.SliceToArray.par_baseline1 thrpt 30 25093,141 ± 376,402 > ops/s > value.SliceToArray.par_limit 1 thrpt 30 24798,889 ± 760,762 > ops/s > value.SliceToArray.par_skipLimit 1 thrpt 30 16456,310 ± 926,882 > ops/s > value.SliceToArray.seq_baseline1 thrpt 30 69669,787 ± 494,562 > ops/s > value.SliceToArray.seq_limit 1 thrpt 30 21097,081 ± 117,338 > ops/s > value.SliceToArray.seq_skipLimit 1 thrpt 30 15522,871 ± 112,557 > ops/s > > > After patch: > > Benchmark (size) Mode Cnt Score Error > Units > ref.SliceToList.par_baseline 1 thrpt 30 14793,373 ± 64,905 > ops/s > ref.SliceToList.par_limit 1 thrpt 30 13301,024 ± 1300,431 > ops/s > ref.SliceToList.par_skipLimit 1 thrpt 30 11131,698 ± 1769,932 > ops/s > ref.SliceToList.seq_baseline 1 thrpt 30 24101,048 ± 263,528 > ops/s > ref.SliceToList.seq_limit 1 thrpt 30 16872,168 ± 76,696 > ops/s > ref.SliceToList.seq_skipLimit 1 thrpt 30 11953,253 ± 105,231 > ops/s > value.SliceToArray.par_baseline1 thrpt 30 25442,442 ± 455,554 > ops/s > value.SliceToArray.par_limit 1 thrpt 30 23111,730 ± 2246,086 > ops/s > value.SliceToArray.par_skipLimit 1 thrpt 30 17980,750 ± 2329,077 > ops/s > value.SliceToArray.seq_baseline1 thrpt 30 66512,898 ± 1001,042 > ops/s > value.SliceToArray.seq_limit 1 thrpt 30 41792,549 ± 1085,547 > ops/s > value.SliceToArray.seq_skipLimit 1 thrpt 30 18007,613 ± 141,716 > ops/s > > > I also modernized SliceOps a little bit, using switch expression (with no > explicit default!) and diamonds on anonymous classes. Tagir F. Valeev has updated the pull request incr
Re: RFR: 8265029: Preserve SIZED characteristics on slice operations (skip, limit) [v7]
On Mon, 24 May 2021 19:51:04 GMT, Paul Sandoz wrote:
>> Tagir F. Valeev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Trailing whitespace removed
>
> src/java.base/share/classes/java/util/stream/AbstractPipeline.java line 471:
>
>> 469: int flags = getStreamAndOpFlags();
>> 470: long size = StreamOpFlag.SIZED.isKnown(flags) ?
>> spliterator.getExactSizeIfKnown() : -1;
>> 471: if (size != -1 && StreamOpFlag.SIZE_ADJUSTING.isKnown(flags) &&
>> !isParallel()) {
>
> Very nice. It's a good compromise to support only for sequential streams,
> since we have no size adjusting intermediate stateless op. If that was the
> case we would need to step back through the pipeline until the depth was
> zero, then step forward. I think it worth a comment here to inform our future
> selves if we ever add such an operation.
>
> Strictly speaking we only need to call `exactOutputSize` if the stage is size
> adjusting. Not sure it really matters perf-wise. If we leave as is maybe add
> a comment.
It's hard to imagine SIZE_ADJUSTING stateless intermediate op (probably the op
that duplicates every stream element like `flatMap(x -> Stream.of(x, x))` as a
single op?). Nevertheless, I added the comment. Please check if it's clear
enough. Thanks!
-
PR: https://git.openjdk.java.net/jdk/pull/3427
Re: RFR: 8267123: Remove RMI Activation
On Tue, 25 May 2021 18:04:45 GMT, Stuart Marks wrote: > This is the implementation of [JEP 407](https://openjdk.java.net/jeps/407). > > This is a fairly straightforward removal of this component. > - Activation implementation classes removed > - Activation tests removed > - adjustments to various comments to remove references to Activation > - adjustments to some code to remove special-case support for Activation > from core RMI > - adjustments to several tests to remove testing and support for, and > mentions of Activation > - remove `rmid` tool > > (Personally, I found that reviewing using the webrev was easier than > navigating github's diff viewer.) Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4194
Re: RFR: 8195129: System.load() fails to load from unicode paths [v2]
On Tue, 25 May 2021 09:45:34 GMT, Maxim Kartashev wrote: >> Character strings within JVM are produced and consumed in several formats. >> Strings come from/to Java in the UTF8 format and POSIX APIs (like fprintf() >> or dlopen()) consume strings also in UTF8. On Windows, however, the >> situation is far less simple: some new(er) APIs expect UTF16 (wide-character >> strings), some older APIs can only work with strings in a "platform" format, >> where not all UTF8 characters can be represented; which ones can depends on >> the current "code page". >> >> This commit switches the Windows version of native library loading code to >> using the new UTF16 API `LoadLibraryW()` and attempts to streamline the use >> of various string formats in the surrounding code. >> >> Namely, exception messages are made to consume strings explicitly in the >> UTF8 format, while logging functions (that end up using legacy Windows API) >> are made to consume "platform" strings in most cases. One exception is >> `JVM_LoadLibrary()` logging where the UTF8 name of the library is logged, >> which can, of course, be fixed, but was considered not worth the additional >> code (NB: this isn't a new bug). >> >> The test runs in a separate JVM in order to make NIO happy about non-ASCII >> characters in the file name; tests are executed with LC_ALL=C and that >> doesn't let NIO work with non-ASCII file names even on Linux or MacOS. >> >> Tested by running `test/hotspot/jtreg:tier1` on Linux and >> `jtreg:test/hotspot/jtreg/runtime` on Windows 10. The new test (` >> jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode`) was explicitly ran >> on those platforms as well. >> >> Results from Linux: >> >> Test summary >> == >>TEST TOTAL PASS FAIL ERROR >> >>jtreg:test/hotspot/jtreg:tier1 1784 1784 0 0 >> >> == >> TEST SUCCESS >> >> >> Building target 'run-test-only' in configuration >> 'linux-x86_64-server-release' >> Test selection 'jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode', >> will run: >> * jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode >> >> Running test 'jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode' >> Passed: runtime/jni/loadLibraryUnicode/LoadLibraryUnicodeTest.java >> Test results: passed: 1 >> >> >> Results from Windows 10: >> >> Test summary >> == >>TEST TOTAL PASS FAIL ERROR >>jtreg:test/hotspot/jtreg/runtime746 746 0 0 >> == >> TEST SUCCESS >> Finished building target 'run-test-only' in configuration >> 'windows-x86_64-server-fastdebug' >> >> >> Building target 'run-test-only' in configuration >> 'windows-x86_64-server-fastdebug' >> Test selection 'test/hotspot/jtreg/runtime/jni/loadLibraryUnicode', will run: >> * jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode >> >> Running test 'jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode' >> Passed: runtime/jni/loadLibraryUnicode/LoadLibraryUnicodeTest.java >> Test results: passed: 1 > > Maxim Kartashev has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. The pull request contains one new > commit since the last revision: > > 8195129: System.load() fails to load from unicode paths Hi Maxim, Overall this seems okay. I've focused mainly on the hotspot parts, including the test. A few minor changes requested. I do have some concerns about the impact on startup though and the efficiency of the conversion routines. Thanks, David src/hotspot/os/windows/os_windows.cpp line 1462: > 1460: const int flag_source_str_is_null_terminated = -1; > 1461: const int flag_estimate_chars_count = 0; > 1462: int utf16_chars_count_estimated = MultiByteToWideChar(source_encoding, Your local naming style is somewhat excessive. You could just comment the values of the flags when you pass them eg: MultiByteToWideChar(source_encoding, MB_ERR_INVALID_CHARS, source_str, -1, //source is null-terminated NULL, // no output buffer 0); // calculate required buffer size Or you could just add a comment before the call: // Perform a dummy conversion so that we can get the required size of the buffer to // allocate. The source is null-terminated. Trying to document parameter semantics by variable naming doesn't work in my opinion - at some point if you want to know you have to RTFM for the API. And utf16_len is perfectly adequate for the returned size. src/hotspot/os/windows/os_windows.cpp line 1541: > 1539: void * os::dll_load(co
Re: RFR: 8195129: System.load() fails to load from unicode paths [v2]
On Tue, 25 May 2021 09:45:34 GMT, Maxim Kartashev wrote: >> Character strings within JVM are produced and consumed in several formats. >> Strings come from/to Java in the UTF8 format and POSIX APIs (like fprintf() >> or dlopen()) consume strings also in UTF8. On Windows, however, the >> situation is far less simple: some new(er) APIs expect UTF16 (wide-character >> strings), some older APIs can only work with strings in a "platform" format, >> where not all UTF8 characters can be represented; which ones can depends on >> the current "code page". >> >> This commit switches the Windows version of native library loading code to >> using the new UTF16 API `LoadLibraryW()` and attempts to streamline the use >> of various string formats in the surrounding code. >> >> Namely, exception messages are made to consume strings explicitly in the >> UTF8 format, while logging functions (that end up using legacy Windows API) >> are made to consume "platform" strings in most cases. One exception is >> `JVM_LoadLibrary()` logging where the UTF8 name of the library is logged, >> which can, of course, be fixed, but was considered not worth the additional >> code (NB: this isn't a new bug). >> >> The test runs in a separate JVM in order to make NIO happy about non-ASCII >> characters in the file name; tests are executed with LC_ALL=C and that >> doesn't let NIO work with non-ASCII file names even on Linux or MacOS. >> >> Tested by running `test/hotspot/jtreg:tier1` on Linux and >> `jtreg:test/hotspot/jtreg/runtime` on Windows 10. The new test (` >> jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode`) was explicitly ran >> on those platforms as well. >> >> Results from Linux: >> >> Test summary >> == >>TEST TOTAL PASS FAIL ERROR >> >>jtreg:test/hotspot/jtreg:tier1 1784 1784 0 0 >> >> == >> TEST SUCCESS >> >> >> Building target 'run-test-only' in configuration >> 'linux-x86_64-server-release' >> Test selection 'jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode', >> will run: >> * jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode >> >> Running test 'jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode' >> Passed: runtime/jni/loadLibraryUnicode/LoadLibraryUnicodeTest.java >> Test results: passed: 1 >> >> >> Results from Windows 10: >> >> Test summary >> == >>TEST TOTAL PASS FAIL ERROR >>jtreg:test/hotspot/jtreg/runtime746 746 0 0 >> == >> TEST SUCCESS >> Finished building target 'run-test-only' in configuration >> 'windows-x86_64-server-fastdebug' >> >> >> Building target 'run-test-only' in configuration >> 'windows-x86_64-server-fastdebug' >> Test selection 'test/hotspot/jtreg/runtime/jni/loadLibraryUnicode', will run: >> * jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode >> >> Running test 'jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode' >> Passed: runtime/jni/loadLibraryUnicode/LoadLibraryUnicodeTest.java >> Test results: passed: 1 > > Maxim Kartashev has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. The pull request contains one new > commit since the last revision: > > 8195129: System.load() fails to load from unicode paths Hi Maxim, Please don't force-push changes once you have opened a PR and review has commenced as it messes up the history and comments. Just push each commit directly. The final integration will squash everything into one clean commit. Thanks, David - PR: https://git.openjdk.java.net/jdk/pull/4169
Re: RFR: 8195129: System.load() fails to load from unicode paths [v2]
On 27/05/2021 6:59 am, Gerard Ziemski wrote:
On Tue, 25 May 2021 09:45:34 GMT, Maxim Kartashev
wrote:
I tried verifying the test on **macOS** by running:
`jtreg -nr -va -jdk:./build/macosx-x86_64-server-fastdebug/images/jdk
test/hotspot/jtreg/runtime/jni/loadLibraryUnicode`
and I get:
`TEST RESULT: Error. Use -nativepath to specify the location of native code`
I looked at the test and thought it was handling loading the native lib on its
own, without the test needing to set **nativepath** explicitly. What am I
doing wrong?
You always have to set nativepath explicitly for jtreg. The test then
reads the property that jtreg defines using the value of -nativepath:
String testNativePath =
LoadLibraryUnicodeTest.getSystemProperty("test.nativepath");
Cheers,
David
-
-
PR: https://git.openjdk.java.net/jdk/pull/4169
Re: RFR: 8264859: Implement Context-Specific Deserialization Filters [v10]
On Wed, 26 May 2021 22:02:36 GMT, Brent Christian wrote:
>> Roger Riggs has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Editorial updates to review comments.
>> Simplify the builtin filter factory implementation.
>> Add atomic update to setting the filter factory.
>> Clarify the description of OIS.setObjectInputFilter.
>> Cleanup of the example code.
>
> test/jdk/java/io/Serializable/serialFilter/SerialFilterFactoryTest.java line
> 332:
>
>> 330: public void next(ObjectInputFilter next) {
>> 331: this. next = next;
>> 332: }
>
> Is this next() used anywhere ?
Neither is used, IntellJ was happy to create getters and setters... Can be
removed.
-
PR: https://git.openjdk.java.net/jdk/pull/3996
Re: RFR: 8266310: deadlock while loading the JNI code [v5]
On Fri, 21 May 2021 15:49:09 GMT, Aleksei Voitylov wrote: >> Please review this PR which fixes the deadlock in ClassLoader between the >> two lock objects - a lock object associated with the class being loaded, and >> the ClassLoader.loadedLibraryNames hash map, locked during the native >> library load operation. >> >> Problem being fixed: >> >> The initial reproducer demonstrated a deadlock between the JarFile/ZipFile >> and the hash map. That deadlock exists even when the ZipFile/JarFile lock is >> removed because there's another lock object in the class loader, associated >> with the name of the class being loaded. Such objects are stored in >> ClassLoader.parallelLockMap. The deadlock occurs when JNI_OnLoad() loads >> exactly the same class, whose signature is being verified in another thread. >> >> Proposed fix: >> >> The proposed patch suggests to get rid of locking loadedLibraryNames hash >> map and synchronize on each entry name, as it's done with class names in see >> ClassLoader.getClassLoadingLock(name) method. >> >> The patch introduces nativeLibraryLockMap which holds the lock objects for >> each library name, and the getNativeLibraryLock() private method is used to >> lazily initialize the corresponding lock object. nativeLibraryContext was >> changed to ThreadLocal, so that in any concurrent thread it would have a >> NativeLibrary object on top of the stack, that's being currently >> loaded/unloaded in that thread. nativeLibraryLockMap accumulates the names >> of all native libraries loaded - in line with class loading code, it is not >> explicitly cleared. >> >> Testing: jtreg and jck testing with no regressions. A new regression test >> was developed. > > Aleksei Voitylov has updated the pull request incrementally with one > additional commit since the last revision: > > fix whitespace Hi Aleksei, I'll review it next week as I'm taking some time off this week. Mandy - PR: https://git.openjdk.java.net/jdk/pull/3976
Re: RFR: 8264859: Implement Context-Specific Deserialization Filters [v10]
On Wed, 26 May 2021 16:48:53 GMT, Roger Riggs wrote:
>> JEP 415: Context-specific Deserialization Filters extends the
>> deserialization filtering mechanisms with more flexible and customizable
>> protections against malicious deserialization. See JEP 415:
>> https://openjdk.java.net/jeps/415.
>> The `java.io.ObjectInputFilter` and `java.io.ObjectInputStream` classes are
>> extended with additional
>> configuration mechanisms and filter utilities.
>>
>> javadoc for `ObjectInputFilter`, `ObjectInputFilter.Config`, and
>> `ObjectInputStream`:
>>
>> http://cr.openjdk.java.net/~rriggs/filter-factory/java.base/java/io/ObjectInputFilter.html
>
> Roger Riggs has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Editorial updates to review comments.
> Simplify the builtin filter factory implementation.
> Add atomic update to setting the filter factory.
> Clarify the description of OIS.setObjectInputFilter.
> Cleanup of the example code.
test/jdk/java/io/Serializable/serialFilter/SerialFilterFactoryTest.java line
328:
> 326: public void current(ObjectInputFilter current) {
> 327: this.current = current;
> 328: }
Is this current() used anywhere ?
test/jdk/java/io/Serializable/serialFilter/SerialFilterFactoryTest.java line
332:
> 330: public void next(ObjectInputFilter next) {
> 331: this. next = next;
> 332: }
Is this next() used anywhere ?
-
PR: https://git.openjdk.java.net/jdk/pull/3996
Re: RFR: 8264859: Implement Context-Specific Deserialization Filters [v12]
> JEP 415: Context-specific Deserialization Filters extends the deserialization > filtering mechanisms with more flexible and customizable protections against > malicious deserialization. See JEP 415: https://openjdk.java.net/jeps/415. > The `java.io.ObjectInputFilter` and `java.io.ObjectInputStream` classes are > extended with additional > configuration mechanisms and filter utilities. > > javadoc for `ObjectInputFilter`, `ObjectInputFilter.Config`, and > `ObjectInputStream`: > > http://cr.openjdk.java.net/~rriggs/filter-factory/java.base/java/io/ObjectInputFilter.html Roger Riggs 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 13 additional commits since the last revision: - Merge branch 'master' into 8264859-context-filter-factory - Added test for rejectUndecidedClass array cases Added test for preventing disabling filter factory Test cleanup - Editorial updates to review comments. Simplify the builtin filter factory implementation. Add atomic update to setting the filter factory. Clarify the description of OIS.setObjectInputFilter. Cleanup of the example code. - Editorial updates Updated java.security properties to include jdk.serialFilterFactory Added test cases to SerialFilterFactoryTest for java.security properties and enabling of the SecurityManager with existing policy permission files Corrected a test that OIS.setObjectInputFilter could not be called twice. Removed a Factory test that was not intended to be committed - Moved utility filter methods to be static on ObjectInputFilter Rearranged the class javadoc of OIF to describe the parts of deserialization filtering, filters, composite filters, and the filter factory. And other review comment updates... - Refactored tests for utility functions to SerialFilterFunctionTest.java Deleted confused Config.allowMaxLimits() method Updated example to match move of methods to Config Added test of restriction on setting the filterfactory after a OIS has been created Additional Editorial updates - Move merge and rejectUndecidedClass methods to OIF.Config As default methods on OIF, their implementations were not concrete and not trustable - Review suggestions included; Added @implSpec for default methods in OIF; Added restriction that the filter factory cannot be set after an ObjectInputStream has been created and applied the current filter factory - Editorial javadoc updated based on review comments. Clarified behavior of rejectUndecidedClass method. Example test added to check status returned from file. - Editorial updates to review comments Add filter tracing support - ... and 3 more: https://git.openjdk.java.net/jdk/compare/9870b028...0930f0f8 - Changes: - all: https://git.openjdk.java.net/jdk/pull/3996/files - new: https://git.openjdk.java.net/jdk/pull/3996/files/19b6aad3..0930f0f8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3996&range=11 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3996&range=10-11 Stats: 44803 lines in 2037 files changed: 20137 ins; 18278 del; 6388 mod Patch: https://git.openjdk.java.net/jdk/pull/3996.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3996/head:pull/3996 PR: https://git.openjdk.java.net/jdk/pull/3996
Integrated: 8267751: (test) jtreg.SkippedException has no serial VersionUID
On Wed, 26 May 2021 01:28:17 GMT, Roger Riggs wrote: > The class `test/lib/jtreg/SkippedException.java` is missing a > serialVersionUID causing additional noise in compiler output of tests. This pull request has now been integrated. Changeset: 0fc7c8d1 Author:Roger Riggs URL: https://git.openjdk.java.net/jdk/commit/0fc7c8d101d526f1bc86831996b6883209d77451 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod 8267751: (test) jtreg.SkippedException has no serial VersionUID Reviewed-by: naoto, iignatyev, iris - PR: https://git.openjdk.java.net/jdk/pull/4197
Re: RFR: 8191441: (Process) add Readers and Writer access to java.lang.Process streams [v4]
On Tue, 25 May 2021 16:53:28 GMT, Roger Riggs wrote:
>> Methods are added to java.lang.Process to read and write characters and
>> lines from and to a spawned Process.
>> The Charset used to encode and decode characters to bytes can be specified
>> or use the
>> operating system native encoding as is available from the "native.encoding"
>> system property.
>
> Roger Riggs has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Described charset mapping of malformed chars in outputWriter
> Repeated calls to inputReader, errorReader, and outputWriter now return
> the same instance
> and check for inconsistent charset argument
> Added warnings about concurrently use of input/output streams and
> readers/writers.
src/java.base/share/classes/java/lang/Process.java line 254:
> 252: * {@code
> 253: * return new BufferedReader(new
> InputStreamReader(getInputStream(), charset));
> 254: * }
Does not seem to be equivalent any longer. Also, should it describe the
behavior that it rejects the different Charset after the first invocation?
src/java.base/share/classes/java/lang/Process.java line 326:
> 324: * {@code
> 325: * return new BufferedReader(new
> InputStreamReader(getErrorStream(), charset));
> 326: * }
Same comment in `inputReader(Charset)` stands here too.
-
PR: https://git.openjdk.java.net/jdk/pull/4134
Re: RFR: 8264859: Implement Context-Specific Deserialization Filters [v11]
> JEP 415: Context-specific Deserialization Filters extends the deserialization > filtering mechanisms with more flexible and customizable protections against > malicious deserialization. See JEP 415: https://openjdk.java.net/jeps/415. > The `java.io.ObjectInputFilter` and `java.io.ObjectInputStream` classes are > extended with additional > configuration mechanisms and filter utilities. > > javadoc for `ObjectInputFilter`, `ObjectInputFilter.Config`, and > `ObjectInputStream`: > > http://cr.openjdk.java.net/~rriggs/filter-factory/java.base/java/io/ObjectInputFilter.html Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Added test for rejectUndecidedClass array cases Added test for preventing disabling filter factory Test cleanup - Changes: - all: https://git.openjdk.java.net/jdk/pull/3996/files - new: https://git.openjdk.java.net/jdk/pull/3996/files/a65d6205..19b6aad3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3996&range=10 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3996&range=09-10 Stats: 71 lines in 3 files changed: 55 ins; 7 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/3996.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3996/head:pull/3996 PR: https://git.openjdk.java.net/jdk/pull/3996
Re: RFR: 8195129: System.load() fails to load from unicode paths [v2]
On Tue, 25 May 2021 09:45:34 GMT, Maxim Kartashev wrote: >> Character strings within JVM are produced and consumed in several formats. >> Strings come from/to Java in the UTF8 format and POSIX APIs (like fprintf() >> or dlopen()) consume strings also in UTF8. On Windows, however, the >> situation is far less simple: some new(er) APIs expect UTF16 (wide-character >> strings), some older APIs can only work with strings in a "platform" format, >> where not all UTF8 characters can be represented; which ones can depends on >> the current "code page". >> >> This commit switches the Windows version of native library loading code to >> using the new UTF16 API `LoadLibraryW()` and attempts to streamline the use >> of various string formats in the surrounding code. >> >> Namely, exception messages are made to consume strings explicitly in the >> UTF8 format, while logging functions (that end up using legacy Windows API) >> are made to consume "platform" strings in most cases. One exception is >> `JVM_LoadLibrary()` logging where the UTF8 name of the library is logged, >> which can, of course, be fixed, but was considered not worth the additional >> code (NB: this isn't a new bug). >> >> The test runs in a separate JVM in order to make NIO happy about non-ASCII >> characters in the file name; tests are executed with LC_ALL=C and that >> doesn't let NIO work with non-ASCII file names even on Linux or MacOS. >> >> Tested by running `test/hotspot/jtreg:tier1` on Linux and >> `jtreg:test/hotspot/jtreg/runtime` on Windows 10. The new test (` >> jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode`) was explicitly ran >> on those platforms as well. >> >> Results from Linux: >> >> Test summary >> == >>TEST TOTAL PASS FAIL ERROR >> >>jtreg:test/hotspot/jtreg:tier1 1784 1784 0 0 >> >> == >> TEST SUCCESS >> >> >> Building target 'run-test-only' in configuration >> 'linux-x86_64-server-release' >> Test selection 'jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode', >> will run: >> * jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode >> >> Running test 'jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode' >> Passed: runtime/jni/loadLibraryUnicode/LoadLibraryUnicodeTest.java >> Test results: passed: 1 >> >> >> Results from Windows 10: >> >> Test summary >> == >>TEST TOTAL PASS FAIL ERROR >>jtreg:test/hotspot/jtreg/runtime746 746 0 0 >> == >> TEST SUCCESS >> Finished building target 'run-test-only' in configuration >> 'windows-x86_64-server-fastdebug' >> >> >> Building target 'run-test-only' in configuration >> 'windows-x86_64-server-fastdebug' >> Test selection 'test/hotspot/jtreg/runtime/jni/loadLibraryUnicode', will run: >> * jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode >> >> Running test 'jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode' >> Passed: runtime/jni/loadLibraryUnicode/LoadLibraryUnicodeTest.java >> Test results: passed: 1 > > Maxim Kartashev has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. The pull request contains one new > commit since the last revision: > > 8195129: System.load() fails to load from unicode paths I tried verifying the test on **macOS** by running: `jtreg -nr -va -jdk:./build/macosx-x86_64-server-fastdebug/images/jdk test/hotspot/jtreg/runtime/jni/loadLibraryUnicode` and I get: `TEST RESULT: Error. Use -nativepath to specify the location of native code` I looked at the test and thought it was handling loading the native lib on its own, without the test needing to set **nativepath** explicitly. What am I doing wrong? - PR: https://git.openjdk.java.net/jdk/pull/4169
Integrated: 8266851: Implement JEP 403: Strongly Encapsulate JDK Internals
On Thu, 13 May 2021 17:14:36 GMT, Mark Reinhold wrote:
> Please review this implementation of JEP 403
> (https://openjdk.java.net/jeps/403).
> Alan Bateman is the original author of almost all of it. Passes tiers 1-3 on
> {linux,macos,windows}-x64 and {linux,macos}-aarch64.
This pull request has now been integrated.
Changeset: e6302354
Author:Mark Reinhold
URL:
https://git.openjdk.java.net/jdk/commit/e63023546aaf48ae39c72ab37f6ef3f5474e19cc
Stats: 2842 lines in 26 files changed: 0 ins; 2792 del; 50 mod
8266851: Implement JEP 403: Strongly Encapsulate JDK Internals
Co-authored-by: Alan Bateman
Reviewed-by: mchung, alanb, hseigel
-
PR: https://git.openjdk.java.net/jdk/pull/4015
Re: RFR: 8262891: Compiler implementation for Pattern Matching for switch (Preview) [v4]
On Tue, 25 May 2021 16:14:56 GMT, Jan Lahoda wrote: > I'd like to note this is a preview feature - we can change the desugaring. At > the same time, I don't think this does not work with sub-patterns (those can > be easily desugared to guards, I think). Yes, but in that case the classcheck du to a sub-pattern matching will be either an instanceof or some other invokedynamic. Having several indy will bloat the code and if we use instanceof, why not using instanceof all along the way. > Regarding efficiency, it may be a balance between classfile overhead (which > will be higher if we need to desugar every guard to a separate method), and > the possibility to tweak the matching at runtime. I fear more the 2 bytes length bytecode limit of a method than the constant pool length limit mostly because people are already using a bunch of lambdas to simulate pattern matching. - PR: https://git.openjdk.java.net/jdk/pull/3863
Re: RFR: 8267123: Remove RMI Activation
On Tue, 25 May 2021 18:04:45 GMT, Stuart Marks wrote: > This is the implementation of [JEP 407](https://openjdk.java.net/jeps/407). > > This is a fairly straightforward removal of this component. > - Activation implementation classes removed > - Activation tests removed > - adjustments to various comments to remove references to Activation > - adjustments to some code to remove special-case support for Activation > from core RMI > - adjustments to several tests to remove testing and support for, and > mentions of Activation > - remove `rmid` tool > > (Personally, I found that reviewing using the webrev was easier than > navigating github's diff viewer.) AlanBateman wrote: > test/jdk/sun/rmi/log/ReliableLog/LogAlignmentTest.java Oh yes, this test mentions `rmid`, but it is basically a direct test of the `ReliableLog` stuff and it doesn't actually use or depend on Activation or `rmid`. The test still runs and passes. I'll do a followup pass to see if anything in the system uses anything in the `sun.rmi.log` package. Maybe the only user was Activation. - PR: https://git.openjdk.java.net/jdk/pull/4194
Re: RFR: 8262891: Compiler implementation for Pattern Matching for switch (Preview) [v5]
On Wed, 26 May 2021 17:52:36 GMT, Jan Lahoda wrote: >> This is a preview of a patch implementing JEP 406: Pattern Matching for >> switch (Preview): >> https://bugs.openjdk.java.net/browse/JDK-8213076 >> >> The current draft of the specification is here: >> http://cr.openjdk.java.net/~gbierman/jep406/jep406-20210430/specs/patterns-switch-jls.html >> >> A summary of notable parts of the patch: >> -to support cases expressions and patterns in cases, there is a new common >> superinterface for expressions and patterns, `CaseLabelTree`, which >> expressions and patterns implement, and a list of case labels is returned >> from `CaseTree.getLabels()`. >> -to support `case default`, there is an implementation of `CaseLabelTree` >> that represents it (`DefaultCaseLabelTree`). It is used also to represent >> the conventional `default` internally and in the newly added methods. >> -in the parser, parenthesized patterns and expressions need to be >> disambiguated when parsing case labels. >> -Lower has been enhanced to handle `case null` for ordinary >> (boxed-primitive, String, enum) switches. This is a bit tricky for boxed >> primitives, as there is no value that is not part of the input domain so >> that could be used to represent `case null`. Requires a bit shuffling with >> values. >> -TransPatterns has been enhanced to handle the pattern matching switch. It >> produces code that delegates to a new bootstrap method, that will classify >> the input value to the switch and return the case number, to which the >> switch then jumps. To support guards, the switches (and the bootstrap >> method) are restartable. The bootstrap method as such is written very simply >> so far, but could be much more optimized later. >> -nullable type patterns are `case String s, null`/`case null, String >> s`/`case null: case String s:`/`case String s: case null:`, handling of >> these required a few tricks in `Attr`, `Flow` and `TransPatterns`. >> >> The specdiff for the change is here (to be updated): >> http://cr.openjdk.java.net/~jlahoda/8265981/specdiff.preview.01/overview-summary.html > > Jan Lahoda has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains 12 commits: > > - Post-merge fix - need to include jdk.internal.javac in the list of > packages used by jdk.compiler again, as we now (again) have a preview feature > in javac. > - Correcting LineNumberTable for rule switches. > - Merging master into JDK-8262891 > - Fixing various error-related bugs. > - Avoiding fall-through from the total case to a synthetic default but > changing total patterns to default. > - Reflecting recent spec changes. > - Reflecting review comments. > - Reflecting review comments on SwitchBootstraps. > - Trailing whitespaces. > - Cleanup, reflecting review comments. > - ... and 2 more: > https://git.openjdk.java.net/jdk/compare/083416d3...fd748501 In: https://github.com/openjdk/jdk/pull/3863/commits/7d1abc141ecfecaf0798a2bad899705c116154e7 I've updated the code to produce better/more useful LineNumberTable for rule switches. - PR: https://git.openjdk.java.net/jdk/pull/3863
Integrated: 8265248: Implementation Specific Properties: change prefix, plus add existing properties
On Fri, 23 Apr 2021 00:41:17 GMT, Joe Wang wrote: > Update module summary, add a few existing properties and features into the > tables. This pull request has now been integrated. Changeset: 8c4719a5 Author:Joe Wang URL: https://git.openjdk.java.net/jdk/commit/8c4719a58834dddcea39d69b199abf1aabf780e2 Stats: 2869 lines in 61 files changed: 1571 ins; 678 del; 620 mod 8265248: Implementation Specific Properties: change prefix, plus add existing properties Reviewed-by: lancea, rriggs - PR: https://git.openjdk.java.net/jdk/pull/3644
Re: RFR: 8267751: (test) jtreg.SkippedException has no serial VersionUID [v2]
On Wed, 26 May 2021 17:52:31 GMT, Roger Riggs wrote: >> The class `test/lib/jtreg/SkippedException.java` is missing a >> serialVersionUID causing additional noise in compiler output of tests. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Removed unintended classfile Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4197
Re: RFR: 8187649: ArrayIndexOutOfBoundsException in java.util.JapaneseImperialCalendar
On Tue, 25 May 2021 16:40:53 GMT, Naoto Sato wrote: > Please review the fix. The issue was informed yesterday by @amaembo that it > offends some code analysis tools. > Although the fix is to change the condition error, it turned out that > `JapaneseImperialCalendar.roll()` did not work for `WEEK_OF_MONTH` and > `DAY_OF_MONTH`. There was an inherent issue where `GregorianCalendar` does > not follow the `roll()` spec, i.e., "The MONTH must not change when the > WEEK_OF_MONTH is rolled." during the Julian/Gregorian transition. JDK-6191841 > seems to have tried to fix it but it was closed as `Future Project`... > So I simply fixed the `roll()` issue in `JapaneseImperialCalendar` to follow > the spec here, which seems to be the intent of the original author. Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4191
Re: RFR: 8267123: Remove RMI Activation
On Tue, 25 May 2021 18:04:45 GMT, Stuart Marks wrote: > This is the implementation of [JEP 407](https://openjdk.java.net/jeps/407). > > This is a fairly straightforward removal of this component. > - Activation implementation classes removed > - Activation tests removed > - adjustments to various comments to remove references to Activation > - adjustments to some code to remove special-case support for Activation > from core RMI > - adjustments to several tests to remove testing and support for, and > mentions of Activation > - remove `rmid` tool > > (Personally, I found that reviewing using the webrev was easier than > navigating github's diff viewer.) LGTM - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4194
Re: RFR: 8265248: Implementation Specific Properties: change prefix, plus add existing properties [v7]
On Mon, 24 May 2021 06:23:59 GMT, Joe Wang wrote: >> Update module summary, add a few existing properties and features into the >> tables. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > Consolidated impl specific properties; deprecated legacy properties; made > new ones take preference Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3644
Re: RFR: 8267751: (test) jtreg.SkippedException has no serial VersionUID [v2]
On Wed, 26 May 2021 17:52:31 GMT, Roger Riggs wrote: >> The class `test/lib/jtreg/SkippedException.java` is missing a >> serialVersionUID causing additional noise in compiler output of tests. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Removed unintended classfile LGTM - Marked as reviewed by iignatyev (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4197
Re: RFR: 8191441: (Process) add Readers and Writer access to java.lang.Process streams [v4]
On Tue, 25 May 2021 16:53:28 GMT, Roger Riggs wrote: >> Methods are added to java.lang.Process to read and write characters and >> lines from and to a spawned Process. >> The Charset used to encode and decode characters to bytes can be specified >> or use the >> operating system native encoding as is available from the "native.encoding" >> system property. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Described charset mapping of malformed chars in outputWriter > Repeated calls to inputReader, errorReader, and outputWriter now return > the same instance > and check for inconsistent charset argument > Added warnings about concurrently use of input/output streams and > readers/writers. Process is abstract. Is there any use for these new methods to be overridden? Perhaps they should be final. The suggestion in the CSR was to document them using @ implSpec, to acknowledge that the subclass can do something different. - PR: https://git.openjdk.java.net/jdk/pull/4134
Re: RFR: 8262891: Compiler implementation for Pattern Matching for switch (Preview) [v5]
> This is a preview of a patch implementing JEP 406: Pattern Matching for > switch (Preview): > https://bugs.openjdk.java.net/browse/JDK-8213076 > > The current draft of the specification is here: > http://cr.openjdk.java.net/~gbierman/jep406/jep406-20210430/specs/patterns-switch-jls.html > > A summary of notable parts of the patch: > -to support cases expressions and patterns in cases, there is a new common > superinterface for expressions and patterns, `CaseLabelTree`, which > expressions and patterns implement, and a list of case labels is returned > from `CaseTree.getLabels()`. > -to support `case default`, there is an implementation of `CaseLabelTree` > that represents it (`DefaultCaseLabelTree`). It is used also to represent the > conventional `default` internally and in the newly added methods. > -in the parser, parenthesized patterns and expressions need to be > disambiguated when parsing case labels. > -Lower has been enhanced to handle `case null` for ordinary (boxed-primitive, > String, enum) switches. This is a bit tricky for boxed primitives, as there > is no value that is not part of the input domain so that could be used to > represent `case null`. Requires a bit shuffling with values. > -TransPatterns has been enhanced to handle the pattern matching switch. It > produces code that delegates to a new bootstrap method, that will classify > the input value to the switch and return the case number, to which the switch > then jumps. To support guards, the switches (and the bootstrap method) are > restartable. The bootstrap method as such is written very simply so far, but > could be much more optimized later. > -nullable type patterns are `case String s, null`/`case null, String s`/`case > null: case String s:`/`case String s: case null:`, handling of these required > a few tricks in `Attr`, `Flow` and `TransPatterns`. > > The specdiff for the change is here (to be updated): > http://cr.openjdk.java.net/~jlahoda/8265981/specdiff.preview.01/overview-summary.html Jan Lahoda has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits: - Post-merge fix - need to include jdk.internal.javac in the list of packages used by jdk.compiler again, as we now (again) have a preview feature in javac. - Correcting LineNumberTable for rule switches. - Merging master into JDK-8262891 - Fixing various error-related bugs. - Avoiding fall-through from the total case to a synthetic default but changing total patterns to default. - Reflecting recent spec changes. - Reflecting review comments. - Reflecting review comments on SwitchBootstraps. - Trailing whitespaces. - Cleanup, reflecting review comments. - ... and 2 more: https://git.openjdk.java.net/jdk/compare/083416d3...fd748501 - Changes: https://git.openjdk.java.net/jdk/pull/3863/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3863&range=04 Stats: 4551 lines in 77 files changed: 4235 ins; 119 del; 197 mod Patch: https://git.openjdk.java.net/jdk/pull/3863.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3863/head:pull/3863 PR: https://git.openjdk.java.net/jdk/pull/3863
Re: RFR: 8267751: (test) jtreg.SkippedException has no serial VersionUID [v2]
> The class `test/lib/jtreg/SkippedException.java` is missing a > serialVersionUID causing additional noise in compiler output of tests. Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Removed unintended classfile - Changes: - all: https://git.openjdk.java.net/jdk/pull/4197/files - new: https://git.openjdk.java.net/jdk/pull/4197/files/8c629b22..7aff0846 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4197&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4197&range=00-01 Stats: 0 lines in 1 file changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4197.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4197/head:pull/4197 PR: https://git.openjdk.java.net/jdk/pull/4197
Re: RFR: 8264859: Implement Context-Specific Deserialization Filters [v9]
On Tue, 25 May 2021 23:25:41 GMT, Brent Christian wrote:
>> Roger Riggs has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Editorial updates
>> Updated java.security properties to include jdk.serialFilterFactory
>> Added test cases to SerialFilterFactoryTest for java.security properties
>> and
>> enabling of the SecurityManager with existing policy permission files
>> Corrected a test that OIS.setObjectInputFilter could not be called twice.
>> Removed a Factory test that was not intended to be committed
>
> src/java.base/share/classes/java/io/ObjectInputFilter.java line 551:
>
>> 549: final class Config {
>> 550: /**
>> 551: * Lock object for filter and filter factory.
>
> The lock is not used for the filter factory, is it?
The lock was needed for the factory to make the check and setting atomic.
-
PR: https://git.openjdk.java.net/jdk/pull/3996
Re: RFR: 8264859: Implement Context-Specific Deserialization Filters [v9]
On Wed, 26 May 2021 06:35:46 GMT, Peter Levart wrote:
>> src/java.base/share/classes/java/io/ObjectInputStream.java line 1265:
>>
>>> 1263: * must return a non-null filter. It is not permitted to remove
>>> filtering once established.
>>> 1264: * See the {@linkplain ObjectInputFilter filter models} for
>>> examples of composition and delegation.
>>> 1265: *
>>
>> Hi Roger,
>> When I first read this javadoc, I was a little confused and had to peek into
>> the implementation. After that, I understood the above text, but without
>> peeking and in-depth knowledge, I couldn't. The confusing part is the
>> apparently conflicting claims made by 1st vs. 2nd paragraph. Both talk about
>> setting the deserialization filter - the 1st just says "set the
>> deserialization filter for the stream", and with the `setObjectInputFilter`
>> method having a sole `filter` parameter, together these establish a simple
>> picture - ah, just a setter method. But no, the 2nd paragraph talks about
>> something entirely different which doesn't fit into the established picture.
>> So would it be possible to rephrase that 1st paragraph somehow? Or what
>> about starting with 2nd paragraph: "Set the deserialization filter for the
>> stream to the filter returned by invoking " followed by 1st paragraph:
>> "The filter can be set and only set once before reading any objects..."
>
> Also a better wording for the following paragraph could be: "This method can
> only be called once and before reading any objects with this
> ObjectInputStream"
> Talking about "The filter can only be set once" is a little confusing, since
> the filter may actually already be set to JVM-wide filter when this methods
> is called to replace it with per-OIS filter.
Rewrote and included your suggestions.
-
PR: https://git.openjdk.java.net/jdk/pull/3996
Re: RFR: 8264859: Implement Context-Specific Deserialization Filters [v10]
> JEP 415: Context-specific Deserialization Filters extends the deserialization > filtering mechanisms with more flexible and customizable protections against > malicious deserialization. See JEP 415: https://openjdk.java.net/jeps/415. > The `java.io.ObjectInputFilter` and `java.io.ObjectInputStream` classes are > extended with additional > configuration mechanisms and filter utilities. > > javadoc for `ObjectInputFilter`, `ObjectInputFilter.Config`, and > `ObjectInputStream`: > > http://cr.openjdk.java.net/~rriggs/filter-factory/java.base/java/io/ObjectInputFilter.html Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Editorial updates to review comments. Simplify the builtin filter factory implementation. Add atomic update to setting the filter factory. Clarify the description of OIS.setObjectInputFilter. Cleanup of the example code. - Changes: - all: https://git.openjdk.java.net/jdk/pull/3996/files - new: https://git.openjdk.java.net/jdk/pull/3996/files/b83da61a..a65d6205 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3996&range=09 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3996&range=08-09 Stats: 102 lines in 3 files changed: 27 ins; 41 del; 34 mod Patch: https://git.openjdk.java.net/jdk/pull/3996.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3996/head:pull/3996 PR: https://git.openjdk.java.net/jdk/pull/3996
Re: RFR: 8264859: Implement Context-Specific Deserialization Filters [v8]
On Tue, 25 May 2021 21:53:26 GMT, Brent Christian wrote:
>> Roger Riggs has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Moved utility filter methods to be static on ObjectInputFilter
>>Rearranged the class javadoc of OIF to describe the parts of
>>deserialization filtering, filters, composite filters, and the filter
>> factory.
>>And other review comment updates...
>> - Refactored tests for utility functions to SerialFilterFunctionTest.java
>>Deleted confused Config.allowMaxLimits() method
>>Updated example to match move of methods to Config
>>Added test of restriction on setting the filterfactory after a OIS has
>> been created
>>Additional Editorial updates
>
> src/java.base/share/classes/java/io/ObjectInputFilter.java line 205:
>
>> 203: * // Returns a composite filter of the static JVM-wide filter, a
>> thread-specific filter,
>> 204: * // and the stream-specific filter.
>> 205: * public ObjectInputFilter apply(ObjectInputFilter curr,
>> ObjectInputFilter next) {
>
> `@Override` on `apply` ?
yes, but omitted to make the code more readable.
-
PR: https://git.openjdk.java.net/jdk/pull/3996
Integrated: 8263202: Update Hebrew/Indonesian/Yiddish ISO 639 language codes to current
On Mon, 17 May 2021 16:55:35 GMT, Naoto Sato wrote: > Please review the changes to the subject issue. java.util.Locale class has a > long-standing issue for those obsolete ISO 639 languages where its > normalization ends up in the obsolete codes. This change intends to flip the > normalization towards the current codes, providing a system property for > compatibility behavior. ResourceBundle class is also modified to load either > obsolete/current bundles. For more detail, take a look at the CSR. This pull request has now been integrated. Changeset: a4c46e1e Author:Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/a4c46e1e4f4f2f05c8002b2af683a390fc46b424 Stats: 386 lines in 35 files changed: 240 ins; 41 del; 105 mod 8263202: Update Hebrew/Indonesian/Yiddish ISO 639 language codes to current Reviewed-by: joehw - PR: https://git.openjdk.java.net/jdk/pull/4069
Re: RFR: 8265248: Implementation Specific Properties: change prefix, plus add existing properties [v7]
On Mon, 24 May 2021 06:23:59 GMT, Joe Wang wrote: >> Update module summary, add a few existing properties and features into the >> tables. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > Consolidated impl specific properties; deprecated legacy properties; made > new ones take preference Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3644
Re: RFR: 8267751: (test) jtreg.SkippedException has no serial VersionUID
On Wed, 26 May 2021 01:28:17 GMT, Roger Riggs wrote: > The class `test/lib/jtreg/SkippedException.java` is missing a > serialVersionUID causing additional noise in compiler output of tests. Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4197
RFR: 8267751: (test) jtreg.SkippedException has no serial VersionUID
The class `test/lib/jtreg/SkippedException.java` is missing a serialVersionUID causing additional noise in compiler output of tests. - Commit messages: - Added serialVersionUID to jtreg.SkippedException to eliminate compilation warnings Changes: https://git.openjdk.java.net/jdk/pull/4197/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4197&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8267751 Stats: 3 lines in 2 files changed: 2 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4197.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4197/head:pull/4197 PR: https://git.openjdk.java.net/jdk/pull/4197
Integrated: 8267583: jmod fails on symlink to class file
On Wed, 26 May 2021 07:55:53 GMT, Athijegannathan Sundararajan wrote: > FileVisitOption.FOLLOW_LINKS used. Test extended for file symlinks case. This pull request has now been integrated. Changeset: bf8d4a8e Author:Athijegannathan Sundararajan URL: https://git.openjdk.java.net/jdk/commit/bf8d4a8ecab216e7d117ce045d4498d1fa1a6029 Stats: 41 lines in 2 files changed: 39 ins; 0 del; 2 mod 8267583: jmod fails on symlink to class file Reviewed-by: alanb - PR: https://git.openjdk.java.net/jdk/pull/4202
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v7]
> Hi, > > Could someone please review my code for updating the code in the `java.io`, > `java.math`, and `java.text` packages to make use of the switch expressions? > > Kind regards, > Patrick Patrick Concannon 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 eight additional commits since the last revision: - Merge remote-tracking branch 'origin/master' into JDK-8267670 - 8267670: Added missing brace - Merge remote-tracking branch 'origin/master' into JDK-8267670 - 8267670: Removed trailing whitespace - 8267670: Remove redundant code from switch - 8267670: Updated code to use yield - Merge remote-tracking branch 'origin/master' into JDK-8267670 - 8267670: Update java.io, java.math, and java.text to use switch expressions - Changes: - all: https://git.openjdk.java.net/jdk/pull/4182/files - new: https://git.openjdk.java.net/jdk/pull/4182/files/a0dfbeba..57184fbf Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4182&range=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4182&range=05-06 Stats: 29 lines in 1 file changed: 17 ins; 5 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/4182.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4182/head:pull/4182 PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v6]
> Hi, > > Could someone please review my code for updating the code in the `java.io`, > `java.math`, and `java.text` packages to make use of the switch expressions? > > Kind regards, > Patrick Patrick Concannon has updated the pull request incrementally with one additional commit since the last revision: 8267670: Added missing brace - Changes: - all: https://git.openjdk.java.net/jdk/pull/4182/files - new: https://git.openjdk.java.net/jdk/pull/4182/files/ad7aeacd..a0dfbeba Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4182&range=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4182&range=04-05 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4182.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4182/head:pull/4182 PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267583: jmod fails on symlink to class file
On Wed, 26 May 2021 07:55:53 GMT, Athijegannathan Sundararajan wrote: > FileVisitOption.FOLLOW_LINKS used. Test extended for file symlinks case. It strikes me a bit unusual to be following sym links here but okay. - Marked as reviewed by alanb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4202
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v5]
On Wed, 26 May 2021 09:39:44 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.io`, >> `java.math`, and `java.text` packages to make use of the switch expressions? >> >> Kind regards, >> Patrick > > Patrick Concannon 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 six additional > commits since the last revision: > > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - 8267670: Removed trailing whitespace > - 8267670: Remove redundant code from switch > - 8267670: Updated code to use yield > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - 8267670: Update java.io, java.math, and java.text to use switch expressions Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v5]
> Hi, > > Could someone please review my code for updating the code in the `java.io`, > `java.math`, and `java.text` packages to make use of the switch expressions? > > Kind regards, > Patrick Patrick Concannon 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 six additional commits since the last revision: - Merge remote-tracking branch 'origin/master' into JDK-8267670 - 8267670: Removed trailing whitespace - 8267670: Remove redundant code from switch - 8267670: Updated code to use yield - Merge remote-tracking branch 'origin/master' into JDK-8267670 - 8267670: Update java.io, java.math, and java.text to use switch expressions - Changes: - all: https://git.openjdk.java.net/jdk/pull/4182/files - new: https://git.openjdk.java.net/jdk/pull/4182/files/d4a6cc44..ad7aeacd Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4182&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4182&range=03-04 Stats: 1031 lines in 36 files changed: 717 ins; 101 del; 213 mod Patch: https://git.openjdk.java.net/jdk/pull/4182.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4182/head:pull/4182 PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v4]
On Wed, 26 May 2021 09:05:34 GMT, Patrick Concannon
wrote:
>> Hi,
>>
>> Could someone please review my code for updating the code in the `java.io`,
>> `java.math`, and `java.text` packages to make use of the switch expressions?
>>
>> Kind regards,
>> Patrick
>
> Patrick Concannon has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8267670: Removed trailing whitespace
> _Mailing list message from [Brian Goetz](mailto:brian.go...@oracle.com) on
> [core-libs-dev](mailto:core-libs-...@mail.openjdk.java.net):_
>
> In the last hunk, you convert
>
> case Collator.IDENTICAL: toAddTo.append('='); break;
> case Collator.TERTIARY: toAddTo.append(','); break;
> case Collator.SECONDARY: toAddTo.append(';'); break;
> case Collator.PRIMARY: toAddTo.append('<'); break;
> case RESET: toAddTo.append('&'); break;
> case UNSET: toAddTo.append('?'); break;
>
> to
>
> case Collator.IDENTICAL -> toAddTo.append('=');
> case Collator.TERTIARY -> toAddTo.append(',');
> case Collator.SECONDARY -> toAddTo.append(';');
> case Collator.PRIMARY -> toAddTo.append('<');
> case RESET -> toAddTo.append('&');
> case UNSET -> toAddTo.append('?');
>
> But, you can go further, pulling the toAddTo.append() call out of the switch.
> This was one of the benefits we anticipated with expression switches; that it
> would expose more opportunities to push the conditional logic farther down
> towards the leaves. I suspect there are other opportunities for this in this
> patch too.
Hi Brian,
Thanks for your suggestion. I've incorporated it into the PR, and you can find
it in commit e503ed2
-
PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v4]
> Hi, > > Could someone please review my code for updating the code in the `java.io`, > `java.math`, and `java.text` packages to make use of the switch expressions? > > Kind regards, > Patrick Patrick Concannon has updated the pull request incrementally with one additional commit since the last revision: 8267670: Removed trailing whitespace - Changes: - all: https://git.openjdk.java.net/jdk/pull/4182/files - new: https://git.openjdk.java.net/jdk/pull/4182/files/e503ed26..d4a6cc44 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4182&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4182&range=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4182.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4182/head:pull/4182 PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v2]
On Tue, 25 May 2021 15:18:46 GMT, Chris Hegarty wrote:
>> Patrick Concannon 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:
>>
>> - 8267670: Updated code to use yield
>> - Merge remote-tracking branch 'origin/master' into JDK-8267670
>> - 8267670: Update java.io, java.math, and java.text to use switch
>> expressions
>
> src/java.base/share/classes/java/io/ObjectStreamClass.java line 2172:
>
>> 2170: switch (typeCodes[i]) {
>> 2171: case 'L', '[' -> vals[offsets[i]] =
>> unsafe.getReference(obj, readKeys[i]);
>> 2172: default -> throw new InternalError();
>
> suggest:
>
> vals[offsets[i]] = switch (typeCodes[i]) {
> case 'L', '[' -> unsafe.getReference(obj, readKeys[i]);
> default -> throw new InternalError();
Comment addressed in e503ed2
> src/java.base/share/classes/java/io/StreamTokenizer.java line 787:
>
>> 785: case TT_WORD-> ret = sval;
>> 786: case TT_NUMBER -> ret = "n=" + nval;
>> 787: case TT_NOTHING -> ret = "NOTHING";
>
> This is not correct, the assignments to ret in these case arms is redundant.
Comment addressed in e503ed2
-
PR: https://git.openjdk.java.net/jdk/pull/4182
RFR: 8267583: jmod fails on symlink to class file
FileVisitOption.FOLLOW_LINKS used. Test extended for file symlinks case. - Commit messages: - 8267583: jmod fails on symlink to class file Changes: https://git.openjdk.java.net/jdk/pull/4202/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4202&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8267583 Stats: 41 lines in 2 files changed: 39 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/4202.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4202/head:pull/4202 PR: https://git.openjdk.java.net/jdk/pull/4202
Re: RFR: 8266310: deadlock while loading the JNI code [v5]
On Fri, 21 May 2021 15:49:09 GMT, Aleksei Voitylov
wrote:
>> Please review this PR which fixes the deadlock in ClassLoader between the
>> two lock objects - a lock object associated with the class being loaded, and
>> the ClassLoader.loadedLibraryNames hash map, locked during the native
>> library load operation.
>>
>> Problem being fixed:
>>
>> The initial reproducer demonstrated a deadlock between the JarFile/ZipFile
>> and the hash map. That deadlock exists even when the ZipFile/JarFile lock is
>> removed because there's another lock object in the class loader, associated
>> with the name of the class being loaded. Such objects are stored in
>> ClassLoader.parallelLockMap. The deadlock occurs when JNI_OnLoad() loads
>> exactly the same class, whose signature is being verified in another thread.
>>
>> Proposed fix:
>>
>> The proposed patch suggests to get rid of locking loadedLibraryNames hash
>> map and synchronize on each entry name, as it's done with class names in see
>> ClassLoader.getClassLoadingLock(name) method.
>>
>> The patch introduces nativeLibraryLockMap which holds the lock objects for
>> each library name, and the getNativeLibraryLock() private method is used to
>> lazily initialize the corresponding lock object. nativeLibraryContext was
>> changed to ThreadLocal, so that in any concurrent thread it would have a
>> NativeLibrary object on top of the stack, that's being currently
>> loaded/unloaded in that thread. nativeLibraryLockMap accumulates the names
>> of all native libraries loaded - in line with class loading code, it is not
>> explicitly cleared.
>>
>> Testing: jtreg and jck testing with no regressions. A new regression test
>> was developed.
>
> Aleksei Voitylov has updated the pull request incrementally with one
> additional commit since the last revision:
>
> fix whitespace
Changes requested by plevart (Reviewer).
src/java.base/share/classes/jdk/internal/loader/NativeLibraries.java line 481:
> 479: throw new Error("Maximum lock count exceeded");
> 480: }
> 481:
Hi Aleksei,
I know in practice this counter will never overflow, but just to be pedantic,
it would be better that you either decrement back the counter before throwing
Error or you check for overflow without incrementing it. Otherwise the lock is
left in a corrupted state since you use it like this:
acquireNativeLibraryLock(name);
try {
...
} finally {
releaseNativeLibraryLock(name);
}
...you see, if acquire fails, it is not paired with release, which is correct
since the ReentrantLock has not been acquired, but the counter *HAS* been
incremented...
-
PR: https://git.openjdk.java.net/jdk/pull/3976
