RFR: JDK-8231281: Consider eliminating --identifier option

[Alexander Matveev]

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).


- Removed --identifier option, since it only usage is to specify 
identifier when generating pkg on macOS and as default value for 
--mac-package-identifier.
- Specifying identifier is no longer needed when generating pkg from app 
image and it will be extract from app image by default. 
--mac-package-identifier can be used to overwrite default value.
- Removed check for identifier from DMG bundler, since it is not 
required by DMG.


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

[2] http://cr.openjdk.java.net/~almatvee/8231281/webrev.00/

Thanks,
Alexander

[Ping] 8229333: java/io/File/SetLastModified.java timed out

[Brian Burkhalter]

> Begin forwarded message:
> 
> From: Brian Burkhalter 
> Subject: Re: 8229333: java/io/File/SetLastModified.java timed out
> Date: September 6, 2019 at 9:36:39 AM PDT
> To: core-libs-dev 
> 
> An alternative would be to increase the required memory:
> 
> @@ -23,6 +23,7 @@
>  
>  /* @test
> @bug 4091757 6652379 8177809
> +   @requires os.maxMemory >= 16G
> @summary Basic test for setLastModified method
>   */
> 
> Thanks,
> 
> Brian
> 
>> On Aug 28, 2019, at 10:51 AM, Lance Andersen > > wrote:
>> 
>> Looks fine Brian, fingers crossed the little bugger does not come back :-)
>> 
>>> On Aug 27, 2019, at 2:00 PM, Brian Burkhalter >> > wrote:
>>> 
>>> The failure reported in [1] was observed to occur on two Mac minis with 
>>> HDDs. Given the large file access it is possibly due to slow disk access. 
>>> The suggested fix is to increase the timeout [2]. If it is still observed 
>>> at a later date a new issue can be filed.
>>> 
>>> Thanks,
>>> 
>>> Brian
>>> 
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8229333 
>>> 
>>> [2] diff:
>>> 
>>> @@ -22,11 +22,13 @@
>>>  */
>>> 
>>> /* @test
>>> -   @bug 4091757 6652379 8177809
>>> -   @summary Basic test for setLastModified method
>>> + * @bug 4091757 6652379 8177809
>>> + * @summary Basic test for setLastModified method
>>> + * @run main/timeout=180 SetLastModified
>>>  */
>>> 
>>> -import java.io .*;
>>> +import java.io.File;
>>> +import java.io.FileOutputStream;
>>> import java.nio.ByteBuffer;
>>> import java.nio.channels.FileChannel;
>> 
>>  
>> 
>>   
>> 
>>  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: jsr166 integration 2019-09

[Martin Buchholz]

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.
>

I committed the changes other than for @ReservedStackAccess, which remains
in limbo.

Re: Formatting rules for exception messages

[mark . reinhold]

2019/9/27 6:23:54 -0700, Florian Weimer :
> * mark reinhold:
> 
>> 2019/3/25 5:24:37 -0700, Florian Weimer :
>>> Are there any guidelines for formatting exception messages?
>>> 
>>> In particular, I'm interested in the case when the exception message
>>> is a (clipped) sentence.  Is it supposed to start with a capital
>>> letter?
>>> 
>>> If the message refers to a parameter name, the spelling should reflect
>>> the name exactly, of course.  There seems to be a slight bias towards
>>> capitalization, based on a few greps.  ...
>> 
>> The first word of any exception message in code that I’ve written, or
>> reviewed, is always capitalized unless that word conveys case-sensitive
>> technical information (e.g., a parameter name, as you mentioned).  This
>> improves readability, especially in longer exception messages that
>> contain additional punctuation characters.
> 
> Thank you for confirming my observation.  Would it make sense to have
> these rules documented somewhere?

Yes, it would.  Perhaps an informational JEP -- I’ll add it to my queue.

- Mark

Re: RFR 8231427 (test) Warning cleanup in tests of java.io.Serializable

[Joe Darcy]

Hi Roger,

Generally looks fine. Adding some comments about why the 
@SuppressWarnings annotations were being applied might help future 
readers of the code.


Cheers,

-Joe

On 9/27/2019 10:02 AM, Roger Riggs wrote:
Please review cleanup of tests in java/io/Serializable. There are 
warnings for mis-use of
serialization methods and fields; some are intentional as they are the 
focus of the tests.
The test code had quite a few warnings about raw types, deprecated 
methods, and

missing serialVersionUIDs, and deprecated methods, etc.

Issue:
https://bugs.openjdk.java.net/browse/JDK-8231427

Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-ser-test-cleanup-8231427-2/

Thanks, Roger

Re: RFR[8229338]: clean up test/jdk/java/util/RandomAccess/Basic.java

[Lance Andersen]

Hi Patrick,

This looks much better.

Best
lance
> On Sep 27, 2019, at 10:53 AM, Patrick Concannon 
>  wrote:
> 
> Hi Lance,
> 
> 
> 
> Thanks for your feedback. I've added in those changes, and you can find them 
> in the new webrev linked below.
> 
> webrev: http://cr.openjdk.java.net/~pconcannon/8229338/webrevs/webrev.01/ 
> 
> 
> Kind regards,
> 
> Patrick
> 
> 
> On 26/09/2019 13:36, Lance Andersen wrote:
>> Hi Patrick,
>> 
>> Overall I think this looks ok.
>> 
>> A few minor comments
>> 
>> Please add 8229338 to the @bug line
>> 
>> I might suggest adding either a comment to the DataProvider or the test 
>> which uses it with an overview of the parameters to make it easier and 
>> quicker for future maintainers to know the intent.
>> 
>> Lines 86 and 91, you could if you want  use String.format and just 
>> substitute the changed values.
>> 
>> Your testCopy and testFlil methods you can probably consider using a 
>> DataProvider so that you can also test other types such as Vector or was 
>> this intentional to omit them ?
>> 
>> HTH
>> 
>> Lance
>> 
>>> On Sep 26, 2019, at 4:38 AM, Patrick Concannon 
>>> mailto:patrick.concan...@oracle.com>> wrote:
>>> 
>>> Hi,
>>> 
>>> 
>>> Would it be possible to have my fix for JDK-8229338 reviewed?
>>> 
>>> This a general refactoring of test/jdk/java/util/RandomAccess/Basic.java as 
>>> outlined in JDK-8229338 'clean up 
>>> test/jdk/java/util/RandomAccess/Basic.java'.
>>> 
>>> 
>>> Further information on this bug can be found here: 
>>> https://bugs.openjdk.java.net/browse/JDK-8229338 
>>> 
>>> 
>>> Webrev: http://cr.openjdk.java.net/~pconcannon/8229338/webrevs/webrev.00/ 
>>> 
>>> 
>>> 
>>> Kind regards,
>>> 
>>> Patrick
>>> 
>> 
>>  
>> 
>>   
>> 
>>  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 [14/java.xml] 8016914: CoreDocumentImpl.setXmlVersion NPE

[Joe Wang]

Thanks Lance!

On 9/27/19 10:47 AM, Lance Andersen wrote:

Hi Joe,

The change looks fine as does the test.

Best
Lance
On Sep 27, 2019, at 12:29 PM, Joe Wang > wrote:


Please review a fix to the NPE issue. This is part of the work on the 
XML declaration related issues.


JBS: https://bugs.openjdk.java.net/browse/JDK-8016914
webrev: http://cr.openjdk.java.net/~joehw/jdk14/8016914/webrev/

Thanks,
Joe




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 [14/java.xml] 8016914: CoreDocumentImpl.setXmlVersion NPE

[Lance Andersen]

Hi Joe,

The change looks fine as does the test.

Best
Lance
> On Sep 27, 2019, at 12:29 PM, Joe Wang  wrote:
> 
> Please review a fix to the NPE issue. This is part of the work on the XML 
> declaration related issues.
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8016914
> webrev: http://cr.openjdk.java.net/~joehw/jdk14/8016914/webrev/
> 
> Thanks,
> Joe

 
  

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

RFR 8231427 (test) Warning cleanup in tests of java.io.Serializable

[Roger Riggs]

Please review cleanup of tests in java/io/Serializable. There are 
warnings for mis-use of
serialization methods and fields; some are intentional as they are the 
focus of the tests.
The test code had quite a few warnings about raw types, deprecated 
methods, and

missing serialVersionUIDs, and deprecated methods, etc.

Issue:
https://bugs.openjdk.java.net/browse/JDK-8231427

Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-ser-test-cleanup-8231427-2/

Thanks, Roger

RFR: 8231584: Deadlock with ClassLoader.findLibrary and System.loadLibrary call

[Anton Kozlov]

Hi,

I think that JDK-8194653 [0] affects jdk/jdk and it doesn't specific to 
FileSystems.getDefault.

I'm starting a new thread/bug as the original issue marked as resolved ([3])

Ryan, thanks for investigation and providing a test [1]! 

But the test fails in the same way if the FileSystems.getDefault replaced with 
anything that calls System.loadLibrary in an initializer. For jdk/jdk it may be 
jdk.net.ExtendedSocketOptions.SO_FLOW_SLA and the issue will arise.

I don't think it's possible to provide a general fix for that. We are unable to 
prevent a user to take the Runtime's monitor and do initialization and 
loadLibrary, which make a room for deadlock. Taking the lock is incorrect 
action at first. We can only prevent doing that by accident.

>From the original deadlock report [2], initialization and loadLibrary under 
>the monitor caused not by java.* code, but class loader's findLibrary 
>(sun.misc.ExtClassLoader to be specific). A workaround for ExtClassLoader is 
>possible, to ensure all necessary classes are inited before findLibrary 
>called, i.e. in the constructor. But then we should require the same for any 
>custom class loader. Chances that it would be implemented correctly are not 
>high since even ExtClassLoader is flawed.

I propose to call a ClassLoader.findLibrary with Runtime's monitor unlocked:

http://cr.openjdk.java.net/~akozlov/8231584/webrev.00

Thanks,
Anton

[0]: https://bugs.openjdk.java.net/browse/JDK-8194653
[1]: 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-April/059811.html
[2]: 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-January/050819.html
[3]: https://bugs.openjdk.java.net/browse/JDK-8231584

RFR [14/java.xml] 8016914: CoreDocumentImpl.setXmlVersion NPE

[Joe Wang]

Please review a fix to the NPE issue. This is part of the work on the 
XML declaration related issues.


JBS: https://bugs.openjdk.java.net/browse/JDK-8016914
webrev: http://cr.openjdk.java.net/~joehw/jdk14/8016914/webrev/

Thanks,
Joe

Re: JDK 14 RFR of JDK-8231546: Suppress warnings on non-serializable instance fields in java.prefs module

[Joe Darcy]

Hmm. That would seem to be more consistent. I'll take another look and 
likely file a follow-up bug since I had already pushed JDK-8231546 when 
I saw this message.


Thanks for the careful review,

-Joe

On 9/26/2019 12:58 PM, Chris Hegarty wrote:

+1. Should the @serial javadoc tag be removed?

-Chris.


On 26 Sep 2019, at 19:27, Roger Riggs  wrote:

Looks fine, Joe


On 9/26/19 1:51 PM, Joe Darcy wrote:
Hello,

Another part of the cleanup of library serialization usage ahead of augmented 
javac -Xlint warnings (JDK-8160675), the non-serializable instance fields of 
serializable types in the java.prefs module should have their serial warnings 
suppressed. The analogous issue in core libs is JDK-8231202.

For the type in question, while NodeChangeEvent is typed as serializable, both its 
readObject and writeObject methods are written to unconditionally throw exceptions, so it 
is not serializable in practice. The class does define a serialVersionUID, so the one 
suspect field can be marked as "transient" without causing an effective change 
in serialization support.

Patch below.

Thanks,

-Joe

diff -r e23e560afbcb 
src/java.prefs/share/classes/java/util/prefs/NodeChangeEvent.java
--- a/src/java.prefs/share/classes/java/util/prefs/NodeChangeEvent.java Wed Sep 
25 21:26:38 2019 -0700
+++ b/src/java.prefs/share/classes/java/util/prefs/NodeChangeEvent.java Thu Sep 
26 10:48:34 2019 -0700
@@ -49,7 +49,7 @@
   *
   * @serial
   */
-private Preferences child;
+private transient Preferences child;

  /**
   * Constructs a new {@code NodeChangeEvent} instance.

Re: RFR[8229338]: clean up test/jdk/java/util/RandomAccess/Basic.java

[Patrick Concannon]

Hi Lance,


Thanks for your feedback. I've added in those changes, and you can find 
them in the new webrev linked below.


webrev: http://cr.openjdk.java.net/~pconcannon/8229338/webrevs/webrev.01/


Kind regards,

Patrick


On 26/09/2019 13:36, Lance Andersen wrote:

Hi Patrick,

Overall I think this looks ok.

A few minor comments

Please add 8229338 to the @bug line

I might suggest adding either a comment to the DataProvider or the 
test which uses it with an overview of the parameters to make it 
easier and quicker for future maintainers to know the intent.


Lines 86 and 91, you could if you want  use String.format and just 
substitute the changed values.


Your testCopy and testFlil methods you can probably consider using a 
DataProvider so that you can also test other types such as Vector or 
was this intentional to omit them ?


HTH

Lance

On Sep 26, 2019, at 4:38 AM, Patrick Concannon 
mailto:patrick.concan...@oracle.com>> 
wrote:


Hi,


Would it be possible to have my fix for JDK-8229338 reviewed?

This a general refactoring of 
test/jdk/java/util/RandomAccess/Basic.java as outlined in JDK-8229338 
'clean up test/jdk/java/util/RandomAccess/Basic.java'.



Further information on this bug can be found here: 
https://bugs.openjdk.java.net/browse/JDK-8229338


Webrev: http://cr.openjdk.java.net/~pconcannon/8229338/webrevs/webrev.00/


Kind regards,

Patrick





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 (XS) 8230415 : Avoid redundant permission checking in FilePermissionCollection and SocketPermissionCollection

[Sean Mullan]

Hi Ivan,

The fix looks good. Good catch.

--Sean

On 8/30/19 7:32 PM, Ivan Gerasimov wrote:

Hello!

In the two implementations of PermissionCollection.implies(Permission), 
all the permissions are traversed, and their corresponding bit mask are 
checked.


For example, here's a snippet from FilePermission.java:

     int desired = fperm.getMask();
     int effective = 0;
     int needed = desired;

     for (Permission perm : perms.values()) {
     FilePermission fp = (FilePermission)perm;
     if (((needed & fp.getMask()) != 0) && 
fp.impliesIgnoreMask(fperm)) {

     effective |= fp.getMask();
     if ((effective & desired) == desired) {
     return true;
     }
     needed = (desired ^ effective);// <<< should be 
(desired & ~effective)

     }
     }

Here, if a permission's mask `fp.getMask()` intersects with `needed`, 
but does not fully cover all the needed bits, the variable `needed` is 
updated as XOR of desired and effective. This can raise a 
not-really-needed bits in the `needed` mask, so that for all subsequent 
permissions from the collection with that unneeded bits in the mask, an 
expensive fp.impliesIgnoreMask(fperm) will be executed.


The fix does not change the behavior, but helps avoid unnecessary calls 
to impliesIgnoreMask().


Would you please help review a trivial fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8230415
WEBREV: http://cr.openjdk.java.net/~igerasim/8230415/00/webrev/

Thanks in advance!

Re: Formatting rules for exception messages

[Florian Weimer]

* mark reinhold:

> 2019/3/25 5:24:37 -0700, Florian Weimer :
>> Are there any guidelines for formatting exception messages?
>> 
>> In particular, I'm interested in the case when the exception message
>> is a (clipped) sentence.  Is it supposed to start with a capital
>> letter?
>> 
>> If the message refers to a parameter name, the spelling should reflect
>> the name exactly, of course.  There seems to be a slight bias towards
>> capitalization, based on a few greps.  ...
>
> The first word of any exception message in code that I’ve written, or
> reviewed, is always capitalized unless that word conveys case-sensitive
> technical information (e.g., a parameter name, as you mentioned).  This
> improves readability, especially in longer exception messages that
> contain additional punctuation characters.

Thank you for confirming my observation.  Would it make sense to have
these rules documented somewhere?

Re: CharsetEncoder.maxBytesPerChar()

[Martin Buchholz]

Like Ulf, I am sometimes annoyed by the use of the "character" misnomer
throughout the API docs, and would support an effort to use "character" the
way that unicode.org uses it.
"char" no longer represents a Unicode character, but at least it provides a
short clear name, in the Java language, for "UTF-16 code unit" - if we use
it consistently!
https://unicode.org/faq/utf_bom.html#utf16-1

On Thu, Sep 26, 2019 at 2:24 PM  wrote:

> 2019/9/24 13:00:21 -0700, ulf.zi...@cosoco.de:
> > 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]
> >
> > wouldn't it be more clear to use "char" or even "{@code char}" instead
> > "character" as replacment for the $xtype$ parameters?
>
> The specifications of the Charset{De,En}coder classes make it clear
> up front that “character” means “sixteen-bit Unicode character,” so
> I don’t think changing “character” everywhere to “{@code char}” is
> necessary.
>
> This usage of “character” is common throughout the API specification.
> With the introduction of 32-bit Unicode characters we started calling
> those “code points,” but kept on calling sixteen-bit characters just
> “characters.”  (I don’t think the official term “Unicode code unit”
> ever caught on, and it’s a bit of a mouthful anyway.)
>
> - Mark
>

Re: [8u] PING: RFR: 8213734: SAXParser.parse(File, ..) does not close resources when Exception occurs.

[Severin Gehwolf]

Hi!

I'd appreciate a review of this one. Thanks in advance! See below.

On Tue, 2019-09-03 at 17:50 +0200, Severin Gehwolf wrote:
> On Mon, 2019-08-26 at 17:54 +0200, Severin Gehwolf wrote:
> > Hi,
> > 
> > Please review this 8u backport. The OpenJDK 11u patch does not apply
> > cleanly after path-reshuffeling due to a) test and code for jaxp are
> > split in JDK 8 b) Some rejects in comments - copyright and last
> > modified date. I've omitted rejected comments. I can manually add them
> > if that's preferred. See below.
> > 
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8213734
> > webrevs:
> >   jdk:  
> > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jdk/01/webrev/
> >   jaxp: 
> > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jaxp/01/webrev/
> > 
> > Testing: tier1 on Linux x86_64. javax/xml/jaxp tests. New test on
> >  Windows without the fix fails and passes with it.
> > 
> > Thanks to Simon Tooke who helped testing this patch on Windows.
> > 
> > Rejects:
> > $ cat src/com/sun/org/apache/xerces/internal/impl/XMLEntityManager.java.rej
> > --- XMLEntityManager.java
> > +++ XMLEntityManager.java
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright (c) 2009, 2017, Oracle and/or its affiliates. All rights 
> > reserved.
> > + * Copyright (c) 2009, 2018, Oracle and/or its affiliates. All rights 
> > reserved.
> >   */
> >  /*
> >   * Licensed to the Apache Software Foundation (ASF) under one or more
> > @@ -89,7 +89,7 @@
> >   * @author K.Venugopal SUN Microsystems
> >   * @author Neeraj Bajaj SUN Microsystems
> >   * @author Sunitha Reddy SUN Microsystems
> > - * @LastModified: Oct 2017
> > + * @LastModified: Nov 2018
> >   */
> >  public class XMLEntityManager implements XMLComponent, XMLEntityResolver {
> > 
> 
> Gentle reminder.

Anyone? It's been a month :(

FWIW, I've updated XMLEntityManager to include the copyright update
upper bound to 2018. The @LastModified lines don't exist in JDK 8u, so
I've omitted that. Latest webrevs:

webrevs:
   jdk:  
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jdk/01/webrev/
   jaxp: 
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jaxp/02/webrev/

Thanks,
Severin