JDK 14 RFR of JDK-8231442 : Suppress warnings on non-serializable instance fields in java.sql.* modules

[Joe Darcy]

Hello,

Next step in the serialization warnings pre-cleanup is java.sql.* 
modules. Please review the proposed changes:


    JDK-8231442 : Suppress warnings on non-serializable instance fields 
in java.sql.* modules

    http://cr.openjdk.java.net/~darcy/8231442.0/

Several of the writeObject methods do runtime checks on whether on not 
objects pointed to by the fields in question are serializable. The 
fields with this runtime check get a explanatory comment noting that 
different from the usual comment.


Thanks,

-Joe

RandomAccess Interface and List Heirarchy

[Cyrus Vafadari]

Hello all,

*TLDR: Why doesn't RandomAccess interface extend List?*

I'm maintaining a framework that lets developers build plugins, and
developers implement a `put(List thingList)` in their plugins.
However, I want to guarantee to the implementer that their List will
support RandomAccess. I see Java does support a syntax for declaring that
the argument should implement both List and RandomAccess using the "&"
operator, and I could declare my own interface that extends both, but I am
surprised that RandomAccess itself does not extend List and appear in the
List hierarchy. According to the docs, it only applies to List
implementations anyway, and we use `instanceof` extensively in the code to
bifurcate how we handle both cases. It seems more natural to me for
RandomAccess to extend List, so that I can let my implementer know
explicitly they can rely on RandomAccess.

I am happy to give more details on my scenario, and very excited to learn
more why this decision was made, and if there is an aspect I am missing or
if improvements could be made!

Best wishes,

Cyrus

Build errors - JDK-8200758-branch

[Scott Hill]

Build fails on my CI Window Server 2008 & 2012 agents.
I am only able to get it to build on a Windows 10 system.

[12:49:04][Step 1/1] ERROR: Build failed for target 'jdk' in configuration 
'windows-x86-server-release' (exit code 2)
[12:49:05][Step 1/1]
[12:49:05][Step 1/1] === Output from failing command(s) repeated here ===
[12:49:05][Step 1/1] * For target 
support_native_jdk.jpackage_libjpackage_WindowsRegistry.obj:
[12:49:05][Step 1/1] WindowsRegistry.cpp
[12:49:05][Step 1/1] c:\progra~2\micros~2.0\vc\include\tchar.h(24) : fatal 
error C1189: #error : Need to include strsafe.h after tchar.h
[12:49:05][Step 1/1] ... (rest of output omitted)
[12:49:05][Step 1/1]
[12:49:05][Step 1/1] * All command lines available in 
/cygdrive/e/BuildAgent/work/ccdca5c47124cb38/build/windows-x86-server-release/make-support/failure-logs.
[12:49:05][Step 1/1] === End of repeated output ===



Build script:

make dist-clean bash configure --with-target-bits=32 
--disable-warnings-as-errors make jdk

Any help would be appreciated.

Thanks,

Scott Hill | Software Build Manager
Turning Technologies | 14557 N. 82nd Street Scottsdale, AZ 85260
Direct: 480-443-2249 | Main: 330-746-3015 | Fax: 330-884-6065
sh...@turningtechnologies.com | 
TurningTechnologies.com

This email message, including any attached files, is for the sole use of the 
intended recipients(s) and may contain confidential and/or privileged 
information. Any unauthorized review, use, disclosure or distribution is 
prohibited. To manage your email preferences or to opt-out of marketing 
communications completely, click 
here.

Re: RFR: JDK-8230920 : jpackage problems when -input dir contains any files with "cfg" extension.

[Alexander Matveev]

Hi Andy,

Looks good.

Do you think it would be better to write CLI arguments in some internal 
format instead of just raw dump? It might be better in case if we plan 
to change CLI between versions and at same time supporting generation of 
installers for app image using different jpackage versions.


Thanks,
Alexander

On 9/24/2019 5:54 AM, Andy Herrick wrote:

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This fix replaces the practice we were using on windows to determine 
the application name, and name of additional launchers (by looking for 
".cfg" files in the app dir.


Instead we now add a file ".jpackage.args" to the root of the 
app-image, and record in that file all the arguments used to create 
the app-image.  We later read that file to determine the original app 
name and any additional launcher names.


This change also fixes the shortcut creation on windows to create 
shortcuts (if so directed) for all launchers.


[1] https://bugs.openjdk.java.net/browse/JDK-8230920

[2] http://cr.openjdk.java.net/~herrick/8230920/webrev.01/

/Andy

Re: RFR: JDK-8230927 : Wrong arguments set for additional launchers

[Alexander Matveev]

Looks good.

On 9/24/2019 11:08 AM, Alexey Semenyuk wrote:

Looks good.

- Alexey

On 9/24/2019 1:24 PM, Andy Herrick wrote:
Revision 2 or the webrev ( [3] )  removes the comment about 
`--linux-deb-copyright-file` option., replaces the literal strings 
with the CLIOptions id's, and adds unit test to AddLauncherTest.java


[3] http://cr.openjdk.java.net/~herrick/8230927/webrev.02/

/Andy

On 9/24/2019 9:19 AM, Alexey Semenyuk wrote:

Andy,

Please remove javadoc update about `--linux-deb-copyright-file` 
option. It will be dropped in 
https://bugs.openjdk.java.net/browse/JDK-8231277 patch.

Line 180:
---
if (additional.containsKey("java-optiions")) {
---
Looks like a typo. Should be "java-options", not "java-optiions", I 
guess.
We probably need a test or at least a follow up CR to track the task 
of adding a test for "java-options" parameter in additional launchers.


- Alexey

On 9/24/2019 8:46 AM, Andy Herrick wrote:

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This change applies when arguments or java-options are used in an 
add-launcher properties file.


In these cases the arguments or java-options from the properties 
files should replace instead of being added to the same options 
used in the command line  for the main launcher.


[1] https://bugs.openjdk.java.net/browse/JDK-8230927

[2] http://cr.openjdk.java.net/~herrick/8230927

/Andy

Re: RFR: jsr166 integration 2019-09

[Martin Buchholz]

Frederic, could you figure out how to resolve

8231031: runtime/ReservedStack/ReservedStackTest.java fails after jsr166
refresh
https://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/ReservedStackTest/index.html
https://bugs.openjdk.java.net/browse/JDK-8231031

On Mon, Sep 23, 2019 at 11:08 PM David Holmes 
wrote:

> Except when I run it through our test system ReservedStackTest is still
> failing :(
>
> I tested it initially when Fred proposed it and that went fine. It also
> passes for me locally on Linux.
>
> David
>
> On 24/09/2019 12:20 pm, David Holmes wrote:
> > Hi Martin,
> >
> > That all seems fine to me.
> >
> > Thanks,
> > David
> >
> > On 24/09/2019 3:43 am, Martin Buchholz wrote:
> >> We now have a fix-up integration that removes all the previously
> >> excluded tests from their exclude lists.
> >>
> >>
> https://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/overview.html
> >>
> >>
> >> 8231031: runtime/ReservedStack/ReservedStackTest.java fails after
> >> jsr166 refresh
> >>
> https://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/ReservedStackTest/index.html
> >>
> >> https://bugs.openjdk.java.net/browse/JDK-8231031
> >>
> >> LockInfo objects now restored to their previous values (although David
> >> was looking at future improvements).
> >>
> >> 8231032: ThreadMXBean locking tests fail after JSR 166 refresh
> >>
> https://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/wrong-blocker/index.html
> >>
> >> https://bugs.openjdk.java.net/browse/JDK-8231032
> >>
> >> 8231036: vmTestbase monitoring tests fail after JSR 166 refresh
> >>
> https://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/SynchronizerLockingThreads/index.html
> >>
> >> https://bugs.openjdk.java.net/browse/JDK-8231036
>

Re: RFR(s): 8228580: DnsClient TCP socket timeout

[Pavel Rappo]

Milan,

I satisfied myself by running the final version of the test some 8k times and 
then pushed the change. Thanks for your patience and persistence.
I saw your question on the net-dev and nio-dev mailing lists. Thanks.

-Pavel

> On 24 Sep 2019, at 13:41, Milan Mimica  wrote:
> 
> Pavel,
> 
> Deal. Handling early returns too:
> http://cr.openjdk.java.net/~mmimica/8228580/webrev.05/
> I will ask there about socket timeout semantics.
> 
> On Tue, 24 Sep 2019 at 12:51, Pavel Rappo  wrote:
>> 
>> Milan,
>> 
>> Thanks for looking into this. I think you should ask a question on the 
>> expected timing semantics and guarantees on net-dev (with maybe a cc to 
>> nio-dev).
>> As for our test. I agree with you that we should simply work a possibility 
>> of early returns into the check.
>> 
>> ...
>> 
>> /* The acceptable variation of early returns from timed socket operations. */
>> private static final long PREMATURE_RETURN = adjustTimeout(100);
>> 
>> ...
>> 
>> long elapsed = NANOSECONDS.toMillis(System.nanoTime() - startNanos);
>> if (elapsed < timeout - PREMATURE_RETURN || elapsed > timeout + TOLERANCE) {
>>String msg = String.format(
>>"elapsed=%s, timeout=%s, TOLERANCE=%s, PREMATURE_RETURN=%s",
>>timeout, timeout, TOLERANCE, PREMATURE_RETURN);
>>throw new RuntimeException(msg);
>> }
>> 
>> ...
>> 
>> Thoughts?
>> 
>> -Pavel
>> 
>>> On 24 Sep 2019, at 09:12, Milan Mimica  wrote:
>>> 
>>> Hi Pavel
>>> 
>>> Wow, I find this awesome. I don't have a Windows machine to play with,
>>> but I think I may have found something.
>>> The difference is how Java_sun_nio_ch_Net_poll is implemented. On unix
>>> it uses poll(2), on Windows it uses select(2). Regarding timeouts,
>>> poll() has "wait at least" semantics and overruns by design[1], while
>>> select() on windows has "waits at most" semantics, or how they put
>>> it[2]: "specifies the maximum time that select should wait before
>>> returning.". It returns early by design! Is this a known thing? I
>>> don't think there is much one can do here. It probably makes no sense
>>> to loop it and wait for time remainder.
>>> Java's soTimeout does not specify[3] should it wait at least or at
>>> most the specified timeout, so it's fine I guess. Old, "plain" socket
>>> impl are not much different.
>>> 
>>> If the above is correct, should I just add a tolerance for the lower bound?
>>> 
>>> [1] http://man7.org/linux/man-pages/man2/poll.2.html
>>> [2] 
>>> https://docs.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-select
>>> [3] 
>>> https://docs.oracle.com/javase/9/docs/api/java/net/Socket.html#setSoTimeout-int-
>>> 
>>> On Mon, 23 Sep 2019 at 16:15, Pavel Rappo  wrote:
 
 Milan,
 
 I'm observing the latest version (.04) of this test failing quite 
 frequently (4/100) on Windows (Windows Server 2012 R2 6.3 (amd64)) 
 machines. The test passes fine on macOS and Linux. Here's the typical 
 output I see in the logs:
 
   java.lang.RuntimeException: Query took 4997 ms. . Timeout value is 5000
   java.lang.RuntimeException: Query took 4999 ms. . Timeout value is 5000
   java.lang.RuntimeException: Query took 4995 ms. . Timeout value is 5000
   java.lang.RuntimeException: Query took 4998 ms. . Timeout value is 5000
   ...
 
 Now, there might be many reasons for that. One of which would be that the 
 DnsClient code is buggy. The other reason would be that the accuracy 
 guaranteed by Windows implementation of `read` is not what we would 
 expect. Would you be able to investigate that?
 
 P.S. The good news is that the CSR has been approved:
 
   https://bugs.openjdk.java.net/browse/JDK-8230965
 
> On 23 Sep 2019, at 14:20, Milan Mimica  wrote:
> 
> Got it. Thanks Pavel!
> 
> 
> On Mon, 23 Sep 2019 at 13:37, Pavel Rappo  wrote:
>> 
>> Milan,
>> 
>> How do you check which tests are run? That's what I see in the 
>> /test-support/jtreg_open_test_jdk_com_sun_jndi_dns_ConfigTests_TcpTimeout_java/com/sun/jndi/dns/ConfigTests/TcpTimeout.jtr
>>  file after I have run the test locally on my machine:
>> 
>> --messages:(5/233)--
>> command: main TcpTimeout
>> reason: User specified action: run main TcpTimeout
>> Mode: othervm
>> Additional options from @modules: --add-modules java.base --add-exports 
>> java.base/sun.security.util=ALL-UNNAMED
>> elapsed time (seconds): 1.751
>> 
>> ...
>> 
>> --messages:(5/313)--
>> command: main TcpTimeout -Dcom.sun.jndi.dns.timeout.initial=5000
>> reason: User specified action: run main TcpTimeout 
>> -Dcom.sun.jndi.dns.timeout.initial=5000
>> Mode: othervm
>> Additional options from @modules: --add-modules java.base --add-exports 
>> java.base/sun.security.util=ALL-UNNAMED
>> elapsed time (seconds): 5.498
>> 
>> 
>> 
>>

Why you want a better specification and a fast implementation of Double.toString(double)

[Raffaello Giulietti]

Hi,

I'd like this community to help me pushing a new spec and the 
accompanying implementation of Double.toString(double) and 
Float.toString(float) into the OpenJDK 14 release.


All material has already been submitted to this mailing list months ago 
[2] and passes tier 1 tests. Yet, there hasn't been much discussion or 
progress.


I understand that everybody is busy with many other big and small 
issues, duties and enhancements. But while my sponsor Brian Burkhalter 
started to look at it, he can't do everything alone, so I'm still 
looking for reviewers interested in the subject.


I would also be much interested in tactics, strategies and techniques to 
accelerate the review and approval process. Languishing here and waiting 
for good souls is not a nice position to be ;-)




Greetings
Raffaello



Q: Why a new implementation? What's wrong with the current one?

A: The current one tries to adhere to the rather non-mathematical 
current spec and does so in a costly and buggy way. Not that the bugs 
produce patently wrong results, but they are annoying anyway. They can 
all be categorized under the "too many digits" kind of bug.




Q: Isn't there a risk in replacing the current implementation with a new 
one?


A: Every change in the OpenJDK bears potential risks. On the other hand, 
the proposed implementation has been extensively tested in all aspects, 
without troubles. The risks are related more to reliance on current 
(mis)behavior than to unnoticed bugs in the new implementation.




Q: What are the cons of the new implementation?

A: Technically, there are none known to me. As alluded above, the only 
disadvantage is that the new outcomes are sometimes different, and then 
for the better (shorter outputs). Code that relies on the exact current 
behavior can possibly break.
For example, a test that expects Double.toString(2e23) to produce 
"1.9998E23", as currently done, rather than "2.0E23", as 
returned by the new implementation, will fail.




Q: Can you list some of the pros of the new implementation?

A: Two stand out:
(1) on the average, a conversion is about 15x faster (around 90 ns on a 
6 years old home-grade laptop)

(2) no temporary objects are ever allocated



Q: Why is there a need for another spec?

A: Because the current one is rather unclear in its main goal. In 
addition, it intermingles the selection of a decimal to represent the 
floating-point number with its formatting, which adds to the unclarity.




Q: What about the proposed spec.
A: It clearly separates the rather simple, mathematical and 100% 
unambiguous selection of the decimal from the formatting aspects.




Q: Which kind of help is needed?

A: You can:
(1) Take a look at the proposed spec [1] and send questions and comments 
to this mailing list.
(2) Take a look at the overall structure of the implementation [2] 
(webrev is in [3]) and discuss it here, even without understanding the 
underlying algorithms. The focus could be on quality, conventions, etc.
(3) Take a look at the accompanying paper [4] which presents all the 
nitty-gritty details and let me know if there's something unclear or wrong.
(4) You can try out the code. It is now about 6 months old, meant for 
and tested on OpenJDK 13. I guess it will work on OpenJDK 14 as well, 
but I didn't try out as of today.




Q: What is the investment in time?

A: The spec can be read and understood in about 10-15 minutes.
The paper needs 0.5-1.0 days, depending on your willingness to 
understand every detail. Once the paper has been worked out, however, 
the core of the implementation is straightforward to understand, say a 
couple of hours.




[1] https://bugs.openjdk.java.net/browse/JDK-8202555
[2] 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-April/059783.html

[3] http://cr.openjdk.java.net/~bpb/4511638/webrev.03/
[4] https://drive.google.com/open?id=1KLtG_LaIbK9ETXI290zqCxvBW94dj058

Re: RFR 8221623 : Add StackWalker micro benchmarks to jdk repo

[Mandy Chung]

On 9/24/19 1:01 PM, Brent Christian wrote:

http://cr.openjdk.java.net/~bchristi/8221623/webrev11/


Looks okay.  Thanks for doing this.

Mandy

Re: RFR 8221623 : Add StackWalker micro benchmarks to jdk repo

[Brent Christian]

On 9/23/19 4:48 PM, Mandy Chung wrote:


I think doing the measurement for one of these would be adequate.

StackWalkBench.forEach_AllOpts
StackWalkBench.forEach_DefaultOpts
StackWalkBench.forEach_HiddenAndReflectFrames


OK, reduced to just DefaultOpts.


There are a couple of commented benchmarks.  Should they be taken out?


Sure; testGoThereBH() was from some early experimenting and is not 
needed - removed.


The getCallerClass() tests in ThrowableRuntimeMicros served as a 
reminder that they need updating, but having JDK-8230976 filed is 
sufficient.


IMO, it would be nice to have the PerfCounter example in StackWalkBench 
ready-to-uncomment, in case it's needed again sometime.


Though really, since logging is no longer using Throwable to examine 
the call stack, maybe it makes more sense to move the logging 
benchmarks to their own file under:


test/micro/org/openjdk/bench/java/util/logging/


That seems a good idea.


Yes, those are moved.


http://cr.openjdk.java.net/~bchristi/8221623/webrev11/

Thanks,
-Brent

Re: CharsetEncoder.maxBytesPerChar()

[Ulf Zibis]

Am 21.09.19 um 00:03 schrieb mark.reinh...@oracle.com:
> To avoid this confusion, a more verbose specification might read:
>  * Returns the maximum number of $otype$s that will be produced for each
>  * $itype$ of input.  This value may be used to compute the worst-case 
> size
>  * of the output buffer required for a given input sequence. This value
>  * accounts for any necessary content-independent prefix or suffix
> #if[encoder]
>  * $otype$s, such as byte-order marks.
> #end[encoder]
> #if[decoder]
>  * $otype$s.
> #end[decoder]
>
> (The example of byte-order marks applies only to CharsetEncoders, so
>  I’ve conditionalized that text for Charset-X-Coder.java.template.)

Hi,

wouldn't it be more clear to use "char" or even "{@code char}" instead
"character" as replacment for the $xtype$ parameters?

-Ulf

Re: RFR(s): 8228580: DnsClient TCP socket timeout

[Chris Hegarty]

> On 24 Sep 2019, at 13:41, Milan Mimica  wrote:
> 
> Pavel,
> 
> Deal. Handling early returns too:
> http://cr.openjdk.java.net/~mmimica/8228580/webrev.05/

LGTM

-Chris

Re: RFR: JDK-8230927 : Wrong arguments set for additional launchers

[Alexey Semenyuk]

Looks good.

- Alexey

On 9/24/2019 1:24 PM, Andy Herrick wrote:
Revision 2 or the webrev ( [3] )  removes the comment about 
`--linux-deb-copyright-file` option., replaces the literal strings 
with the CLIOptions id's, and adds unit test to AddLauncherTest.java


[3] http://cr.openjdk.java.net/~herrick/8230927/webrev.02/

/Andy

On 9/24/2019 9:19 AM, Alexey Semenyuk wrote:

Andy,

Please remove javadoc update about `--linux-deb-copyright-file` 
option. It will be dropped in 
https://bugs.openjdk.java.net/browse/JDK-8231277 patch.

Line 180:
---
if (additional.containsKey("java-optiions")) {
---
Looks like a typo. Should be "java-options", not "java-optiions", I 
guess.
We probably need a test or at least a follow up CR to track the task 
of adding a test for "java-options" parameter in additional launchers.


- Alexey

On 9/24/2019 8:46 AM, Andy Herrick wrote:

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This change applies when arguments or java-options are used in an 
add-launcher properties file.


In these cases the arguments or java-options from the properties 
files should replace instead of being added to the same options used 
in the command line  for the main launcher.


[1] https://bugs.openjdk.java.net/browse/JDK-8230927

[2] http://cr.openjdk.java.net/~herrick/8230927

/Andy

Re: RFR: JDK-8230927 : Wrong arguments set for additional launchers

[Andy Herrick]

Revision 2 or the webrev ( [3] )  removes the comment about 
`--linux-deb-copyright-file` option., replaces the literal strings with 
the CLIOptions id's, and adds unit test to AddLauncherTest.java


[3] http://cr.openjdk.java.net/~herrick/8230927/webrev.02/

/Andy

On 9/24/2019 9:19 AM, Alexey Semenyuk wrote:

Andy,

Please remove javadoc update about `--linux-deb-copyright-file` 
option. It will be dropped in 
https://bugs.openjdk.java.net/browse/JDK-8231277 patch.

Line 180:
---
if (additional.containsKey("java-optiions")) {
---
Looks like a typo. Should be "java-options", not "java-optiions", I 
guess.
We probably need a test or at least a follow up CR to track the task 
of adding a test for "java-options" parameter in additional launchers.


- Alexey

On 9/24/2019 8:46 AM, Andy Herrick wrote:

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This change applies when arguments or java-options are used in an 
add-launcher properties file.


In these cases the arguments or java-options from the properties 
files should replace instead of being added to the same options used 
in the command line  for the main launcher.


[1] https://bugs.openjdk.java.net/browse/JDK-8230927

[2] http://cr.openjdk.java.net/~herrick/8230927

/Andy

Re: Thread.suspend/resume - any reason not to deprecate forRemoval=true

[mark . reinhold]

2019/9/24 8:04:55 -0700, alan.bate...@oracle.com:
> Thread.suspend/resume (and the corresponding methods in ThreadGroup) 
> have been deprecated since 1.2 (1998). I haven't see anything use these 
> methods in many years. Would anyone care if their deprecation is changed 
> to forRemoval=true with a view to really removing them in the future? 
> Just to clear, I'm only asking about the Thread/ThreadGroups APIs, we 
> have to keep suspend/resume support in the debugger and tool APIs.

Go for it!

- Mark

Re: [14] RFR: 8230531: API Doc for CharsetEncoder.maxBytesPerChar() should be clearer about BOMs

[Alan Bateman]

On 23/09/2019 21:45, naoto.s...@oracle.com wrote:

Hello,

Please review the fix to the following issue:

https://bugs.openjdk.java.net/browse/JDK-8230531

Relevant CSR (in draft) and proposed changeset are located at:

[CSR]: https://bugs.openjdk.java.net/browse/JDK-8231319
[changeset]: https://cr.openjdk.java.net/~naoto/8230531/webrev.00/

Looks good.

-Alan

Thread.suspend/resume - any reason not to deprecate forRemoval=true

[Alan Bateman]

Thread.suspend/resume (and the corresponding methods in ThreadGroup) 
have been deprecated since 1.2 (1998). I haven't see anything use these 
methods in many years. Would anyone care if their deprecation is changed 
to forRemoval=true with a view to really removing them in the future? 
Just to clear, I'm only asking about the Thread/ThreadGroups APIs, we 
have to keep suspend/resume support in the debugger and tool APIs.


-Alan.

RE: RFR (L, final): 8218626: Add detailed message to NullPointerException describing what is null.

[Lindenmaier, Goetz]

Thanks!

Best regards,
  Goetz

> -Original Message-
> From: Roger Riggs 
> Sent: Dienstag, 24. September 2019 15:54
> To: Lindenmaier, Goetz ; Hotspot dev runtime
> ; Java Core Libs  d...@openjdk.java.net>
> Subject: Re: RFR (L, final): 8218626: Add detailed message to
> NullPointerException describing what is null.
> 
> Hi Goetz,
> 
> Looks good.
> Count me as a (java) Reviewer.
> 
> Thanks, Roger
> 
> 
> On 9/24/19 4:13 AM, Lindenmaier, Goetz wrote:
> 
> 
>   Hi Roger,
> 
>   thanks for improving the text!  Good point to add
>   @implNote.
>   This webrev includes the fixed comments:
>   http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/19/
>   Is it ok to add you as reviewer (for the java.base part)?
> 
>   Best regards,
> Goetz.
> 
> 
> 
>   -Original Message-
>   From: Roger Riggs 
> 
>   Sent: Montag, 23. September 2019 17:30
>   To: Lindenmaier, Goetz 
>  ; Hotspot dev runtime
> runtime-...@openjdk.java.net> ; Java Core Libsd...@openjdk.java.net  >
>   Subject: Re: RFR (L, final): 8218626: Add detailed message to
>   NullPointerException describing what is null.
> 
>   Hi Goetz,
> 
>   A bit of wordsmithing on the javadoc of
> NullPointerException.getMessage
>   and separating out the implementation specific description to
> an @implNote
> 
> 
>   75:
>   /**
>* Returns the detail message string of this throwable.
>* 
>* If a non-null message was supplied in a constructor it is
> returned.
>* Otherwise, an implementation specific message or {@code
> null} is
>   returned.
>* @ImplNote
>* If no explicit message was passed to the constructor, and
> as
>* long as certain internal information is available, a 
> verbose
>* description of the null reference is returned.
>* The internal information is not available in deserialized
>   NullPointerExceptions.
>*
>* @return the detail message string, which may be {@code
> null}.
>*
>   94-97: The comment on the getExtendedNPEMessage should
> use the normal
>   /**... */ syntax.
> 
>   Thanks, Roger
> 
> 
> 
> 
>   On 9/17/19 10:18 AM, Lindenmaier, Goetz wrote:
> 
> 
>   @core-libs experts, I would appreciate comments on
> the changes
>   to NullPointerException.java, especially wrt. the
> Javadoc comment.
>   The change there is S.
> 
>   Best regards,
> Goetz.
> 
> 
>   -Original Message-
>   From: Lindenmaier, Goetz
>   Sent: Dienstag, 10. September 2019 11:48
>   To: 'Hotspot dev runtime'd...@openjdk.java.net  >
>   d...@openjdk.java.net>  ; Java
>   Core
>   Libs 
>      d...@openjdk.java.net>  d...@openjdk.java.net>
>   Subject: RFR (L, final): 8218626: Add detailed
> message to
>   NullPointerException
>   describing what is null.
> 
>   Hi,
> 
> 
> 
>   this is the implementation of JEP 358: Helpful
>   NullPointerExceptions.
> 
> 
> 
>   The JEP is in status "Candidate". Coleen (many,
> many thanks!)
>   ran
> 
>   it through the Oracle-internal processes.  Now I
> please need
>   final reviews
> 
>   for this change so that I can propose it to
> target jdk 14.
> 
> 
> 
>   JEP:
> https://bugs.openjdk.java.net/browse/JDK-8220715
> 
>   Enhancement:
> https://bugs.openjdk.java.net/browse/JDK-
>   8218628
> 
>   webrev:
> http://cr.openjdk.java.net/~goetz/wr19/8218628-
>   exMsg-NPE/16/
> 
> 
> 
>   The change ran through a lot of testing, all
> jtreg and jck tests to
>   name
> 
>   only some. The webrev points to patch
> 
> 
>   http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-
>   NPE/16/enable_NPE_m

Re: RFR: JDK-8231277: Adjust Linux application image layout

[Andy Herrick]

Looks good.

/Andy

On 9/20/19 7:57 AM, Alexey Semenyuk wrote:


Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This fix:

- directory layout of Linux app image adjusted to better comply with 
Linux FSH.

- some unused code clean up.
- added support to run JUnit4 tests along with the standard jtreg tests.
- DeployParamsTest.java jtreg test converted into JUnit test.
- unit tests added.

Baseline for the fix is 
http://cr.openjdk.java.net/~asemenyuk/8225249/webrev.02/ patch.


[1] https://bugs.openjdk.java.net/browse/JDK-8231277

[2] http://cr.openjdk.java.net/~asemenyuk/8231277/webrev.00/

Re: RFR: JDK-8231282: Revisit --linux-deb-copyright option

[Andy Herrick]

looks good

/Andy

On 9/23/2019 7:55 PM, Alexey Semenyuk wrote:

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This fix:

- remove --linux-deb-copyright option.
- add tests to verify that placing copyright template in custom 
resource dir provides the same functionality as the use of 
--linux-deb-copyright option.
- adjusted test helper classes to save from writing explicit code to 
rethrow checked exceptions as RuntimeException.


Baseline for the fix is [3].

[1] https://bugs.openjdk.java.net/browse/JDK-8231277

[2] http://cr.openjdk.java.net/~asemenyuk/8231282/webrev.00/

[3] http://cr.openjdk.java.net/~asemenyuk/8231279/webrev.01/

Re: RFR (L, final): 8218626: Add detailed message to NullPointerException describing what is null.

[Roger Riggs]

Hi Goetz,

Looks good.
Count me as a (java) Reviewer.

Thanks, Roger

On 9/24/19 4:13 AM, Lindenmaier, Goetz wrote:

Hi Roger,

thanks for improving the text!  Good point to add
@implNote.
This webrev includes the fixed comments:
http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/19/
Is it ok to add you as reviewer (for the java.base part)?

Best regards,
   Goetz.



-Original Message-
From: Roger Riggs 
Sent: Montag, 23. September 2019 17:30
To: Lindenmaier, Goetz ; Hotspot dev runtime
; Java Core Libs 
Subject: Re: RFR (L, final): 8218626: Add detailed message to
NullPointerException describing what is null.

Hi Goetz,

A bit of wordsmithing on the javadoc of NullPointerException.getMessage
and separating out the implementation specific description to an @implNote


75:
 /**
  * Returns the detail message string of this throwable.
  * 
  * If a non-null message was supplied in a constructor it is returned.
  * Otherwise, an implementation specific message or {@code null} is
returned.
  * @ImplNote
  * If no explicit message was passed to the constructor, and as
  * long as certain internal information is available, a verbose
  * description of the null reference is returned.
  * The internal information is not available in deserialized
NullPointerExceptions.
  *
  * @return the detail message string, which may be {@code null}.
  *
94-97: The comment on the getExtendedNPEMessage should use the normal
/**... */ syntax.

Thanks, Roger




On 9/17/19 10:18 AM, Lindenmaier, Goetz wrote:


@core-libs experts, I would appreciate comments on the changes
to NullPointerException.java, especially wrt. the Javadoc comment.
The change there is S.

Best regards,
  Goetz.


-Original Message-
From: Lindenmaier, Goetz
Sent: Dienstag, 10. September 2019 11:48
To: 'Hotspot dev runtime'   ; Java
Core
Libs  
Subject: RFR (L, final): 8218626: Add detailed message to
NullPointerException
describing what is null.

Hi,



this is the implementation of JEP 358: Helpful
NullPointerExceptions.



The JEP is in status "Candidate". Coleen (many, many thanks!)
ran

it through the Oracle-internal processes.  Now I please need
final reviews

for this change so that I can propose it to target jdk 14.



JEP: https://bugs.openjdk.java.net/browse/JDK-8220715

Enhancement: https://bugs.openjdk.java.net/browse/JDK-
8218628

webrev: http://cr.openjdk.java.net/~goetz/wr19/8218628-
exMsg-NPE/16/



The change ran through a lot of testing, all jtreg and jck 
tests to
name

only some. The webrev points to patch

http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-
NPE/16/enable_NPE_message.patch

that enables the change by default,  which was useful for
testing to

assure the code is used in the tests.

I just pushed the change to jdk/submit once more.



Please review.



Thanks!

  Goetz.

Re: RFR: JDK-8230927 : Wrong arguments set for additional launchers

[Alexey Semenyuk]

Andy,

Please remove javadoc update about `--linux-deb-copyright-file` option. 
It will be dropped in https://bugs.openjdk.java.net/browse/JDK-8231277 
patch.

Line 180:
---
if (additional.containsKey("java-optiions")) {
---
Looks like a typo. Should be "java-options", not "java-optiions", I guess.
We probably need a test or at least a follow up CR to track the task of 
adding a test for "java-options" parameter in additional launchers.


- Alexey

On 9/24/2019 8:46 AM, Andy Herrick wrote:

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This change applies when arguments or java-options are used in an 
add-launcher properties file.


In these cases the arguments or java-options from the properties files 
should replace instead of being added to the same options used in the 
command line  for the main launcher.


[1] https://bugs.openjdk.java.net/browse/JDK-8230927

[2] http://cr.openjdk.java.net/~herrick/8230927

/Andy

RFR: JDK-8230920 : jpackage problems when -input dir contains any files with "cfg" extension.

[Andy Herrick]

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This fix replaces the practice we were using on windows to determine the 
application name, and name of additional launchers (by looking for 
".cfg" files in the app dir.


Instead we now add a file ".jpackage.args" to the root of the app-image, 
and record in that file all the arguments used to create the app-image.  
We later read that file to determine the original app name and any 
additional launcher names.


This change also fixes the shortcut creation on windows to create 
shortcuts (if so directed) for all launchers.


[1] https://bugs.openjdk.java.net/browse/JDK-8230920

[2] http://cr.openjdk.java.net/~herrick/8230920/webrev.01/

/Andy

RFR: JDK-8230927 : Wrong arguments set for additional launchers

[Andy Herrick]

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This change applies when arguments or java-options are used in an 
add-launcher properties file.


In these cases the arguments or java-options from the properties files 
should replace instead of being added to the same options used in the 
command line  for the main launcher.


[1] https://bugs.openjdk.java.net/browse/JDK-8230927

[2] http://cr.openjdk.java.net/~herrick/8230927

/Andy

Re: RFR(s): 8228580: DnsClient TCP socket timeout

[Milan Mimica]

Pavel,

Deal. Handling early returns too:
http://cr.openjdk.java.net/~mmimica/8228580/webrev.05/
I will ask there about socket timeout semantics.

On Tue, 24 Sep 2019 at 12:51, Pavel Rappo  wrote:
>
> Milan,
>
> Thanks for looking into this. I think you should ask a question on the 
> expected timing semantics and guarantees on net-dev (with maybe a cc to 
> nio-dev).
> As for our test. I agree with you that we should simply work a possibility of 
> early returns into the check.
>
> ...
>
> /* The acceptable variation of early returns from timed socket operations. */
> private static final long PREMATURE_RETURN = adjustTimeout(100);
>
> ...
>
> long elapsed = NANOSECONDS.toMillis(System.nanoTime() - startNanos);
> if (elapsed < timeout - PREMATURE_RETURN || elapsed > timeout + TOLERANCE) {
> String msg = String.format(
> "elapsed=%s, timeout=%s, TOLERANCE=%s, PREMATURE_RETURN=%s",
> timeout, timeout, TOLERANCE, PREMATURE_RETURN);
> throw new RuntimeException(msg);
> }
>
> ...
>
> Thoughts?
>
> -Pavel
>
> > On 24 Sep 2019, at 09:12, Milan Mimica  wrote:
> >
> > Hi Pavel
> >
> > Wow, I find this awesome. I don't have a Windows machine to play with,
> > but I think I may have found something.
> > The difference is how Java_sun_nio_ch_Net_poll is implemented. On unix
> > it uses poll(2), on Windows it uses select(2). Regarding timeouts,
> > poll() has "wait at least" semantics and overruns by design[1], while
> > select() on windows has "waits at most" semantics, or how they put
> > it[2]: "specifies the maximum time that select should wait before
> > returning.". It returns early by design! Is this a known thing? I
> > don't think there is much one can do here. It probably makes no sense
> > to loop it and wait for time remainder.
> > Java's soTimeout does not specify[3] should it wait at least or at
> > most the specified timeout, so it's fine I guess. Old, "plain" socket
> > impl are not much different.
> >
> > If the above is correct, should I just add a tolerance for the lower bound?
> >
> > [1] http://man7.org/linux/man-pages/man2/poll.2.html
> > [2] 
> > https://docs.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-select
> > [3] 
> > https://docs.oracle.com/javase/9/docs/api/java/net/Socket.html#setSoTimeout-int-
> >
> > On Mon, 23 Sep 2019 at 16:15, Pavel Rappo  wrote:
> >>
> >> Milan,
> >>
> >> I'm observing the latest version (.04) of this test failing quite 
> >> frequently (4/100) on Windows (Windows Server 2012 R2 6.3 (amd64)) 
> >> machines. The test passes fine on macOS and Linux. Here's the typical 
> >> output I see in the logs:
> >>
> >>java.lang.RuntimeException: Query took 4997 ms. . Timeout value is 5000
> >>java.lang.RuntimeException: Query took 4999 ms. . Timeout value is 5000
> >>java.lang.RuntimeException: Query took 4995 ms. . Timeout value is 5000
> >>java.lang.RuntimeException: Query took 4998 ms. . Timeout value is 5000
> >>...
> >>
> >> Now, there might be many reasons for that. One of which would be that the 
> >> DnsClient code is buggy. The other reason would be that the accuracy 
> >> guaranteed by Windows implementation of `read` is not what we would 
> >> expect. Would you be able to investigate that?
> >>
> >> P.S. The good news is that the CSR has been approved:
> >>
> >>https://bugs.openjdk.java.net/browse/JDK-8230965
> >>
> >>> On 23 Sep 2019, at 14:20, Milan Mimica  wrote:
> >>>
> >>> Got it. Thanks Pavel!
> >>>
> >>>
> >>> On Mon, 23 Sep 2019 at 13:37, Pavel Rappo  wrote:
> 
>  Milan,
> 
>  How do you check which tests are run? That's what I see in the 
>  /test-support/jtreg_open_test_jdk_com_sun_jndi_dns_ConfigTests_TcpTimeout_java/com/sun/jndi/dns/ConfigTests/TcpTimeout.jtr
>   file after I have run the test locally on my machine:
> 
>  --messages:(5/233)--
>  command: main TcpTimeout
>  reason: User specified action: run main TcpTimeout
>  Mode: othervm
>  Additional options from @modules: --add-modules java.base --add-exports 
>  java.base/sun.security.util=ALL-UNNAMED
>  elapsed time (seconds): 1.751
> 
>  ...
> 
>  --messages:(5/313)--
>  command: main TcpTimeout -Dcom.sun.jndi.dns.timeout.initial=5000
>  reason: User specified action: run main TcpTimeout 
>  -Dcom.sun.jndi.dns.timeout.initial=5000
>  Mode: othervm
>  Additional options from @modules: --add-modules java.base --add-exports 
>  java.base/sun.security.util=ALL-UNNAMED
>  elapsed time (seconds): 5.498
> 
>  
> 
>  Which is consistent with what I would expect given the timeout values.
> 
>  The following output does not tell the full story, just the name of the 
>  test:
> 
>  ==
>  Test summary
>  ==
>   TEST  TOTAL  PASS  

Re: RFR: 8231186: Replace html tag foo with javadoc tag {@code foo} in java.base

[Daniel Fuchs]

Thanks Julia!

It's now pushed.

best regards,

-- daniel

On 24/09/2019 10:50, Julia Boes wrote:

Hi,

Thanks for the review, Lance and Brent!

Changeset: http://cr.openjdk.java.net/~jboes/webrevs/8231186/webrev.04/


Regards,

Julia

RE: RFR (L, final): 8218626: Add detailed message to NullPointerException describing what is null.

[Schmelter, Ralf]

Hi Goetz,

just one thing:
 
>> In NullPointerExceptionTest.java:
> > 
> > It seems you don't have tests for invokeinterface or invokespecial calls to 
> > cause
> > an NPE (e.g. by calling a null interface variable or a private non-static 
> > method
> > of a null objects).
> That is because the code is the same as for invokevirtual in all places in 
> bytecodeUtils.cpp.

The test should not omit these two bytecodes because the current implementation 
is the same. This can change and it is not much additional code to add the two 
cases.

Thanks and best regards,
  Goetz.

Re: RFR(s): 8228580: DnsClient TCP socket timeout

[Pavel Rappo]

Milan,

Thanks for looking into this. I think you should ask a question on the expected 
timing semantics and guarantees on net-dev (with maybe a cc to nio-dev).
As for our test. I agree with you that we should simply work a possibility of 
early returns into the check.

...

/* The acceptable variation of early returns from timed socket operations. */
private static final long PREMATURE_RETURN = adjustTimeout(100);

...

long elapsed = NANOSECONDS.toMillis(System.nanoTime() - startNanos);
if (elapsed < timeout - PREMATURE_RETURN || elapsed > timeout + TOLERANCE) {
String msg = String.format(
"elapsed=%s, timeout=%s, TOLERANCE=%s, PREMATURE_RETURN=%s",
timeout, timeout, TOLERANCE, PREMATURE_RETURN);
throw new RuntimeException(msg);
}

...

Thoughts?

-Pavel

> On 24 Sep 2019, at 09:12, Milan Mimica  wrote:
> 
> Hi Pavel
> 
> Wow, I find this awesome. I don't have a Windows machine to play with,
> but I think I may have found something.
> The difference is how Java_sun_nio_ch_Net_poll is implemented. On unix
> it uses poll(2), on Windows it uses select(2). Regarding timeouts,
> poll() has "wait at least" semantics and overruns by design[1], while
> select() on windows has "waits at most" semantics, or how they put
> it[2]: "specifies the maximum time that select should wait before
> returning.". It returns early by design! Is this a known thing? I
> don't think there is much one can do here. It probably makes no sense
> to loop it and wait for time remainder.
> Java's soTimeout does not specify[3] should it wait at least or at
> most the specified timeout, so it's fine I guess. Old, "plain" socket
> impl are not much different.
> 
> If the above is correct, should I just add a tolerance for the lower bound?
> 
> [1] http://man7.org/linux/man-pages/man2/poll.2.html
> [2] 
> https://docs.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-select
> [3] 
> https://docs.oracle.com/javase/9/docs/api/java/net/Socket.html#setSoTimeout-int-
> 
> On Mon, 23 Sep 2019 at 16:15, Pavel Rappo  wrote:
>> 
>> Milan,
>> 
>> I'm observing the latest version (.04) of this test failing quite frequently 
>> (4/100) on Windows (Windows Server 2012 R2 6.3 (amd64)) machines. The test 
>> passes fine on macOS and Linux. Here's the typical output I see in the logs:
>> 
>>java.lang.RuntimeException: Query took 4997 ms. . Timeout value is 5000
>>java.lang.RuntimeException: Query took 4999 ms. . Timeout value is 5000
>>java.lang.RuntimeException: Query took 4995 ms. . Timeout value is 5000
>>java.lang.RuntimeException: Query took 4998 ms. . Timeout value is 5000
>>...
>> 
>> Now, there might be many reasons for that. One of which would be that the 
>> DnsClient code is buggy. The other reason would be that the accuracy 
>> guaranteed by Windows implementation of `read` is not what we would expect. 
>> Would you be able to investigate that?
>> 
>> P.S. The good news is that the CSR has been approved:
>> 
>>https://bugs.openjdk.java.net/browse/JDK-8230965
>> 
>>> On 23 Sep 2019, at 14:20, Milan Mimica  wrote:
>>> 
>>> Got it. Thanks Pavel!
>>> 
>>> 
>>> On Mon, 23 Sep 2019 at 13:37, Pavel Rappo  wrote:
 
 Milan,
 
 How do you check which tests are run? That's what I see in the 
 /test-support/jtreg_open_test_jdk_com_sun_jndi_dns_ConfigTests_TcpTimeout_java/com/sun/jndi/dns/ConfigTests/TcpTimeout.jtr
  file after I have run the test locally on my machine:
 
 --messages:(5/233)--
 command: main TcpTimeout
 reason: User specified action: run main TcpTimeout
 Mode: othervm
 Additional options from @modules: --add-modules java.base --add-exports 
 java.base/sun.security.util=ALL-UNNAMED
 elapsed time (seconds): 1.751
 
 ...
 
 --messages:(5/313)--
 command: main TcpTimeout -Dcom.sun.jndi.dns.timeout.initial=5000
 reason: User specified action: run main TcpTimeout 
 -Dcom.sun.jndi.dns.timeout.initial=5000
 Mode: othervm
 Additional options from @modules: --add-modules java.base --add-exports 
 java.base/sun.security.util=ALL-UNNAMED
 elapsed time (seconds): 5.498
 
 
 
 Which is consistent with what I would expect given the timeout values.
 
 The following output does not tell the full story, just the name of the 
 test:
 
 ==
 Test summary
 ==
  TEST  TOTAL  PASS  FAIL ERROR
  jtreg:open/test/jdk/com/sun/jndi/dns/ConfigTests/TcpTimeout.java
1 1 0 0
 ==
 TEST SUCCESS
 
 -Pavel
 
> On 20 Sep 2019, at 15:42, Milan Mimica  wrote:
> 
> Pavel,
> 
> Here it is: http://cr.openjdk.java.net/~mmimica/8228580/webrev.04/
> I don't see the 

Re: RFR: 8231186: Replace html tag foo with javadoc tag {@code foo} in java.base

[Julia Boes]

Hi,

Thanks for the review, Lance and Brent!

Changeset: http://cr.openjdk.java.net/~jboes/webrevs/8231186/webrev.04/


Regards,

Julia

On 23/09/2019 19:58, Lance Andersen wrote:

Hi Julia,

I made a quick pass and the changes seem OK
On Sep 23, 2019, at 2:17 PM, Julia Boes > wrote:


Hi Brent,

I was able to generate a webrev without any missing sdiffs (using 
gawk instead of awk with webrev.ksh) and made the requested changes 
below.



src/java.base/share/classes/java/util/ResourceBundle.java


I believe the  tag spanning L2801-2 can be changed:

2801  * Special cases for Norwegian.  Both 
Locale("no", "NO",
2802  * "NY") and {@code Locale("nn", "NO")} 
represent Norwegian


Same at L2818-9:

2818  * Bokmål "nb".  Except for the single case 
Locale("no",
2819  * "NO", "NY") (handled above), when an input 
{@code Locale}



The copyright year is now updated where applicable.

Updated webrev: 
http://cr.openjdk.java.net/~jboes/webrevs/8231186/webrev.03/



Regards,

Julia






Lance 
Andersen| Principal Member of Technical Staff | +1.781.442.2037

Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com 

RE: RFR (L, final): 8218626: Add detailed message to NullPointerException describing what is null.

[Lindenmaier, Goetz]

Hi Roger, 

thanks for improving the text!  Good point to add
@implNote.
This webrev includes the fixed comments:
http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/19/
Is it ok to add you as reviewer (for the java.base part)?

Best regards,
  Goetz.


> -Original Message-
> From: Roger Riggs 
> Sent: Montag, 23. September 2019 17:30
> To: Lindenmaier, Goetz ; Hotspot dev runtime
> ; Java Core Libs  d...@openjdk.java.net>
> Subject: Re: RFR (L, final): 8218626: Add detailed message to
> NullPointerException describing what is null.
> 
> Hi Goetz,
> 
> A bit of wordsmithing on the javadoc of NullPointerException.getMessage
> and separating out the implementation specific description to an @implNote
> 
> 
> 75:
> /**
>  * Returns the detail message string of this throwable.
>  * 
>  * If a non-null message was supplied in a constructor it is returned.
>  * Otherwise, an implementation specific message or {@code null} is
> returned.
>  * @ImplNote
>  * If no explicit message was passed to the constructor, and as
>  * long as certain internal information is available, a verbose
>  * description of the null reference is returned.
>  * The internal information is not available in deserialized
> NullPointerExceptions.
>  *
>  * @return the detail message string, which may be {@code null}.
>  *
> 94-97: The comment on the getExtendedNPEMessage should use the normal
> /**... */ syntax.
> 
> Thanks, Roger
> 
> 
> 
> 
> On 9/17/19 10:18 AM, Lindenmaier, Goetz wrote:
> 
> 
>   @core-libs experts, I would appreciate comments on the changes
>   to NullPointerException.java, especially wrt. the Javadoc comment.
>   The change there is S.
> 
>   Best regards,
> Goetz.
> 
> 
>   -Original Message-
>   From: Lindenmaier, Goetz
>   Sent: Dienstag, 10. September 2019 11:48
>   To: 'Hotspot dev runtime'  d...@openjdk.java.net>  ; Java
> Core
>   Libs   d...@openjdk.java.net>
>   Subject: RFR (L, final): 8218626: Add detailed message to
> NullPointerException
>   describing what is null.
> 
>   Hi,
> 
> 
> 
>   this is the implementation of JEP 358: Helpful
> NullPointerExceptions.
> 
> 
> 
>   The JEP is in status "Candidate". Coleen (many, many thanks!)
> ran
> 
>   it through the Oracle-internal processes.  Now I please need
> final reviews
> 
>   for this change so that I can propose it to target jdk 14.
> 
> 
> 
>   JEP: https://bugs.openjdk.java.net/browse/JDK-8220715
> 
>   Enhancement: https://bugs.openjdk.java.net/browse/JDK-
> 8218628
> 
>   webrev: http://cr.openjdk.java.net/~goetz/wr19/8218628-
> exMsg-NPE/16/
> 
> 
> 
>   The change ran through a lot of testing, all jtreg and jck 
> tests to
> name
> 
>   only some. The webrev points to patch
> 
>   http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-
>   NPE/16/enable_NPE_message.patch
> 
>   that enables the change by default,  which was useful for
> testing to
> 
>   assure the code is used in the tests.
> 
>   I just pushed the change to jdk/submit once more.
> 
> 
> 
>   Please review.
> 
> 
> 
>   Thanks!
> 
> Goetz.
> 
> 
> 

Re: RFR(s): 8228580: DnsClient TCP socket timeout

[Milan Mimica]

Hi Pavel

Wow, I find this awesome. I don't have a Windows machine to play with,
but I think I may have found something.
The difference is how Java_sun_nio_ch_Net_poll is implemented. On unix
it uses poll(2), on Windows it uses select(2). Regarding timeouts,
poll() has "wait at least" semantics and overruns by design[1], while
select() on windows has "waits at most" semantics, or how they put
it[2]: "specifies the maximum time that select should wait before
returning.". It returns early by design! Is this a known thing? I
don't think there is much one can do here. It probably makes no sense
to loop it and wait for time remainder.
Java's soTimeout does not specify[3] should it wait at least or at
most the specified timeout, so it's fine I guess. Old, "plain" socket
impl are not much different.

If the above is correct, should I just add a tolerance for the lower bound?

[1] http://man7.org/linux/man-pages/man2/poll.2.html
[2] 
https://docs.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-select
[3] 
https://docs.oracle.com/javase/9/docs/api/java/net/Socket.html#setSoTimeout-int-

On Mon, 23 Sep 2019 at 16:15, Pavel Rappo  wrote:
>
> Milan,
>
> I'm observing the latest version (.04) of this test failing quite frequently 
> (4/100) on Windows (Windows Server 2012 R2 6.3 (amd64)) machines. The test 
> passes fine on macOS and Linux. Here's the typical output I see in the logs:
>
> java.lang.RuntimeException: Query took 4997 ms. . Timeout value is 5000
> java.lang.RuntimeException: Query took 4999 ms. . Timeout value is 5000
> java.lang.RuntimeException: Query took 4995 ms. . Timeout value is 5000
> java.lang.RuntimeException: Query took 4998 ms. . Timeout value is 5000
> ...
>
> Now, there might be many reasons for that. One of which would be that the 
> DnsClient code is buggy. The other reason would be that the accuracy 
> guaranteed by Windows implementation of `read` is not what we would expect. 
> Would you be able to investigate that?
>
> P.S. The good news is that the CSR has been approved:
>
> https://bugs.openjdk.java.net/browse/JDK-8230965
>
> > On 23 Sep 2019, at 14:20, Milan Mimica  wrote:
> >
> > Got it. Thanks Pavel!
> >
> >
> > On Mon, 23 Sep 2019 at 13:37, Pavel Rappo  wrote:
> >>
> >> Milan,
> >>
> >> How do you check which tests are run? That's what I see in the 
> >> /test-support/jtreg_open_test_jdk_com_sun_jndi_dns_ConfigTests_TcpTimeout_java/com/sun/jndi/dns/ConfigTests/TcpTimeout.jtr
> >>  file after I have run the test locally on my machine:
> >>
> >> --messages:(5/233)--
> >> command: main TcpTimeout
> >> reason: User specified action: run main TcpTimeout
> >> Mode: othervm
> >> Additional options from @modules: --add-modules java.base --add-exports 
> >> java.base/sun.security.util=ALL-UNNAMED
> >> elapsed time (seconds): 1.751
> >>
> >> ...
> >>
> >> --messages:(5/313)--
> >> command: main TcpTimeout -Dcom.sun.jndi.dns.timeout.initial=5000
> >> reason: User specified action: run main TcpTimeout 
> >> -Dcom.sun.jndi.dns.timeout.initial=5000
> >> Mode: othervm
> >> Additional options from @modules: --add-modules java.base --add-exports 
> >> java.base/sun.security.util=ALL-UNNAMED
> >> elapsed time (seconds): 5.498
> >>
> >> 
> >>
> >> Which is consistent with what I would expect given the timeout values.
> >>
> >> The following output does not tell the full story, just the name of the 
> >> test:
> >>
> >> ==
> >> Test summary
> >> ==
> >>   TEST  TOTAL  PASS  FAIL ERROR
> >>   jtreg:open/test/jdk/com/sun/jndi/dns/ConfigTests/TcpTimeout.java
> >> 1 1 0 0
> >> ==
> >> TEST SUCCESS
> >>
> >> -Pavel
> >>
> >>> On 20 Sep 2019, at 15:42, Milan Mimica  wrote:
> >>>
> >>> Pavel,
> >>>
> >>> Here it is: http://cr.openjdk.java.net/~mmimica/8228580/webrev.04/
> >>> I don't see the test is run twice when I execute "make test
> >>> TEST=jtreg:test/jdk/com/sun/jndi/dns/ConfigTests/TcpTimeout.java". Am
> >>> I missing something?
> >>>
> >
> >
> > --
> > Milan Mimica
>


-- 
Milan Mimica

RE: RFR (L, final): 8218626: Add detailed message to NullPointerException describing what is null.

[Lindenmaier, Goetz]

Hi Remi, 

thanks for the heads up, I incorporated it in the 
main webrev:
http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/19/

Best regards,
  Goetz.

> -Original Message-
> From: fo...@univ-mlv.fr 
> Sent: Montag, 23. September 2019 18:02
> To: Lindenmaier, Goetz 
> Cc: hotspot-runtime-dev ; core-libs-
> dev 
> Subject: Re: RFR (L, final): 8218626: Add detailed message to
> NullPointerException describing what is null.
> 
> - Mail original -
> > De: "Goetz Lindenmaier" 
> > À: "Remi Forax" 
> > Cc: "hotspot-runtime-dev" , "core-
> libs-dev" 
> > Envoyé: Lundi 23 Septembre 2019 12:03:30
> > Objet: RE: RFR (L, final): 8218626: Add detailed message to
> NullPointerException describing what is null.
> 
> > Hi Remi,
> 
> Hi Goetz,
> 
> >
> > what do you think about dealing with the problem like this:
> > http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/18-
> obfuscation/
> > It's at the cost of one 64-bit field per bytecode in the analysis data.
> > Also, if there is a real assignment to a parameter it's not named 
> > 'parameteri'
> > after that any more.  See the example in the test.
> >
> 
> yes, it's a nice approximation, having a method with more than 63/64
> parameters is rare.
> 
> patch looks good, thumbs up !
> 
> > Best regards,
> >  Goetz.
> >
> 
> regards,
> Rémi
> 
> >
> >
> >> -Original Message-
> >> From: fo...@univ-mlv.fr 
> >> Sent: Freitag, 20. September 2019 00:01
> >> To: Lindenmaier, Goetz 
> >> Cc: hotspot-runtime-dev ; core-
> libs-
> >> dev 
> >> Subject: Re: RFR (L, final): 8218626: Add detailed message to
> >> NullPointerException describing what is null.
> >>
> >> - Mail original -
> >> > De: "Goetz Lindenmaier" 
> >> > À: "Remi Forax" 
> >> > Cc: "hotspot-runtime-dev" ,
> "core-
> >> libs-dev" 
> >> > Envoyé: Mercredi 18 Septembre 2019 09:37:36
> >> > Objet: RE: RFR (L, final): 8218626: Add detailed message to
> >> NullPointerException describing what is null.
> >>
> >> > Hi Remi,
> >>
> >> Hi Goetz,
> >>
> >> >
> >> >> in bytecodeUtils.cpp, in print_local_var(),
> >> >> i believe that the code
> >> >>   if (!method->is_static() && (slot == 0)) {
> >> >>   os->print("this");
> >> >>   }
> >> >>   ...
> >> >> is only true if the bytecode is generated by javac and ecj, tools like
> proguard
> >> >> that tries to obfuscate the code will reuse the slot 0 once "this" is 
> >> >> not
> >> needed
> >> >> anymore.
> >> >> This is obviously also true for any other parameters, so in my opinion,
> you
> >> >> should not try to be too heroic here and always display "local%d".
> >> > Yes, you are right. I assume the bytecode local slots are mapped 1:1 to
> >> > the parameters and are not reused for other values.
> >> >
> >> >> The other solution is to propagate "this" and "parameter%d" during the
> >> static
> >> >> analysis, so "this" will become "local0" once a store_0 is seen.
> >> > It would be possible to spot the place where "this" is first overwritten.
> >> > For other parameters, this is not feasible, they are not read-only, so
> >> > stores don't indicate obfuscation.
> >>
> >> Java is not the only language to run on the Java plateform so try to detect
> >> "obfuscation" is again akind of shortcut.
> >>
> >> >
> >> > I could mark the whole method as 'obfuscated' once I see a store_0,
> >> > and then print "local" instead of "parameter" in all places.
> >> > This does not work for static methods, though. Nor for methods
> >> > where "this" is live to the end, but the parameter slots are
> >> > reused.
> >>
> >> and you can have a path with a store_0 and one without (the code is not
> >> linear).
> >>
> >> >
> >> > For obfuscated methods I would claim that printing "this" etc
> >> > is fine even if the slot is reused for another value.  It just
> >> > adds to the obfuscation!  But there might be code
> >> > that is just optimized and not meant to be obfuscated.
> >>
> >> I've used obfuscation as an example, but there are also valid case to reuse
> >> slots
> >> likeit will consume less stack memory if you are using a very small device.
> >>
> >> >
> >> > Best regards,
> >> >  Goetz.
> >>
> >> regards,
> >> Rémi
> >>
> >> >
> >> >
> >> >
> >> >
> >> >
> >> >
> >> >>
> >> >> Rémi
> >> >>
> >> >> - Mail original -
> >> >> > De: "Goetz Lindenmaier" 
> >> >> > À: "hotspot-runtime-dev" ,
> >> >> "core-libs-dev" 
> >> >> > Envoyé: Mardi 17 Septembre 2019 16:18:03
> >> >> > Objet: RE: RFR (L, final): 8218626: Add detailed message to
> >> >> NullPointerException describing what is null.
> >> >>
> >> >> > @core-libs experts, I would appreciate comments on the changes
> >> >> > to NullPointerException.java, especially wrt. the Javadoc comment.
> >> >> > The change there is S.
> >> >> >
> >> >> > Best regards,
> >> >> >  Goetz.
> >> >> >
> >> >> >> -Original Message-
> >> >> >> From: Lindenmaier, Goetz
> >> >> >> Sent: Dienstag, 10. September 2019 11:48
> >> >> >> To: 'Hotspot dev runtime' ;
> >> >> Java Core
> >> >> >> Libs 
> >>