RE: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument

[Langer, Christoph]

Hi Serguei,

thanks for the review.

The patch successfully ran through our nightly test system which covers all 
these tests on several platforms.

Best regards
Christoph

> -Original Message-
> From: serguei.spit...@oracle.com 
> Sent: Mittwoch, 20. November 2019 03:21
> To: Langer, Christoph ; core-libs-
> d...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net; gerard
> ziemski 
> Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between
> libjava, hotspot and libinstrument
> 
> Hi Christoph,
> 
> The fix looks good to me.
> I'd recommend to run the jdk_instrument and vmTestbase_nsk_jvmti tests.
> Also, it can be safe to run:
>    open/test/hotspot/jtreg/serviceability/jvmti
>    open/test/hotspot/jtreg/runtime/cds/appcds
>    open/test/hotspot/jtreg/runtime/BootClassAppendProp
> 
> Thanks,
> Serguei
> 
> On 11/14/19 07:37, Langer, Christoph wrote:
> > Hi,
> >
> > please review this cleanup change regarding function "canonicalize" of
> libjava.
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8234185
> > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234185.0/
> >
> >
> > The goal is to cleanup how this function is defined and used. One thing is,
> that there was an unnecessary wrapper function "Canonicalize" in jni_util.c.
> It wrapped the call to "canonicalize". We can get rid of this wrapper.
> Unfortunately, it is not possible to just export "canonicalize" since this 
> will
> conflict with a method signature from the math library, at least on modern
> Linuxes. So I decided to call the method JDK_Canonicalize and will correctly
> define it in jdk_util.h which can be included everywhere.
> >
> >
> >
> > Hotspot's classloader.cpp will dynamically resolve this method, so I add a
> local declaration of the function pointer in there.
> >
> >
> >
> > This change shall be predecessor of JDK-8223261, where a review was
> already started here: https://mail.openjdk.java.net/pipermail/core-libs-
> dev/2019-November/063398.html
> >
> > Thanks
> > Christoph
> >

Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument

[serguei.spit...@oracle.com]

Hi Christoph,

The fix looks good to me.
I'd recommend to run the jdk_instrument and vmTestbase_nsk_jvmti tests.
Also, it can be safe to run:
  open/test/hotspot/jtreg/serviceability/jvmti
  open/test/hotspot/jtreg/runtime/cds/appcds
  open/test/hotspot/jtreg/runtime/BootClassAppendProp

Thanks,
Serguei

On 11/14/19 07:37, Langer, Christoph wrote:

Hi,

please review this cleanup change regarding function "canonicalize" of libjava.

Bug: https://bugs.openjdk.java.net/browse/JDK-8234185
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234185.0/


The goal is to cleanup how this function is defined and used. One thing is, that there was an unnecessary 
wrapper function "Canonicalize" in jni_util.c. It wrapped the call to "canonicalize". We 
can get rid of this wrapper. Unfortunately, it is not possible to just export "canonicalize" since 
this will conflict with a method signature from the math library, at least on modern Linuxes. So I decided to 
call the method JDK_Canonicalize and will correctly define it in jdk_util.h which can be included everywhere.



Hotspot's classloader.cpp will dynamically resolve this method, so I add a 
local declaration of the function pointer in there.



This change shall be predecessor of JDK-8223261, where a review was already 
started here: 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-November/063398.html

Thanks
Christoph

Re: RFR: JDK-8234402: revert change that stopped providing JPackageToolProvider

[Kevin Rushforth]

Good point. Looks good to me once this is fixed.

-- Kevin


On 11/19/2019 6:00 PM, Alexey Semenyuk wrote:

Andy,

I guess 
http://cr.openjdk.java.net/~herrick/8234402/webrev.02/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JavaTool.java.sdiff.html 
can be reverted to its initial state now:

---
public ToolProvider asToolProvider() {
    return ToolProvider.findFirst(name).orElse(null);
}
---

- Alexey

On 11/19/2019 7:00 PM, 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 restores JPackageToolProvider and gets rid of the 
temporary factory class.


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

[2] http://cr.openjdk.java.net/~herrick/8234402/webrev.02/

/Andy

Re: RFR: JDK-8234402: revert change that stopped providing JPackageToolProvider

[Alexey Semenyuk]

Andy,

I guess 
http://cr.openjdk.java.net/~herrick/8234402/webrev.02/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JavaTool.java.sdiff.html 
can be reverted to its initial state now:

---
public ToolProvider asToolProvider() {
    return ToolProvider.findFirst(name).orElse(null);
}
---

- Alexey

On 11/19/2019 7:00 PM, 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 restores JPackageToolProvider and gets rid of the 
temporary factory class.


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

[2] http://cr.openjdk.java.net/~herrick/8234402/webrev.02/

/Andy

Re: RFR: JDK-8234402: revert change that stopped providing JPackageToolProvider

[Alexander Matveev]

Looks good.

On 11/19/19 4:00 PM, 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 restores JPackageToolProvider and gets rid of the 
temporary factory class.


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

[2] http://cr.openjdk.java.net/~herrick/8234402/webrev.02/

/Andy

RFR: JDK-8234402: revert change that stopped providing JPackageToolProvider

[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 restores JPackageToolProvider and gets rid of the temporary 
factory class.


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

[2] http://cr.openjdk.java.net/~herrick/8234402/webrev.02/

/Andy

Re: Class-Path (in jar file) semantics different between Java 11 and 13 (on Windows)?

[Brent Christian]

On 11/18/19 7:36 AM, Alan Bateman wrote:
>
Yes, bad values are now ignored, bringing an end to an effort on the 
run-time over multiple releases to fix the problems this area. 
JDK-8224253[1] is the JDK 13 release note to announce this change and I 
see you've found the system property that you can use to track down bad 
values. In previous releases you will get the same behavior with 
-Djdk.net.URLClassPath.disableClassPathURLCheck=false as the changes to 
reject bad input have been in place for some time.


Right, the "jdk.net.URLClassPath.disableClassPathURLCheck" system 
property can be used to once again allow bad values (though I believe it 
must be set to "true").


Brent can summarize 
but I think the only outstanding work is to fix the javac handling.


That's a good summary. :)  It's the only planned change I'm aware of.

It should be expected, however, that the disableClassPathURLCheck system 
property will be removed in a future release.


-Brent

Re: Class-Path (in jar file) semantics different between Java 11 and 13 (on Windows)?

[David Lloyd]

On Mon, Nov 18, 2019 at 9:37 AM Alan Bateman  wrote:
>
> On 18/11/2019 15:01, Jaikiran Pai wrote:
> > :
> >
> > So this Class-Path entry is being ignored starting Java 13.
> >
> Yes, bad values are now ignored, bringing an end to an effort on the
> run-time over multiple releases to fix the problems this area.
> JDK-8224253[1] is the JDK 13 release note to announce this change and I
> see you've found the system property that you can use to track down bad
> values. In previous releases you will get the same behavior with
> -Djdk.net.URLClassPath.disableClassPathURLCheck=false as the changes to
> reject bad input have been in place for some time. Brent can summarize
> but I think the only outstanding work is to fix the javac handling.

OK, having read the updated specification (thanks Alan!) I'm now quite
curious why `/C:/helloworld.jar` is considered invalid.  It is in fact
a valid relative URL (colons are allowed in path segments, and the
leading `/` unambiguously delineates the URL path), and thus it seems
that it should be considered valid.

Examining the code in URLClassPath:

static URL tryResolveFile(URL base, String input) throws MalformedURLException {
int index = input.indexOf(':'); // <-- A
boolean isFile;
if (index >= 0) {
String scheme = input.substring(0, index);
isFile = "file".equalsIgnoreCase(scheme);
} else {
isFile = true;
}
return (isFile) ? new URL(base, input) : null;
}

The line marked as `A` seems to incorrectly detect URL scheme.  If a
`/` appears before the `:` then the `:` is legitimately part of the
path, not a scheme delimiter, however this code does not check to see
if the `:` appears after a `/`.
-- 
- DML

Re: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem

[Lance Andersen]

Hi Christoph,

Thank you for the follow up.
> On Nov 19, 2019, at 5:37 PM, Langer, Christoph  
> wrote:
> 
> ModulesInCustomFileSystem

At line 71:

—
/**
 * Test exploded modules in a JAR file system.
 */
———

I will look at the rest of the changes tomorrow

Best
Lance

 
  

 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: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem

[Langer, Christoph]

Hi Lance,

> If you look at MultiReleaseJarTest.java, it explicitly references JAR FS in a
> comment. Same for JFSTester.java in the @Summary.  These should
> definitely be updated.  There are comments
> in ModulesInCustomFileSystem.java as well.

Ok, agreed for MultiReleaseJarTest and JFSTester. I fixed the comments in 
there. In ModulesInCustomFileSystem I can't see comments that explicitly refer 
to JarFileSystem.

> I would suggesting moving the code added to the constructor for checking
> the releaseVersion/multi-release properties to a method and grouping it
> with the other methods added for supporting MR jars around line 1396.
> (keeps it easier to follow and maintain)
> 
> Done that.
> 
> Thank you.  I do think however we need to change the name of the method
> to perhaps “checkReleaseVersionProperty” as the current name
> “initializeMultiReleaseJar”  does not seem quite right to me.

Ok, I changed it to " initializeReleaseVersion". I didn't follow your 
suggestion because the method not only checks the release version property but 
also triggers some initialization if a version property is found and it's an 
mr-jar.

> I would also update the comment for the method to something like:
> 
> 
> Checks to see if the Zip File System property  “releaseVersion” has been
> specified.  If it has not been specified then check for the existence of the
> “multi-release” property.   If set and the file represents a multi-release 
> JAR,
> determine the runtime version to use.
> 

I updated the comment, trying to incorporate your suggestion. How do you like 
it?

> Could you also add a comment above the isMultiReleaseJar method to for
> clarity going forward (I know there was not one there before)
> 
> Done, too.
> 
> Thank you.
> 
> I think we can tweak the comment slightly to something like
> 
> ———
> Returns true if the Manifest main attribute “Multi-Release” is set to true;
> false otherwise
> 

Done.

> I might change the name of the lookup field in the method makeParentDirs
> with the addition of the Function lookup on line 126.
> 
> I chose to change the name of the global field in line 126 to be named
> mrlookup. Also updated the comment.
> 
> I am not sure “mrlookup” is the best name as this field is used with or
> without a multi-release jar.  Perhaps “IndexNodeNameLookup” or
> “entryLookup"

Ok, entryLookup seems the best fit to me. Changed that.

http://cr.openjdk.java.net/~clanger/webrevs/8234089.3/

Thanks again for reviewing.

Cheers
Christoph

Re: JDK14 RFR of JDK-8234381: API docs should mention special handling of enums in serialization

[Roger Riggs]

Hi Joe,

The clarification looks fine to me.

Roger


On 11/19/19 1:21 PM, Joe Darcy wrote:

Hello,

Please review this small doc changes to more explicitly discuss the 
special handling of enum types by serialization:


    JDK-8234381: API docs should mention special handling of enums in 
serialization

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

Patch below; thanks,

-Joe

--- old/src/java.base/share/classes/java/io/Serializable.java 
2019-11-19 10:17:45.803443000 -0800
+++ new/src/java.base/share/classes/java/io/Serializable.java 
2019-11-19 10:17:45.399241001 -0800

@@ -134,6 +134,11 @@
  * This readResolve method follows the same invocation rules and
  * accessibility rules as writeReplace.
  *
+ * Enum types are all serializable and receive treatment defined by
+ * the Java Object Serialization Specification during
+ * serialization and deserialization. Any declarations of the special
+ * handling methods discussed above are ignored for enum types.
+ *
  * The serialization runtime associates with each serializable class 
a version
  * number, called a serialVersionUID, which is used during 
deserialization to
  * verify that the sender and receiver of a serialized object have 
loaded

@@ -152,8 +157,9 @@
  * If a serializable class does not explicitly declare a 
serialVersionUID, then
  * the serialization runtime will calculate a default 
serialVersionUID value
  * for that class based on various aspects of the class, as described 
in the
- * Java(TM) Object Serialization Specification.  However, it is 
strongly

- * recommended that all serializable classes explicitly declare
+ * Java Object Serialization Specification.  This specification 
defines the
+ * serialVersionUID of an enum type to be 0L. However, it is 
strongly
+ * recommended that all serializable classes other than enum 
types explicitly declare
  * serialVersionUID values, since the default serialVersionUID 
computation is

  * highly sensitive to class details that may vary depending on compiler
  * implementations, and can thus result in unexpected
--- old/src/java.base/share/classes/java/lang/Enum.java 2019-11-19 
10:17:46.807945000 -0800
+++ new/src/java.base/share/classes/java/lang/Enum.java 2019-11-19 
10:17:46.371727000 -0800

@@ -47,6 +47,13 @@
  * found in section 8.9 of
  * The Java Language Specification.
  *
+ * Enumeration types are all serializable and receive special handling
+ * by the serialization mechanism. The serialized representation used
+ * for enum constants cannot be customized. Declarations of methods
+ * and fields that would otherwise interact with serialization are
+ * ignored, including {@code serialVersionUID}; see the Java
+ * Object Serialization Specification for details.
+ *
  *  Note that when using an enumeration type as the type of a set
  * or as the type of the keys in a map, specialized and efficient
  * {@linkplain java.util.EnumSet set} and {@linkplain

Re: RFR: 8234335: Remove line break in class declaration in java.base

[mark . reinhold]

2019/11/19 11:17:22 -0800, john.r.r...@oracle.com:
> On Nov 19, 2019, at 4:00 AM, Lance Andersen  wrote:
>> Seems to be a “your milage varies”.  I am fine with whatever the
>> final decision is.  However, I do believe the comment above reads
>> better and aligns the methods better.
> 
> FWIW, and as author of many of the lines being changed, I prefer that
> comment on a separate from the actual modifiers.  I think the basic
> modifier patterns are easier to recognize quickly when comments and
> annotations are separate.

Personally I prefer to place such pseudo-modifiers on the same line
as the actual modifiers, but I accept that some prefer otherwise.

When in doubt, respect the original author’s choice.

- Mark

Re: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem

[Lance Andersen]

Hi Christoph,

If you look at MultiReleaseJarTest.java, it explicitly references JAR FS in a 
comment. Same for JFSTester.java in the @Summary.  These should definitely be 
updated.  There are comments in ModulesInCustomFileSystem.java as well.

As far as variable names, I think this is nice to clean up but less important 
as part of this update.

The good news is it is not a big set of changes for the comments so I would 
address those at a minimum.

Best
Lance

> On Nov 19, 2019, at 4:03 PM, Langer, Christoph  
> wrote:
> 
> Hi Lance,
>  
> I’m actually not so sure about this. While my cleanup change would remove the 
> implementation detail of zipfs to use a class named “JarFileSystem” for 
> multi-release jar peculiarities, I think a user of a FileSystem object upon a 
> jar file is not wrong if he names the variable like jarFileSystem or the 
> like. So I don’t see that such cleanup is really worth the while. Would you 
> be ok with that?
>  
> I’ll get back to you soon with an updated webrev regarding the other changes 
> you suggested.
>  
> Best regards
> Christoph
>  
> From: Lance Andersen  > 
> Sent: Dienstag, 19. November 2019 19:57
> To: Langer, Christoph  >
> Cc: core-libs-dev@openjdk.java.net ; 
> nio-dev mailto:nio-...@openjdk.java.net>>
> Subject: Re: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and 
> JarFileSystem
>  
> Hi Christoph,
>  
> Something else that should probably be done as part of this proposed change 
> is  to clean up tests such as NodeCostDumpUtil.java and 
> ModulesInCustomFileSystem.java and a few others as they have methods/fields 
> etc... that make reference to the  Jar File System.  While it does not impact 
> the tests as they are currently written, I think it would make sense to do 
> this for clarity with the change you are making.
>  
> Best
> Lance
> On Nov 18, 2019, at 12:53 PM, Lance Andersen  > wrote:
>  
> Hi Christoph,
>  
> Sorry for the late reply but I was out of the office Friday
> On Nov 15, 2019, at 3:40 AM, Langer, Christoph  > wrote:
>  
> Hi Lance,
> 
> thanks for reviewing. I've addressed your points:
> 
> 
> I would suggesting moving the code added to the constructor for checking
> the releaseVersion/multi-release properties to a method and grouping it
> with the other methods added for supporting MR jars around line 1396.
> (keeps it easier to follow and maintain)
> 
> Done that.
>  
> Thank you.  I do think however we need to change the name of the method to 
> perhaps “checkReleaseVersionProperty” as the current name 
> “initializeMultiReleaseJar”  does not seem quite right to me.
>  
> I would also update the comment for the method to something like:
>  
> 
> Checks to see if the Zip File System property  “releaseVersion” has been 
> specified.  If it has not been specified then check for the existence of the 
> “multi-release” property.   If set and the file represents a multi-release 
> JAR, determine the runtime version to use.
> 
>  
> 
> 
> Could you also add a comment above the isMultiReleaseJar method to for
> clarity going forward (I know there was not one there before)
> 
> Done, too.
>  
> Thank you.
>  
> I think we can tweak the comment slightly to something like
>  
> ———
> Returns true if the Manifest main attribute “Multi-Release” is set to true; 
> false otherwise
> 
> 
> 
> 
> I might change the name of the lookup field in the method makeParentDirs
> with the addition of the Function lookup on line 126.
> 
> I chose to change the name of the global field in line 126 to be named 
> mrlookup. Also updated the comment.
>  
> I am not sure “mrlookup” is the best name as this field is used with or 
> without a multi-release jar.  Perhaps “IndexNodeNameLookup” or
> “entryLookup"
> 
> 
> This is the new webrev: 
> http://cr.openjdk.java.net/~clanger/webrevs/8234089.2/ 
> 
> 
> Cheers
> Christoph
>  
> Thank you again!
>  
> Best
> Lance
> 
>  
>  
>  
> 
> 
>  Lance Andersen| 
> Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering 
> 1 Network Drive 
> Burlington, MA 01803
> lance.ander...@oracle.com 
>  
>  
> 
>  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: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem

[Langer, Christoph]

Hi Lance,

I’m actually not so sure about this. While my cleanup change would remove the 
implementation detail of zipfs to use a class named “JarFileSystem” for 
multi-release jar peculiarities, I think a user of a FileSystem object upon a 
jar file is not wrong if he names the variable like jarFileSystem or the like. 
So I don’t see that such cleanup is really worth the while. Would you be ok 
with that?

I’ll get back to you soon with an updated webrev regarding the other changes 
you suggested.

Best regards
Christoph

From: Lance Andersen 
Sent: Dienstag, 19. November 2019 19:57
To: Langer, Christoph 
Cc: core-libs-dev@openjdk.java.net; nio-dev 
Subject: Re: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and 
JarFileSystem

Hi Christoph,

Something else that should probably be done as part of this proposed change is  
to clean up tests such as NodeCostDumpUtil.java and 
ModulesInCustomFileSystem.java and a few others as they have methods/fields 
etc... that make reference to the  Jar File System.  While it does not impact 
the tests as they are currently written, I think it would make sense to do this 
for clarity with the change you are making.

Best
Lance
On Nov 18, 2019, at 12:53 PM, Lance Andersen 
mailto:lance.ander...@oracle.com>> wrote:

Hi Christoph,

Sorry for the late reply but I was out of the office Friday
On Nov 15, 2019, at 3:40 AM, Langer, Christoph 
mailto:christoph.lan...@sap.com>> wrote:

Hi Lance,

thanks for reviewing. I've addressed your points:


I would suggesting moving the code added to the constructor for checking
the releaseVersion/multi-release properties to a method and grouping it
with the other methods added for supporting MR jars around line 1396.
(keeps it easier to follow and maintain)

Done that.

Thank you.  I do think however we need to change the name of the method to 
perhaps “checkReleaseVersionProperty” as the current name 
“initializeMultiReleaseJar”  does not seem quite right to me.

I would also update the comment for the method to something like:


Checks to see if the Zip File System property  “releaseVersion” has been 
specified.  If it has not been specified then check for the existence of the 
“multi-release” property.   If set and the file represents a multi-release JAR, 
determine the runtime version to use.




Could you also add a comment above the isMultiReleaseJar method to for
clarity going forward (I know there was not one there before)

Done, too.

Thank you.

I think we can tweak the comment slightly to something like

———
Returns true if the Manifest main attribute “Multi-Release” is set to true; 
false otherwise




I might change the name of the lookup field in the method makeParentDirs
with the addition of the Function lookup on line 126.

I chose to change the name of the global field in line 126 to be named 
mrlookup. Also updated the comment.

I am not sure “mrlookup” is the best name as this field is used with or without 
a multi-release jar.  Perhaps “IndexNodeNameLookup” or
“entryLookup"


This is the new webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234089.2/

Cheers
Christoph

Thank you again!

Best
Lance





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

[cid:image001.gif@01D59F25.2712D690]

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: Class-Path (in jar file) semantics different between Java 11 and 13 (on Windows)?

[Alan Bateman]

On 19/11/2019 20:22, David Lloyd wrote:

:
Where can the updated specification be found?  It has in the past been
clearly specified and well understood that class path entries are
interpreted as relative URLs.  If that has changed then this will
definitely break Quarkus and perhaps other applications as well.

A Google search for "JAR file specification" only seems to turn up
older versions (JDK 10 and earlier).
One way to quickly get to it is type "java.util.jar" into the javadoc 
search box and it is linked from the package description. So for the JDK 
13 published on jdk.java.net it is:

https://docs.oracle.com/en/java/javase/13/docs/specs/jar/jar.html#class-path-attribute

The value is a relative URL, same as always, it's just that the 
implementation is less tolerant of bad input. The stricter checking has 
been in place for a few releases, controlled by the system property 
mentioned in the discussion here.


It's always hard to dial up checking after the fact and one of the main 
challenges has been mis-uses where tools have been putting file paths 
(rather than URL) or absolute URLs into the value. This is the reason 
why the spec now allows absolute file URLs (need to specify the "file:" 
scheme).


-Alan

Re: Class-Path (in jar file) semantics different between Java 11 and 13 (on Windows)?

[Lance Andersen]

> On Nov 19, 2019, at 3:22 PM, David Lloyd  wrote:
> 
> On Mon, Nov 18, 2019 at 9:37 AM Alan Bateman  wrote:
>> 
>> On 18/11/2019 15:01, Jaikiran Pai wrote:
>>> :
>>> 
>>> So this Class-Path entry is being ignored starting Java 13.
>>> 
>> Yes, bad values are now ignored, bringing an end to an effort on the
>> run-time over multiple releases to fix the problems this area.
>> JDK-8224253[1] is the JDK 13 release note to announce this change and I
>> see you've found the system property that you can use to track down bad
>> values. In previous releases you will get the same behavior with
>> -Djdk.net.URLClassPath.disableClassPathURLCheck=false as the changes to
>> reject bad input have been in place for some time. Brent can summarize
>> but I think the only outstanding work is to fix the javac handling.
>> 
>> -Alan
>> [1] https://bugs.openjdk.java.net/browse/JDK-8224253
> 
> Where can the updated specification be found?  It has in the past been
> clearly specified and well understood that class path entries are
> interpreted as relative URLs.  If that has changed then this will
> definitely break Quarkus and perhaps other applications as well.
> 
> A Google search for "JAR file specification" only seems to turn up
> older versions (JDK 10 and earlier).

https://docs.oracle.com/en/java/javase/13/docs/specs/jar/jar.html

> -- 
> - DML
> 

 
  

 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: Class-Path (in jar file) semantics different between Java 11 and 13 (on Windows)?

[David Lloyd]

On Mon, Nov 18, 2019 at 9:37 AM Alan Bateman  wrote:
>
> On 18/11/2019 15:01, Jaikiran Pai wrote:
> > :
> >
> > So this Class-Path entry is being ignored starting Java 13.
> >
> Yes, bad values are now ignored, bringing an end to an effort on the
> run-time over multiple releases to fix the problems this area.
> JDK-8224253[1] is the JDK 13 release note to announce this change and I
> see you've found the system property that you can use to track down bad
> values. In previous releases you will get the same behavior with
> -Djdk.net.URLClassPath.disableClassPathURLCheck=false as the changes to
> reject bad input have been in place for some time. Brent can summarize
> but I think the only outstanding work is to fix the javac handling.
>
> -Alan
> [1] https://bugs.openjdk.java.net/browse/JDK-8224253

Where can the updated specification be found?  It has in the past been
clearly specified and well understood that class path entries are
interpreted as relative URLs.  If that has changed then this will
definitely break Quarkus and perhaps other applications as well.

A Google search for "JAR file specification" only seems to turn up
older versions (JDK 10 and earlier).
-- 
- DML

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

[Kevin Rushforth]

I took the "git diff" patch [5] that you uploaded yesterday, applied it, 
and verified that it is the same as what is in the JDK-8200758-branch 
branch of the sandbox. It builds and runs fine for me.


I ran jcheck and it found the following three files with whitespace 
errors that will need to be fixed before you push:


src/jdk.incubator.jpackage/linux/classes/jdk/incubator/jpackage/internal/PackageProperty.java:49: 
Trailing whitespace
src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/ToolProviderFactory.java:35: 
Trailing whitespace

test/jdk/tools/jpackage/share/AddLauncherBase.java:137: Trailing whitespace

The second of these will go away with the fix for JDK-8234402 [6], so 
you don't need to do anything there. Once the fix for JDK-8234402 is 
pushed to sandbox, I presume you will update the webrev, right?


Pending the fix for JDK-8234402 and the needed white-space fixes, this 
is a +1 from me (although I am not a jdk Project Reviewer, so you will 
need at least one review from someone who is).


-- Kevin

[5] http://cr.openjdk.java.net/~herrick/8212780/JDK-EA-10.git.patch
[6] https://bugs.openjdk.java.net/browse/JDK-8234402


On 11/13/2019 4:23 PM, Andy Herrick wrote:
Please review  changes for [1] which is the implementation bug for 
JEP-343.


The webrev at [2] is the total cumulative webrev of changes for the 
jpackage tool, currently in the JDK-8200758-branch branch of the open 
sandbox repository.


The webrev at [3] shows the changes since EA-06 (Build 
13-jpackage+1-49 ) up to the current point.


The latest EA (Build 14-jpackage+1-49 ) is posted at [4].

Please send feedback to core-libs-dev@openjdk.java.net


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

[2] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-10/

[3] http://cr.openjdk.java.net/~herrick/8212780/webrev.06-10/

[4] http://jdk.java.net/jpackage/

Re: RFR: JDK-8233117 Escape Sequences For Line Continuation and White Space (Preview)

[Brent Christian]

On 11/18/19 6:26 AM, Jim Laskey wrote:


http://cr.openjdk.java.net/~jlaskey/8233116/webrev.04/index.html



OK, thanks.

The changes in:
  src/java.base/share/classes/java/lang/String.java
  test/jdk/java/lang/String/TranslateEscapes.java
look fine.

-Brent

Re: RFR: 8234335: Remove line break in class declaration in java.base

[John Rose]

On Nov 19, 2019, at 4:00 AM, Lance Andersen  wrote:
> 
> Seems to be a “your milage varies”.  I am fine with whatever the final 
> decision is.  However, I do believe the comment above reads better and aligns 
> the methods better.

FWIW, and as author of many of the lines being changed, I prefer that comment 
on a separate from the actual modifiers.  I think the basic modifier patterns 
are easier to recognize quickly when comments and annotations are separate.

— John

P.S. And for the record, my intention of writing non-public is to ensure make 
it clear to maintainers that the absence of any modifier is both intentional 
and important (to security, in most cases).  The language doesn’t give any more 
explicit way to document that default access is required.

Re: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem

[Lance Andersen]

Hi Christoph,

Something else that should probably be done as part of this proposed change is  
to clean up tests such as NodeCostDumpUtil.java and 
ModulesInCustomFileSystem.java and a few others as they have methods/fields 
etc... that make reference to the  Jar File System.  While it does not impact 
the tests as they are currently written, I think it would make sense to do this 
for clarity with the change you are making.

Best
Lance
> On Nov 18, 2019, at 12:53 PM, Lance Andersen  
> wrote:
> 
> Hi Christoph,
> 
> Sorry for the late reply but I was out of the office Friday
>> On Nov 15, 2019, at 3:40 AM, Langer, Christoph > > wrote:
>> 
>> Hi Lance,
>> 
>> thanks for reviewing. I've addressed your points:
>> 
>>> I would suggesting moving the code added to the constructor for checking
>>> the releaseVersion/multi-release properties to a method and grouping it
>>> with the other methods added for supporting MR jars around line 1396.
>>> (keeps it easier to follow and maintain)
>> 
>> Done that.
> 
> Thank you.  I do think however we need to change the name of the method to 
> perhaps “checkReleaseVersionProperty” as the current name 
> “initializeMultiReleaseJar”  does not seem quite right to me.
> 
> I would also update the comment for the method to something like:
> 
> 
> Checks to see if the Zip File System property  “releaseVersion” has been 
> specified.  If it has not been specified then check for the existence of the 
> “multi-release” property.   If set and the file represents a multi-release 
> JAR, determine the runtime version to use.
> 
> 
>> 
>>> Could you also add a comment above the isMultiReleaseJar method to for
>>> clarity going forward (I know there was not one there before)
>> 
>> Done, too.
> 
> Thank you.
> 
> I think we can tweak the comment slightly to something like
> 
> ———
> Returns true if the Manifest main attribute “Multi-Release” is set to true; 
> false otherwise
> 
>> 
>>> I might change the name of the lookup field in the method makeParentDirs
>>> with the addition of the Function lookup on line 126.
>> 
>> I chose to change the name of the global field in line 126 to be named 
>> mrlookup. Also updated the comment.
> 
> I am not sure “mrlookup” is the best name as this field is used with or 
> without a multi-release jar.  Perhaps “IndexNodeNameLookup” or
> “entryLookup"
>> 
>> This is the new webrev: 
>> http://cr.openjdk.java.net/~clanger/webrevs/8234089.2/ 
>> 
>> 
>> Cheers
>> Christoph
> 
> Thank you again!
> 
> Best
> Lance
>> 
> 
>  
> 
>   
> 
>  Lance Andersen| 
> Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering 
> 1 Network Drive 
> Burlington, MA 01803
> lance.ander...@oracle.com 
 
  

 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: JEP 358 (Helpful NPEs) and single source file mode

[Jonathan Gibbons]

It's a design constraint that the "single source file mode" does not and 
should not support javac-specific command-line options beyond those that 
are also runtime options (for example, --class-path).


That being said, since one of the use cases is for beginners learning 
Java, it seems reasonable to support debugging information in that case. 
The other use case is for scripts/shebang files, and the debugging 
information should not be an onerous overhead in that case as well.


Therefore, I'm inclined to agree with the suggestion.

-- Jon

On 11/19/19 12:40 AM, Remi Forax wrote:

- Mail original -

De: "Gunnar Morling" 
À: "core-libs-dev" 
Envoyé: Mardi 19 Novembre 2019 09:34:41
Objet: JEP 358 (Helpful NPEs) and single source file mode
Hi,

I've been exploring the new helpful NPE feature a bit.

It's a very welcomed improvement, but I noticed one potential usability
issue in conjunction with the single source file mode (JEP 330): as debug
info is missing in that case, e.g. local variables are rendered as
"" in the exception message. Single source file mode also doesn't
let me specify the "-g:vars" option I could use with javac for adding the
debugging info.

Perhaps single source file mode should add debug information by default to
prevent this issue?

yes !


Thanks,

--Gunnar

Rémi

Re: [14] RFR (S): 8234401: ConstantCallSite may stuck in non-frozen state

[Paul Sandoz]

+1

> On Nov 19, 2019, at 10:12 AM, Vladimir Ivanov  
> wrote:
> 
> Thanks, Paul.
> 
>> CallSite:
>>  public class CallSite {
>>  - // The actual payload of this call site:
>> + // The actual payload of this call site.
>> + // Can be modified using {@link 
>> MethodHandleNatives#setCallSiteTargetNormal} or {@link 
>> MethodHandleNatives#setCallSiteTargetVolatile}.
>>  /*package-private*/
>> - MethodHandle target; // Note: This field is known to the JVM. Do not 
>> change.
>> + final MethodHandle target; // Note: This field is known to the JVM.
>> Is there any benefit to making target final, even though it's not really for 
>> mutable call sites? (With the recent discussion of "final means final" it 
>> would be nice to not introduce more special case stomping on final fields if 
>> we can avoid it).
> 
> CallSite.target is already treated specially: all updates go through 
> MethodHandleNatives and JIT-compiler treat it as "final" irrespective of the 
> flags.
> 
> My main interest in marking it final is to enforce proper initialization on 
> JDK side to make it easier to reason about:
> 
> https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2019-November/036021.html
>  
> 

Ok, I see now in light of that context.


>>  CallSite(MethodType targetType, MethodHandle createTargetHook) throws 
>> Throwable {
>> - this(targetType);
>> + this(targetType); // need to initialize target to make CallSite.type() 
>> work in createTargetHook
>>  ConstantCallSite selfCCS = (ConstantCallSite) this;
>>  MethodHandle boundTarget = (MethodHandle) 
>> createTargetHook.invokeWithArguments(selfCCS);
>> - checkTargetChange(this.target, boundTarget);
>> - this.target = boundTarget;
>> + setTargetNormal(boundTarget); // ConstantCallSite doesn't publish 
>> CallSite.target
>> + UNSAFE.storeStoreFence(); // barrier between target and isFrozen updates
>>  }
>> I wonder if instead of introducing the store store fence here we could move 
>> it into ConstantCallSite?
> 
> Sure, if you prefer to see it on ConstantCallSite side, we can move it there.
> 
> By putting it in CallSite near the call site update, I wanted to stress 
> there's a CallSite.target update happening on partially published instance.
> 

Up to you.

Paul.

JDK14 RFR of JDK-8234381: API docs should mention special handling of enums in serialization

[Joe Darcy]

Hello,

Please review this small doc changes to more explicitly discuss the 
special handling of enum types by serialization:


    JDK-8234381: API docs should mention special handling of enums in 
serialization

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

Patch below; thanks,

-Joe

--- old/src/java.base/share/classes/java/io/Serializable.java 2019-11-19 
10:17:45.803443000 -0800
+++ new/src/java.base/share/classes/java/io/Serializable.java 2019-11-19 
10:17:45.399241001 -0800

@@ -134,6 +134,11 @@
  * This readResolve method follows the same invocation rules and
  * accessibility rules as writeReplace.
  *
+ * Enum types are all serializable and receive treatment defined by
+ * the Java Object Serialization Specification during
+ * serialization and deserialization. Any declarations of the special
+ * handling methods discussed above are ignored for enum types.
+ *
  * The serialization runtime associates with each serializable class a 
version
  * number, called a serialVersionUID, which is used during 
deserialization to

  * verify that the sender and receiver of a serialized object have loaded
@@ -152,8 +157,9 @@
  * If a serializable class does not explicitly declare a 
serialVersionUID, then
  * the serialization runtime will calculate a default serialVersionUID 
value
  * for that class based on various aspects of the class, as described 
in the
- * Java(TM) Object Serialization Specification.  However, it is 
strongly

- * recommended that all serializable classes explicitly declare
+ * Java Object Serialization Specification.  This specification defines the
+ * serialVersionUID of an enum type to be 0L. However, it is strongly
+ * recommended that all serializable classes other than enum types 
explicitly declare
  * serialVersionUID values, since the default serialVersionUID 
computation is

  * highly sensitive to class details that may vary depending on compiler
  * implementations, and can thus result in unexpected
--- old/src/java.base/share/classes/java/lang/Enum.java 2019-11-19 
10:17:46.807945000 -0800
+++ new/src/java.base/share/classes/java/lang/Enum.java 2019-11-19 
10:17:46.371727000 -0800

@@ -47,6 +47,13 @@
  * found in section 8.9 of
  * The Java Language Specification.
  *
+ * Enumeration types are all serializable and receive special handling
+ * by the serialization mechanism. The serialized representation used
+ * for enum constants cannot be customized. Declarations of methods
+ * and fields that would otherwise interact with serialization are
+ * ignored, including {@code serialVersionUID}; see the Java
+ * Object Serialization Specification for details.
+ *
  *  Note that when using an enumeration type as the type of a set
  * or as the type of the keys in a map, specialized and efficient
  * {@linkplain java.util.EnumSet set} and {@linkplain

Re: [14] RFR (S): 8234401: ConstantCallSite may stuck in non-frozen state

[Vladimir Ivanov]

Thanks, Paul.


CallSite:

  public class CallSite {
  
- // The actual payload of this call site:

+ // The actual payload of this call site.
+ // Can be modified using {@link 
MethodHandleNatives#setCallSiteTargetNormal} or {@link 
MethodHandleNatives#setCallSiteTargetVolatile}.

  /*package-private*/
- MethodHandle target; // Note: This field is known to the JVM. Do not 
change.

+ final MethodHandle target; // Note: This field is known to the JVM.


Is there any benefit to making target final, even though it's not really 
for mutable call sites? (With the recent discussion of "final means 
final" it would be nice to not introduce more special case stomping on 
final fields if we can avoid it).


CallSite.target is already treated specially: all updates go through 
MethodHandleNatives and JIT-compiler treat it as "final" irrespective of 
the flags.


My main interest in marking it final is to enforce proper initialization 
on JDK side to make it easier to reason about:



https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2019-November/036021.html 




  CallSite(MethodType targetType, MethodHandle createTargetHook) throws 
Throwable {
- this(targetType);
+ this(targetType); // need to initialize target to make CallSite.type() 
work in createTargetHook

  ConstantCallSite selfCCS = (ConstantCallSite) this;
  MethodHandle boundTarget = (MethodHandle) 
createTargetHook.invokeWithArguments(selfCCS);
- checkTargetChange(this.target, boundTarget);
- this.target = boundTarget;
+ setTargetNormal(boundTarget); // ConstantCallSite doesn't publish 
CallSite.target

+ UNSAFE.storeStoreFence(); // barrier between target and isFrozen updates
  }


I wonder if instead of introducing the store store fence here we could 
move it into ConstantCallSite?


Sure, if you prefer to see it on ConstantCallSite side, we can move it 
there.


By putting it in CallSite near the call site update, I wanted to stress 
there's a CallSite.target update happening on partially published 
instance.


Best regards,
Vladimir Ivanov



  protected ConstantCallSite(MethodType targetType, MethodHandle 
createTargetHook) throws Throwable {
- super(targetType, createTargetHook);
+ super(targetType, createTargetHook); // "this" instance leaks into 
createTargetHook


+ UNSAFE.storeStoreFence(); // barrier between target and isFrozen 
updates<— Add here


  isFrozen = true;
+ UNSAFE.storeStoreFence(); // properly publish isFrozen
  }
On Nov 19, 2019, at 9:49 AM, Vladimir Ivanov 
mailto:vladimir.x.iva...@oracle.com>> 
wrote:



Subtle, so I could be misunderstanding something, did you intend to 
remove the assignment of isFrozen in the ConstantCallSite constructor?


Oh, good catch. It is my fault: the update should be there. (The 
barriers are added just to preserve final field semantics for isFrozen.)


Published the wrong version (with some leftovers from last-minute 
failed experiment). Updated in place.


Best regards,
Vladimir Ivanov

On Nov 19, 2019, at 8:53 AM, Vladimir Ivanov 
mailto:vladimir.x.iva...@oracle.com>> 
wrote:


http://cr.openjdk.java.net/~vlivanov/8234401/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8234401

ConstantCallSite has a ctor which deliberately leaks partially 
initialized instance into user code. isFrozen is declared final and 
if user code is obstinate enough, it can end up with non-frozen 
state embedded into the generated code. It manifests as a 
ConstantCallSite instance which is stuck in non-frozen state.


I switched isFrozen from final to @Stable, so non-frozen state is 
never constant folded. Put some store-store barriers along the way 
to restore final field handling.


I deliberately stopped there (just restoring isFrozen final field 
behavior). Without proper synchronization, there's still a 
theoretical possibility of transiently observing a call site in 
non-frozen state right after ctor is over. But at least there's no 
way anymore to accidentally break an instance in such a way it 
becomes permanently unusable.


PS: converted CallSite.target to final along the way and made some 
other minor refactorings.


Testing: regression test, tier1-2

Best regards,
Vladimir Ivanov

Re: RFR 8233272 : The Class.forName specification should be updated to match the long-standing implementation with respect to class linking

[Brent Christian]

Thank you for the suggestions, Mandy and David.  I've pushed the change.

-Brent

Re: [14] RFR (S): 8234401: ConstantCallSite may stuck in non-frozen state

[Paul Sandoz]

Much better :-)

I accumulated some more questions while I was looking further.

CallSite:
 public class CallSite {
 
-// The actual payload of this call site:
+// The actual payload of this call site.
+// Can be modified using {@link 
MethodHandleNatives#setCallSiteTargetNormal} or {@link 
MethodHandleNatives#setCallSiteTargetVolatile}.
 /*package-private*/
-MethodHandle target;// Note: This field is known to the JVM.  Do not 
change.
+final MethodHandle target;  // Note: This field is known to the JVM.

Is there any benefit to making target final, even though it's not really for 
mutable call sites? (With the recent discussion of "final means final" it would 
be nice to not introduce more special case stomping on final fields if we can 
avoid it).


 CallSite(MethodType targetType, MethodHandle createTargetHook) throws 
Throwable {
-this(targetType);
+this(targetType); // need to initialize target to make CallSite.type() 
work in createTargetHook
 ConstantCallSite selfCCS = (ConstantCallSite) this;
 MethodHandle boundTarget = (MethodHandle) 
createTargetHook.invokeWithArguments(selfCCS);
-checkTargetChange(this.target, boundTarget);
-this.target = boundTarget;
+setTargetNormal(boundTarget); // ConstantCallSite doesn't publish 
CallSite.target
+UNSAFE.storeStoreFence(); // barrier between target and isFrozen 
updates
 }

I wonder if instead of introducing the store store fence here we could move it 
into ConstantCallSite?

 protected ConstantCallSite(MethodType targetType, MethodHandle 
createTargetHook) throws Throwable {
-super(targetType, createTargetHook);
+super(targetType, createTargetHook); // "this" instance leaks into 
createTargetHook
+UNSAFE.storeStoreFence(); // barrier between target and isFrozen 
updates <— Add here
 isFrozen = true;
+UNSAFE.storeStoreFence(); // properly publish isFrozen
 }

Paul.

> On Nov 19, 2019, at 9:49 AM, Vladimir Ivanov  
> wrote:
> 
> 
>> Subtle, so I could be misunderstanding something, did you intend to remove 
>> the assignment of isFrozen in the ConstantCallSite constructor?
> 
> Oh, good catch. It is my fault: the update should be there. (The barriers are 
> added just to preserve final field semantics for isFrozen.)
> 
> Published the wrong version (with some leftovers from last-minute failed 
> experiment). Updated in place.
> 
> Best regards,
> Vladimir Ivanov
> 
>>> On Nov 19, 2019, at 8:53 AM, Vladimir Ivanov  
>>> wrote:
>>> 
>>> http://cr.openjdk.java.net/~vlivanov/8234401/webrev.00/
>>> https://bugs.openjdk.java.net/browse/JDK-8234401
>>> 
>>> ConstantCallSite has a ctor which deliberately leaks partially initialized 
>>> instance into user code. isFrozen is declared final and if user code is 
>>> obstinate enough, it can end up with non-frozen state embedded into the 
>>> generated code. It manifests as a ConstantCallSite instance which is stuck 
>>> in non-frozen state.
>>> 
>>> I switched isFrozen from final to @Stable, so non-frozen state is never 
>>> constant folded. Put some store-store barriers along the way to restore 
>>> final field handling.
>>> 
>>> I deliberately stopped there (just restoring isFrozen final field 
>>> behavior). Without proper synchronization, there's still a theoretical 
>>> possibility of transiently observing a call site in non-frozen state right 
>>> after ctor is over. But at least there's no way anymore to accidentally 
>>> break an instance in such a way it becomes permanently unusable.
>>> 
>>> PS: converted CallSite.target to final along the way and made some other 
>>> minor refactorings.
>>> 
>>> Testing: regression test, tier1-2
>>> 
>>> Best regards,
>>> Vladimir Ivanov

Re: [14] RFR (S): 8234401: ConstantCallSite may stuck in non-frozen state

[Vladimir Ivanov]

Subtle, so I could be misunderstanding something, did you intend to remove the 
assignment of isFrozen in the ConstantCallSite constructor?


Oh, good catch. It is my fault: the update should be there. (The 
barriers are added just to preserve final field semantics for isFrozen.)


Published the wrong version (with some leftovers from last-minute failed 
experiment). Updated in place.


Best regards,
Vladimir Ivanov


On Nov 19, 2019, at 8:53 AM, Vladimir Ivanov  
wrote:

http://cr.openjdk.java.net/~vlivanov/8234401/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8234401

ConstantCallSite has a ctor which deliberately leaks partially initialized 
instance into user code. isFrozen is declared final and if user code is 
obstinate enough, it can end up with non-frozen state embedded into the 
generated code. It manifests as a ConstantCallSite instance which is stuck in 
non-frozen state.

I switched isFrozen from final to @Stable, so non-frozen state is never 
constant folded. Put some store-store barriers along the way to restore final 
field handling.

I deliberately stopped there (just restoring isFrozen final field behavior). 
Without proper synchronization, there's still a theoretical possibility of 
transiently observing a call site in non-frozen state right after ctor is over. 
But at least there's no way anymore to accidentally break an instance in such a 
way it becomes permanently unusable.

PS: converted CallSite.target to final along the way and made some other minor 
refactorings.

Testing: regression test, tier1-2

Best regards,
Vladimir Ivanov

Re: [14] RFR (S): 8234401: ConstantCallSite may stuck in non-frozen state

[Paul Sandoz]

Ah the perils of partial construction :-)

Subtle, so I could be misunderstanding something, did you intend to remove the 
assignment of isFrozen in the ConstantCallSite constructor?

ConstantCallSite:

 protected ConstantCallSite(MethodType targetType, MethodHandle 
createTargetHook) throws Throwable {

-super(targetType, createTargetHook);
-isFrozen = true;
+super(targetType, createTargetHook); // "this" instance leaks into 
createTargetHook
+UNSAFE.storeStoreFence(); // properly publish isFrozen

 }

Paul.

> On Nov 19, 2019, at 8:53 AM, Vladimir Ivanov  
> wrote:
> 
> http://cr.openjdk.java.net/~vlivanov/8234401/webrev.00/
> https://bugs.openjdk.java.net/browse/JDK-8234401
> 
> ConstantCallSite has a ctor which deliberately leaks partially initialized 
> instance into user code. isFrozen is declared final and if user code is 
> obstinate enough, it can end up with non-frozen state embedded into the 
> generated code. It manifests as a ConstantCallSite instance which is stuck in 
> non-frozen state.
> 
> I switched isFrozen from final to @Stable, so non-frozen state is never 
> constant folded. Put some store-store barriers along the way to restore final 
> field handling.
> 
> I deliberately stopped there (just restoring isFrozen final field behavior). 
> Without proper synchronization, there's still a theoretical possibility of 
> transiently observing a call site in non-frozen state right after ctor is 
> over. But at least there's no way anymore to accidentally break an instance 
> in such a way it becomes permanently unusable.
> 
> PS: converted CallSite.target to final along the way and made some other 
> minor refactorings.
> 
> Testing: regression test, tier1-2
> 
> Best regards,
> Vladimir Ivanov

Re: RFR: 8234335: Remove line break in class declaration in java.base

[Lance Andersen]

Hi Julia
> On Nov 19, 2019, at 5:06 AM, Julia Boes  wrote:
> 
> Hi Roger, Lance,
> 
>>> If we're putting "public" on the same line as the method then
>>> it seems useful to put the /* non-public */ on the same line too.
>>> Though I don't know we have style guidance for that.
>>> (And elsewhere too).
>> Is the above common coding in the JDK?  To me it seems to be more readable 
>> to have the comment above the method?
>> 
>> If I run reformat in Intellij for example with code similar to the above, it 
>> will put /*non-public*/ on its own line.
>> 
>> Before reformat:
>> 
>> ——
>> /*non-public*/ static void foo(String f1) {
>> System.out.printf("hello %s%n", f1);
>> }
>> —
>> After reformat:
>> —
>> /*non-public*/
>> static void foo(String f1) {
>> System.out.printf("hello %s%n", f1);
>> }
>> ——
> 
> Looking at the existing code base, the same-line version is slightly more 
> common (57 of 100). I would lean on the side of consistency and stick to the 
> same-line version unless there are any objections.
> 
> Updated webrev: http://cr.openjdk.java.net/~jboes/webrevs/8234335/webrev.01/

Seems to be a “your milage varies”.  I am fine with whatever the final decision 
is.  However, I do believe the comment above reads better and aligns the 
methods better.

Best
Lance
> 
> 
> 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: JEP 358 (Helpful NPEs) and single source file mode

[Maurizio Cimadamore]

It occurred to me that jshell is yet another area where we might want to 
turn better NPE messages on by default.


Maurizio

On 19/11/2019 09:24, Maurizio Cimadamore wrote:

I like this suggestion a lot.

Maurizio

On 19/11/2019 08:34, Gunnar Morling wrote:

Hi,

I've been exploring the new helpful NPE feature a bit.

It's a very welcomed improvement, but I noticed one potential usability
issue in conjunction with the single source file mode (JEP 330): as 
debug

info is missing in that case, e.g. local variables are rendered as
"" in the exception message. Single source file mode also 
doesn't
let me specify the "-g:vars" option I could use with javac for adding 
the

debugging info.

Perhaps single source file mode should add debug information by 
default to

prevent this issue?

Thanks,

--Gunnar

Re: RFR: 8234335: Remove line break in class declaration in java.base

[Julia Boes]

Hi Roger, Lance,



Can you recheck the edit to java/lang/invoke/ClassSpecializer.java: 544

I would think the line should be broken at the "..."

* class TopClass { ... * private static final class Species_LLI 
extends TopClass {


That's right, there was also a closing brace missing.



MemberName.java:1098
It seems like there should be some indentation of the 2nd line of the 
declaration:


public 
MemberName resolveOrFail(byte refKind, MemberName m, Class 
lookupClass,

 Class nsmClass)


Done.



If we're putting "public" on the same line as the method then
it seems useful to put the /* non-public */ on the same line too.
Though I don't know we have style guidance for that.
(And elsewhere too).

Is the above common coding in the JDK?  To me it seems to be more readable to 
have the comment above the method?

If I run reformat in Intellij for example with code similar to the above, it 
will put /*non-public*/ on its own line.

Before reformat:

——
/*non-public*/ static void foo(String f1) {
 System.out.printf("hello %s%n", f1);
}
—
After reformat:
—
/*non-public*/
static void foo(String f1) {
 System.out.printf("hello %s%n", f1);
}
——


Looking at the existing code base, the same-line version is slightly 
more common (57 of 100). I would lean on the side of consistency and 
stick to the same-line version unless there are any objections.


Updated webrev: http://cr.openjdk.java.net/~jboes/webrevs/8234335/webrev.01/


Regards,

Julia

Re: Should Java support ERROR_NO_MORE_FILES when canonicalizing paths on Windows?

[Thorsten Schöning]

Guten Tag Thorsten Schöning,
am Dienstag, 19. November 2019 um 10:30 schrieben Sie:

> Please notice that Windows has ERROR_TOO_MANY_OPEN_FILES for that, so
> in my opinion this is another strong hint that ERROR_NO_MORE_FILES
> really is some kind of success. Only undocumented/unepxected, but that
> seems to be the case with many of the other error codes handled in
> "lastErrorReportable" as well.

I've searched the web again about people using ERROR_NO_MORE_FILES
strictly in context of FindFirstFile and at least found some:

https://www.tek-tips.com/viewthread.cfm?qid=876612
https://www.perlmonks.org/?node_id=1104873
ftp://kermit.columbia.edu/kermit/k95source/findfile.c
https://books.google.de/books?id=AF5Lr5HA5UEC=PA68=PA68=FindFirstFileW+ERROR_NO_MORE_FILES=bl=Uvk3ZWJ3vo=ACfU3U1XcESErD0A6I9zPfMjN9GG2pmJeQ=de=X=2ahUKEwjc-dL1__XlAhWPKFAKHanrDi84FBDoATAJegQIChAB#v=onepage=FindFirstFileW%20ERROR_NO_MORE_FILES=false

Mit freundlichen Grüßen,

Thorsten Schöning

-- 
Thorsten Schöning   E-Mail: thorsten.schoen...@am-soft.de
AM-SoFT IT-Systeme  http://www.AM-SoFT.de/

Telefon...05151-  9468- 55
Fax...05151-  9468- 88
Mobil..0178-8 9468- 04

AM-SoFT GmbH IT-Systeme, Brandenburger Str. 7c, 31789 Hameln
AG Hannover HRB 207 694 - Geschäftsführer: Andreas Muchow

Re: Class-Path (in jar file) semantics different between Java 11 and 13 (on Windows)?

[Dalibor Topic]

On 18.11.2019 16:56, Jaikiran Pai wrote:

Quarkus is a relatively new project and furthermore this specific code
is very new too (a few months old I think). So I think this never got
covered as part of the outreach efforts.



Yeah, Quarkus is not on the OpenJDK Quality Outreach list yet. I've just 
pinged Emmanuel Bernard about adding it there, so we'll see if that'll 
change. ;)


cheers,
dalibor topic
--

Dalibor Topic | Consulting Product Manager
Phone: +494089091214  | Mobile: +491737185961
 | Video: dalibor.to...@oracle.com


Oracle Global Services Germany GmbH
Hauptverwaltung: Riesstr. 25, D-80992 München
Registergericht: Amtsgericht München, HRB 246209
Geschäftsführer: Ralf Herrmann

 Oracle is committed to developing
practices and products that help protect the environment

Re: Should Java support ERROR_NO_MORE_FILES when canonicalizing paths on Windows?

[Thorsten Schöning]

Guten Tag Ioi Lam,
am Montag, 18. November 2019 um 22:21 schrieben Sie:

> https://bugs.openjdk.java.net/browse/JDK-8234363

Thanks for doing that!

> I have not investigated the issue in detail yet. How often do you see 
> ERROR_NO_MORE_FILES happening?

It's difficult to say currently because my customer doesn't monitor
such things. So I don't know when it starts to happen and if so, if it
happens always really. From my tests, it seems to start at some point
and happens occasionally afterwards, maybe even getting more. But it
still doesn't happen always, during my tests there where some times at
which copying the files succeeded.

I'm trying to get some more logs in the meantime to find a pattern.

> Have you checked if your process
> (apache?) has too many open files such that FindFirstFileW is not able
> to open the directory to get a file listing?

With Apache I meant the Java-lib providing some I/O-helpers[1], not
the web server or stuff. My daemon is a plain Java-process started
manually on the shell and I'm somewhat sure that I don't have a
handle-leak in that process because of the following reasons:

At the point where I copy things and ERROR_NOR_MORE_FILES happen, I
don't have any open files myself anymore and looking at SCM-logs, code
didn't change. Commons-IO hasn't been updated in years as well, so is
unlikely to newly introduce leaks as well.

Besides that, what Process Monitor[2] logs during success/failure
looks exactly the same in case of error vs. success, the only
difference being ERROR_NO_SUCH_FILE vs. ERROR_NO_MORE_FILES. If there
would be some handle leak in the process resulting in the IOException,
keeping the process running would fail in former file-related
operations already, where I really read and create files on my own.
But that's not the case, all those operations always succeed, only
when it's about copying the created files into their target directory
things start failing at some point, but even then still succeed
sometimes.

But the most important thing in my opinion is that the error is
persistent during restarts of my daemon, which should clear all open
handles in theory. When the problem happens often, restarting my
daemon doesn't seem to change anything. What instantly solves the
problem is clearing the target directory of the copy operation,
either by renaming the old one and creating a new one or by simply
deleting what is present in that directory currently. ONLY if I do one
of those things the copy operations start to succeed reliably again,
regardless of if the daemon is restartedt or kept running even after
failing before.

I don't care about formerly available contents in the target directory
myself, but am using files with timestamps and stuff like that. And
that's my point: While there surely is some problem somewhere, I think
it's most likely to be in the infrastructure of my customer, because
he has storage-related problems anyway. Things are too slow sometimes
and all that. While I don't see anything of those problems in ProcMon,
like timeouts, permissions problems or other real errors, when my
problem occurs, it might simply be that Windows internally behaves
undocumented for some currently unknown reason.

By allowing ERROR_NO_MORE_FILES it might be that whatever the problem
in Windows might be simply gets ignored up until a point where a real
problem happens. And if that doesn't occur in the end, one doesn't
need to care as well. Allowing ERROR_NO_MORE_FILES doesn't look that
different to e.g. ERROR_NETWORK_UNREACHABLE to me, because in my setup
the latter would be the even bigger problem, as I'm copying things on
network shares in the end.

> If that is indeed the case, I am not sure what's the best way of
> handling it. If resource (file descriptors) are running out, perhaps the
> current behavior of throwing an exception in 
> WinNTFileSystem.canonicalize0() would be better than just ignoring it 
> and return an incorrect result. But I'll defer to the folks on the 
> core-libs team.

Please notice that Windows has ERROR_TOO_MANY_OPEN_FILES for that, so
in my opinion this is another strong hint that ERROR_NO_MORE_FILES
really is some kind of success. Only undocumented/unepxected, but that
seems to be the case with many of the other error codes handled in
"lastErrorReportable" as well.

[1]: https://commons.apache.org/proper/commons-io/
[2]: https://docs.microsoft.com/en-us/sysinternals/downloads/procmon

Mit freundlichen Grüßen,

Thorsten Schöning

-- 
Thorsten Schöning   E-Mail: thorsten.schoen...@am-soft.de
AM-SoFT IT-Systeme  http://www.AM-SoFT.de/

Telefon...05151-  9468- 55
Fax...05151-  9468- 88
Mobil..0178-8 9468- 04

AM-SoFT GmbH IT-Systeme, Brandenburger Str. 7c, 31789 Hameln
AG Hannover HRB 207 694 - Geschäftsführer: Andreas Muchow

Re: JEP 358 (Helpful NPEs) and single source file mode

[Maurizio Cimadamore]

I like this suggestion a lot.

Maurizio

On 19/11/2019 08:34, Gunnar Morling wrote:

Hi,

I've been exploring the new helpful NPE feature a bit.

It's a very welcomed improvement, but I noticed one potential usability
issue in conjunction with the single source file mode (JEP 330): as debug
info is missing in that case, e.g. local variables are rendered as
"" in the exception message. Single source file mode also doesn't
let me specify the "-g:vars" option I could use with javac for adding the
debugging info.

Perhaps single source file mode should add debug information by default to
prevent this issue?

Thanks,

--Gunnar

RFR [XS] : 8234339: replace JLI_StrTok in java_md_solinux.c

[Baesken, Matthias]

Hello, please review this small change .

JLI_StrTok is only used in one function, so the define can be replaced, 
probably we should use the "safer" strtok_r (the MT safety might not be a big 
deal in the launcher however).


Bug/webrev :

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

http://cr.openjdk.java.net/~mbaesken/webrevs/8234339.0/


Thanks, Matthias

Re: JEP 358 (Helpful NPEs) and single source file mode

[Remi Forax]

- Mail original -
> De: "Gunnar Morling" 
> À: "core-libs-dev" 
> Envoyé: Mardi 19 Novembre 2019 09:34:41
> Objet: JEP 358 (Helpful NPEs) and single source file mode

> Hi,
> 
> I've been exploring the new helpful NPE feature a bit.
> 
> It's a very welcomed improvement, but I noticed one potential usability
> issue in conjunction with the single source file mode (JEP 330): as debug
> info is missing in that case, e.g. local variables are rendered as
> "" in the exception message. Single source file mode also doesn't
> let me specify the "-g:vars" option I could use with javac for adding the
> debugging info.
> 
> Perhaps single source file mode should add debug information by default to
> prevent this issue?

yes !

> 
> Thanks,
> 
> --Gunnar

Rémi

JEP 358 (Helpful NPEs) and single source file mode

[Gunnar Morling]

Hi,

I've been exploring the new helpful NPE feature a bit.

It's a very welcomed improvement, but I noticed one potential usability
issue in conjunction with the single source file mode (JEP 330): as debug
info is missing in that case, e.g. local variables are rendered as
"" in the exception message. Single source file mode also doesn't
let me specify the "-g:vars" option I could use with javac for adding the
debugging info.

Perhaps single source file mode should add debug information by default to
prevent this issue?

Thanks,

--Gunnar