Re: RFR: 8255878: FilterInputStream is missing implementations of Java 9 InputStream methods [v2]

[Alan Bateman]

On Sat, 4 Sep 2021 00:39:19 GMT, Daniel Fuchs  wrote:

> This may potentially break any existing subclasses that assume that they only 
> need to override the three
> `int read()` and `int read(byte[], int, it)` and `int skip(int)` methods - 
> because overriding only these three methods will now no longer be sufficient.

Can you explain this a bit further? InputStream has one abstract method so 
that's the minimum that has to be implemented. It has base implementations of 
readAllBytes and readNBytes that would be very inefficient if the 3-arg read is 
not overridden but I wouldn't expect a correctness issue.

-

PR: https://git.openjdk.java.net/jdk/pull/5367

Re: RFR: JDK-8273246 Amend the test java/nio/channels/DatagramChannel/ManySourcesAndTargets.java to execute in othervm mode

[Mark Sheppard]

On Sat, 4 Sep 2021 06:47:43 GMT, Alan Bateman  wrote:

>> A number of nio DatagramChannel tests are intermittently failing on 
>> macosx-aarch64.
>> In some instances this is a receive call blocking indefinitely waiting on 
>> data which has
>> already been sent, and should be available immediately to the receive method 
>> call.
>> Other test failure scenarios are problems during the test compilation phase 
>> with a SocketException being thrown and the message:
>> "test result: Error. Agent communication error: java.net.SocketException: No 
>> buffer space available; check console log for any additional details"
>> 
>> The ManySourcesAndTargets and other tests execute in agentvm mode. This 
>> results in certain test diagnostic
>> Output being lost during the test failure handling capture process. To 
>> mitigate this lost diagnostics, the
>> ManySourcesAndTargets test has been amended to execute in othervm mode.
>> 
>> Additionally, to assist in the buffer allocation issue, the netstat command 
>> executed by the test
>> failure_handler has an extra argument added to obtain additional details on 
>> mbuf usage.
>> The failure handler will now execute with netstat -mm
>
> test/jdk/java/nio/channels/DatagramChannel/ManySourcesAndTargets.java line 28:
> 
>> 26:  * @summary Test DatagramChannel send/receive and that receive returns 
>> the expected
>> 27:  * sender address
>> 28:  * @run main/othervm ManySourcesAndTargets
> 
> This change looks okay. Do you mind change L143 to use sender.send rather 
> than reader.send while you are there? That will avoid at least some questions 
> while trying to track down the underlying issue and will ensure that the test 
> is printing out the actual sender address.

yes, I can do that ... the suggested change is currently part of a set of 
changes for JDK-8264385, but no problem to add it here now for this change set

-

PR: https://git.openjdk.java.net/jdk/pull/5366

Re: RFR: 6957241: ClassLoader.getResources() returns only 1 instance when using jar indexing

[Lance Andersen]

On Sep 4, 2021, at 2:53 AM, Alan Bateman 
mailto:al...@openjdk.java.net>> wrote:

On Sat, 4 Sep 2021 01:53:35 GMT, wxiang 
mailto:github.com+53162078+shiyu...@openjdk.org>>
 wrote:

I will first create the patch to remove JAR index support from the 
URLClassLoader implementation, the `jar i` option.

Thank you. We'll probably need a new JBS issue and PR for that but let's see 
first if any issues come out of the wood work. Stuart is right that another 
option for the jar tool is to drop the index when updating an existing JAR file 
that has an index, we don't need to decide that just yet.

Perhaps the jar validate option could/should be updated to flag when an index 
is there?

We could add a warning for JDK 18 when main::genIndex is invoked as I assume we 
would want to at least wait until JDK19 to remove this functionality?

Best
Lance



-

PR: https://git.openjdk.java.net/jdk/pull/5316

[cid:E1C4E2F0-ECD0-4C9D-ADB4-B16CA7BCB7FC@home]



Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com

RFR: JDK-8272192 Shortcut String equality checks by checking equality of the value array

[q2q-2q2]

Shortcut String equality checks by checking equality of the value array

-

Commit messages:
 - JDK-8272192

Changes: https://git.openjdk.java.net/jdk/pull/5370/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5370&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8272192
  Stats: 62 lines in 3 files changed: 60 ins; 1 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5370.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5370/head:pull/5370

PR: https://git.openjdk.java.net/jdk/pull/5370

Re: RFR: JDK-8272192 Shortcut String equality checks by checking equality of the value array

[liach]

On Sat, 4 Sep 2021 11:59:58 GMT, q2q-2q2 
 wrote:

> Shortcut String equality checks by checking equality of the value array

src/java.base/share/classes/java/lang/String.java line 1964:

> 1962: public boolean equalsIgnoreCase(String anotherString) {
> 1963: if (anotherString != null) {
> 1964: return false;

This one is definitely wrong.

-

PR: https://git.openjdk.java.net/jdk/pull/5370

Re: Proposal: JDK-8231640 - (prop) Canonical property storage

[Jaikiran Pai]

On 29/08/21 6:41 pm, Jaikiran Pai wrote:


On 28/08/21 8:46 pm, Alan Bateman wrote:

On 28/08/2021 05:45, Jaikiran Pai wrote:
I hadn't considered the system property approach to switch to old 
behaviour in my proposals, so this is a very good input and I 
personally think the most logical proposals so far.


Roger may be right that few would care but it would be changing 
behavior that goes back to JDK 1.0. In your list (and thanks for 
writing down the options with pros/cons) then your 1a where 
SOURCE_DATE_EPOCH changes to write the epoch date rather than the 
current date seems to be most consistent with other tools and the 
wider eco system. It seems the least disruptive in that it doesn't 
change existing behavior and would be opt-in when reproducibility is 
required.


In that case, I will start work along these lines.


I've now opened a PR https://github.com/openjdk/jdk/pull/5372 with these 
proposed changes.


-Jaikiran

Re: RFR: 8231640: (prop) Canonical property storage

[Jaikiran Pai]

On Sat, 4 Sep 2021 15:25:59 GMT, Jaikiran Pai  wrote:

> The commit in this PR implements the proposal for enhancement that was 
> discussed in the core-libs-dev mailing list recently[1], for 
> https://bugs.openjdk.java.net/browse/JDK-8231640
> 
> At a high level - the `store()` APIs in `Properties` have been modified to 
> now look for the `SOURCE_DATE_EPOCH` environment variable[2]. If that env 
> variable is set, then instead of writing out the current date time as a date 
> comment, the `store()` APIs instead will use the value set for this env 
> variable to parse it to a `Date` and write out the string form of such a 
> date. The implementation here uses the `d MMM  HH:mm:ss 'GMT'` date 
> format and `Locale.ROOT` to format and write out such a date. This should 
> provide reproducibility whenever the `SOURCE_DATE_EPOCH` is set. Furthermore, 
> intentionally, no changes in the date format of the "current date" have been 
> done.
> 
> These  modified `store()` APIs work in the presence of the `SecurityManager` 
> too. The caller is expected to have a read permission on the 
> `SOURCE_DATE_EPOCH` environment variable. If the caller doesn't have that 
> permission, then the implementation of these `store()` APIs will write out 
> the "current date" and will ignore any value that has been set for the 
> `SOURCE_DATE_EPOCH` env variable. This should allow for backward 
> compatibility of existing applications, where, when they run under a 
> `SecurityManager` and perhaps with an existing restrictive policy file, the 
> presence of `SOURCE_DATE_EPOCH` shouldn't impact their calls to the `store()` 
> APIs.
> 
> The modified `store()` APIs will also ignore any value for 
> `SOURCE_DATE_EPOCH` that  cannot be parsed to an `long` value. In such cases, 
> the `store()` APIs will write out the "current date" and ignore the value set 
> for this environment variable. No exceptions will be thrown for such invalid 
> values. This is an additional backward compatibility precaution to prevent 
> any rogue value for `SOURCE_DATE_EPOCH` from breaking applications.
> 
> An additional change in the implementation of these `store()` APIs and 
> unrelated to the date comment, is that these APIs will now write out the 
> property keys in a deterministic order. The keys will be written out in the 
> natural ordering as specified by `java.lang.String#compareTo()` API.
> 
> The combination of the ordering of the property keys when written out and the 
> usage of `SOURCE_DATE_EPOCH` environment value to determine the date comment 
> should together allow for reproducibility of the output generated by these 
> `store()` APIs.
> 
> New jtreg test classes have been introduced to verify these changes. The 
> primary focus of `PropertiesStoreTest` is the ordering aspects of the 
> property keys that are written out. On the other hand 
> `StoreReproducibilityTest` focuses on the reproducibility of the output 
> generated by these APIs.  The `StoreReproducibilityTest` runs these tests 
> both in the presence and absence of `SecurityManager`. Plus, in the presence 
> of SecurityManager, it tests both the scenarios where the caller is granted 
> the requisite permission and in other case not granted that permission.
> 
> These new tests and existing tests under `test/jdk/java/util/Properties/` 
> pass with these changes.
> 
> [1] 
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-August/080758.html
> [2] https://reproducible-builds.org/specs/source-date-epoch/

I haven't yet created a CSR for this and will do that next week once the 
initial reviews and changes are sorted out.

-

PR: https://git.openjdk.java.net/jdk/pull/5372

RFR: 8231640: (prop) Canonical property storage

[Jaikiran Pai]

The commit in this PR implements the proposal for enhancement that was 
discussed in the core-libs-dev mailing list recently[1], for 
https://bugs.openjdk.java.net/browse/JDK-8231640

At a high level - the `store()` APIs in `Properties` have been modified to now 
look for the `SOURCE_DATE_EPOCH` environment variable[2]. If that env variable 
is set, then instead of writing out the current date time as a date comment, 
the `store()` APIs instead will use the value set for this env variable to 
parse it to a `Date` and write out the string form of such a date. The 
implementation here uses the `d MMM  HH:mm:ss 'GMT'` date format and 
`Locale.ROOT` to format and write out such a date. This should provide 
reproducibility whenever the `SOURCE_DATE_EPOCH` is set. Furthermore, 
intentionally, no changes in the date format of the "current date" have been 
done.

These  modified `store()` APIs work in the presence of the `SecurityManager` 
too. The caller is expected to have a read permission on the 
`SOURCE_DATE_EPOCH` environment variable. If the caller doesn't have that 
permission, then the implementation of these `store()` APIs will write out the 
"current date" and will ignore any value that has been set for the 
`SOURCE_DATE_EPOCH` env variable. This should allow for backward compatibility 
of existing applications, where, when they run under a `SecurityManager` and 
perhaps with an existing restrictive policy file, the presence of 
`SOURCE_DATE_EPOCH` shouldn't impact their calls to the `store()` APIs.

The modified `store()` APIs will also ignore any value for `SOURCE_DATE_EPOCH` 
that  cannot be parsed to an `long` value. In such cases, the `store()` APIs 
will write out the "current date" and ignore the value set for this environment 
variable. No exceptions will be thrown for such invalid values. This is an 
additional backward compatibility precaution to prevent any rogue value for 
`SOURCE_DATE_EPOCH` from breaking applications.

An additional change in the implementation of these `store()` APIs and 
unrelated to the date comment, is that these APIs will now write out the 
property keys in a deterministic order. The keys will be written out in the 
natural ordering as specified by `java.lang.String#compareTo()` API.

The combination of the ordering of the property keys when written out and the 
usage of `SOURCE_DATE_EPOCH` environment value to determine the date comment 
should together allow for reproducibility of the output generated by these 
`store()` APIs.

New jtreg test classes have been introduced to verify these changes. The 
primary focus of `PropertiesStoreTest` is the ordering aspects of the property 
keys that are written out. On the other hand `StoreReproducibilityTest` focuses 
on the reproducibility of the output generated by these APIs.  The 
`StoreReproducibilityTest` runs these tests both in the presence and absence of 
`SecurityManager`. Plus, in the presence of SecurityManager, it tests both the 
scenarios where the caller is granted the requisite permission and in other 
case not granted that permission.

These new tests and existing tests under `test/jdk/java/util/Properties/` pass 
with these changes.

[1] 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-August/080758.html
[2] https://reproducible-builds.org/specs/source-date-epoch/

-

Commit messages:
 - 8231640: (prop) Canonical property storage

Changes: https://git.openjdk.java.net/jdk/pull/5372/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5372&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8231640
  Stats: 667 lines in 5 files changed: 663 ins; 1 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5372.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5372/head:pull/5372

PR: https://git.openjdk.java.net/jdk/pull/5372

Re: RFR: 8231640: (prop) Canonical property storage

[Andrey Turbanov]

On Sat, 4 Sep 2021 15:25:59 GMT, Jaikiran Pai  wrote:

> The commit in this PR implements the proposal for enhancement that was 
> discussed in the core-libs-dev mailing list recently[1], for 
> https://bugs.openjdk.java.net/browse/JDK-8231640
> 
> At a high level - the `store()` APIs in `Properties` have been modified to 
> now look for the `SOURCE_DATE_EPOCH` environment variable[2]. If that env 
> variable is set, then instead of writing out the current date time as a date 
> comment, the `store()` APIs instead will use the value set for this env 
> variable to parse it to a `Date` and write out the string form of such a 
> date. The implementation here uses the `d MMM  HH:mm:ss 'GMT'` date 
> format and `Locale.ROOT` to format and write out such a date. This should 
> provide reproducibility whenever the `SOURCE_DATE_EPOCH` is set. Furthermore, 
> intentionally, no changes in the date format of the "current date" have been 
> done.
> 
> These  modified `store()` APIs work in the presence of the `SecurityManager` 
> too. The caller is expected to have a read permission on the 
> `SOURCE_DATE_EPOCH` environment variable. If the caller doesn't have that 
> permission, then the implementation of these `store()` APIs will write out 
> the "current date" and will ignore any value that has been set for the 
> `SOURCE_DATE_EPOCH` env variable. This should allow for backward 
> compatibility of existing applications, where, when they run under a 
> `SecurityManager` and perhaps with an existing restrictive policy file, the 
> presence of `SOURCE_DATE_EPOCH` shouldn't impact their calls to the `store()` 
> APIs.
> 
> The modified `store()` APIs will also ignore any value for 
> `SOURCE_DATE_EPOCH` that  cannot be parsed to an `long` value. In such cases, 
> the `store()` APIs will write out the "current date" and ignore the value set 
> for this environment variable. No exceptions will be thrown for such invalid 
> values. This is an additional backward compatibility precaution to prevent 
> any rogue value for `SOURCE_DATE_EPOCH` from breaking applications.
> 
> An additional change in the implementation of these `store()` APIs and 
> unrelated to the date comment, is that these APIs will now write out the 
> property keys in a deterministic order. The keys will be written out in the 
> natural ordering as specified by `java.lang.String#compareTo()` API.
> 
> The combination of the ordering of the property keys when written out and the 
> usage of `SOURCE_DATE_EPOCH` environment value to determine the date comment 
> should together allow for reproducibility of the output generated by these 
> `store()` APIs.
> 
> New jtreg test classes have been introduced to verify these changes. The 
> primary focus of `PropertiesStoreTest` is the ordering aspects of the 
> property keys that are written out. On the other hand 
> `StoreReproducibilityTest` focuses on the reproducibility of the output 
> generated by these APIs.  The `StoreReproducibilityTest` runs these tests 
> both in the presence and absence of `SecurityManager`. Plus, in the presence 
> of SecurityManager, it tests both the scenarios where the caller is granted 
> the requisite permission and in other case not granted that permission.
> 
> These new tests and existing tests under `test/jdk/java/util/Properties/` 
> pass with these changes.
> 
> [1] 
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-August/080758.html
> [2] https://reproducible-builds.org/specs/source-date-epoch/

src/java.base/share/classes/java/util/Properties.java line 924:

> 922: writeDateComment(bw);
> 923: synchronized (this) {
> 924: for (Map.Entry e : new 
> TreeMap<>(map).entrySet()) {

Is this sorting intentionally added? It's not clear from issue description or 
PR description that order of properties should be changed too.
Anyway I think copying to array and then sorting should be faster, that 
creating TreeMap

-

PR: https://git.openjdk.java.net/jdk/pull/5372

Re: RFR: 8273329: Remove redundant null check from String.getBytes(String charsetName)

[Сергей Цыпанов]

On Sat, 4 Sep 2021 01:43:57 GMT, Naoto Sato  wrote:

>> Current implementation looks like this:
>> 
>> public byte[] getBytes(String charsetName)
>> throws UnsupportedEncodingException {
>> if (charsetName == null) throw new NullPointerException();
>> return encode(lookupCharset(charsetName), coder(), value);
>> }
>> 
>> Null check seems to be redundant here because the same check of 
>> `charsetName` is done within `String.lookupCharset(String)`:
>> 
>> private static Charset lookupCharset(String csn) throws 
>> UnsupportedEncodingException {
>> Objects.requireNonNull(csn);
>> try {
>> return Charset.forName(csn);
>> } catch (UnsupportedCharsetException | IllegalCharsetNameException x) {
>> throw new UnsupportedEncodingException(csn);
>> }
>> }
>
> Looks good. Please add some `noreg` keyword to the JIRA issue.

@naotoj done, added `noreg-cleanup` tag

-

PR: https://git.openjdk.java.net/jdk/pull/5361