RFR 8193842: Refactor InputStream-to-OutputStream copy into a utility method

[Brian Burkhalter]

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

Add jdk.internal.io.IOSupport with copy() methods for 
InputStream-to-OutputStream copying and modify some classes to use these new 
methods.

One thing that I noticed when looking at this is that in the fix for 
https://bugs.openjdk.java.net/browse/JDK-8193842, the Files.copy() method had a 
loop like

while ((n = read(…)) > 0)

whereas InputStream.transferTo() had

while((n = read(…)) >= 0)

which is to say that Files.copy() would terminate if there were an empty read() 
but transferTo() would not. The patch for 8193842 therefore possibly introduced 
a subtle behavioral change which no one noticed.

Thanks,

Brian

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

[John Rose]

On Dec 22, 2017, at 11:57 AM, John Rose  wrote:
> 
> On Dec 21, 2017, at 5:31 PM, Stuart Marks  wrote:
>> 
>> I'd like a blanket assertion that view collections are not serializable.
>> 
>> Now this should be considered distinct from whether view collections are 
>> value-based; I'm fine with considering view collections to be value-based. 
>> It's just that some value-based collections are serializable and others 
>> aren't. In particular, given a serializable, value-based list, a sublist 
>> should not be serializable but it can be value-based. A sublist of a sublist 
>> should also not be serializable and can be value-based, and in this case we 
>> can do short-circuiting such as 'return this' if we like.
> 
> The two concerns can be separated, but they necessarily
> have a priority, and I think you have got the priority backward.
> Here's the way I see it:
> 
> 1. Value-based aggregates are inherently lossless when
> serialized, and so should (in general) be made serializable.
> 
> 2. Views are generally lossy when serialized, *if the backing
> store has a significant identity*, and should (in general) not
> be made serializable.
> 
> If we are going to make blanket statements, I claim the first
> is more fundamental than the second.  The second has
> priority only in history, since serialization was invented before
> the value-based distinction.
> 
> I think we should apply the rules in logical order 1, then 2,
> not historical order 2, then 1.
> 

P.S. I am talking about serialization as a general thing with
a future.  If you are talking only about legacy serialization,
then I can see there might be historical influences that would
invert the flow of the above logic.  Still, I'd like to tweak
legacy serialization in better directions AFAP, even if its
ultimately futile, so that when we come up with something
better we will have relatively more good precedents to
preserve.

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

[John Rose]

On Dec 21, 2017, at 5:31 PM, Stuart Marks  wrote:
> 
> I'd like a blanket assertion that view collections are not serializable.
> 
> Now this should be considered distinct from whether view collections are 
> value-based; I'm fine with considering view collections to be value-based. 
> It's just that some value-based collections are serializable and others 
> aren't. In particular, given a serializable, value-based list, a sublist 
> should not be serializable but it can be value-based. A sublist of a sublist 
> should also not be serializable and can be value-based, and in this case we 
> can do short-circuiting such as 'return this' if we like.

The two concerns can be separated, but they necessarily
have a priority, and I think you have got the priority backward.
Here's the way I see it:

1. Value-based aggregates are inherently lossless when
serialized, and so should (in general) be made serializable.

2. Views are generally lossy when serialized, *if the backing
store has a significant identity*, and should (in general) not
be made serializable.

If we are going to make blanket statements, I claim the first
is more fundamental than the second.  The second has
priority only in history, since serialization was invented before
the value-based distinction.

I think we should apply the rules in logical order 1, then 2,
not historical order 2, then 1.

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

[Jason Mehrens]

What are your thoughts on pushing the static EMPTY_XXX declarations from 
ImmutableCollections down in to each respective inner class?  Doing that should 
allow for on demand class loading instead of getting everything when any 
factory is used.

>http://cr.openjdk.java.net/~redestad/8193128/open.04/

Jason

Re: RFR [11] JDK-8179424: Remove terminally deprecated sun.reflect.Reflection.getCallerClass

[mandy chung]

On 12/22/17 2:16 AM, Chris Hegarty wrote:


Webrev with the test updated as suggested:
  http://cr.openjdk.java.net/~chegar/8179424/webrev.03/



Looks good.  Thanks for updating the test.

Mandy

Re: RFR 8193832: Performance of InputStream.readAllBytes() could be improved

[Brian Burkhalter]

Hi Peter,

On Dec 22, 2017, at 1:21 AM, Peter Levart  wrote:

> I think this looks good. No more suggestions from my side for this patch. As 
> Paul notes, it would be interesting to see if using 
> Unsafe.allocateUninitializedArray() to allocate the final array (and 
> intermediate buffers too) has a measurable impact.

Thanks for the final look. I am going to run regression tests once again today 
and if there is no problem I’ll check this in. I’ll also file a follow-up issue 
about the uninitialized array case.

Brian

Re: Adding SocketChannel toString to connection exception messages

[Bernd Eckenfels]

Hello,

I also dearly miss Socket addresses in connection exceptions, however it looks 
like it is not going to make it. However if we add a getter for the Peer 
address (not included in toString) then logging frameworks could detect 
instances of ConnectException and process them accordingly.

Gruss
Bernd
--
http://bernd.eckenfels.net

From: net-dev  on behalf of Chris Hegarty 

Sent: Friday, December 22, 2017 4:17:31 PM
To: Seán Coffey; David Holmes; Steven Schlansker
Cc: core-libs-dev; net-...@openjdk.java.net
Subject: Re: Adding SocketChannel toString to connection exception messages


On 22/12/17 14:59, Seán Coffey wrote:
> As someone who works with alot of log files, I'd like to chime in and
> support Steven's end goal. Looking at a "Connection refused" error in
> the middle of a log file that possibly extends to millions of lines is
> near useless. In the era of cloud compute, diagnosing network issues is
> sure to become a more common task.
>
> While we may not be able to put the sensitive information in an
> exception message, I think we could put it behind a (new?) system
> property which might be able to log this information. Logs contain all
> sorts of sensitive data. Take javax.net.debug=ssl output for example.

I have some sympathy for (capital-L)ogging such detail messages
( given the reasonable restriction on access to log files ), but
a system property that effectively amends exception detail
messages, or prints to stdout/stderr is not a runner in my
opinion.

Maybe we should be looking at instrumentation with JFR events, or
similar. My point being, if someone has time and energy enough
to spend on this, then we can do better than javax.net.debug=ssl.
Also, someone should check that divulging such sensitive information,
even in log files, is acceptable from a security perspective. I'm
personally still dubious.

-Chris.

Re: RFR [jdk8] : 8193807 : AIX: avoid UnsatisfiedLinkError by providing empty basic implementations of getSystemCpuLoad and getProcessCpuLoad

[Volker Simonis]

Hi Martin,

I've just pushed the fix to jdk8u/jdk8u-dev [1] so it should be in the
next regular 8u update (probably 8u172 ?).

You can of course test it any time by building the tip of
http://hg.openjdk.java.net/jdk8u/jdk8u-dev yourself :)

Merry Christmas and a happy new year,
Volker

[1] http://hg.openjdk.java.net/jdk8u/jdk8u-dev/jdk/rev/def07b5ce3be

On Wed, Dec 20, 2017 at 11:18 AM, Evans, Martin
 wrote:
> Many thanks guys, I look forward to testing.
>
> Regards,
>
> Martin
>
> -Original Message-
> From: Volker Simonis [mailto:volker.simo...@gmail.com]
> Sent: 20 December 2017 09:33
> To: Baesken, Matthias 
> Cc: jdk8u-dev-requ...@openjdk.java.net; core-libs-dev@openjdk.java.net; 
> ppc-aix-port-...@openjdk.java.net; Simonis, Volker ; 
> Evans, Martin ; build-dev 
> 
> Subject: Re: RFR [jdk8] : 8193807 : AIX: avoid UnsatisfiedLinkError by 
> providing empty basic implementations of getSystemCpuLoad and 
> getProcessCpuLoad
>
> Hi Matthias,
>
> the change looks good!
> I can sponsor it once we get the approval.
>
> Also forwarded to buil-dev for the minimal build change.
>
> Thank you and best regards,
> Volker
>
>
> On Wed, Dec 20, 2017 at 9:50 AM, Baesken, Matthias  
> wrote:
>> Hello  , Mark reported  this issue on AIX  with OpenJDK8 :
>>
>>>
>>>I'm getting an unsatisfied link error when using logstash on AIX but I 
>>>suspect the issue is with AIX or the JRE, the exception is:
>>>
>>> java.lang.UnsatisfiedLinkError: 
>>> sun.management.OperatingSystemImpl.getProcessCpuLoad()D
>>>  getProcessCpuLoad at 
>>> sun/management/OperatingSystemImpl.java:-2
>>>  at 
>>> org/logstash/instrument/monitors/ProcessMonitor.java:40
>>> detect at 
>>> org/logstash/instrument/monitors/ProcessMonitor.java:79
>>>   generate at
>>> org/logstash/instrument/reports/ProcessReport.java:15
>>>
>>> this is the line in logstash:
>>>
>>> this.cpuProcessPercent =
>>> scaleLoadToPercent(unixOsBean.getProcessCpuLoad());
>>>
>>> https://github.com/elastic/logstash/blob/master/logstash-core/src/mai
>>> n/java/org/logstash/instrument/monitors/ProcessMonitor.java
>>>
>>>
>>> Could anybody help steer me in the right direction?
>>>
>>
>> This fix addresses  the missing  getSystemCpuLoad and getProcessCpuLoad  on 
>> AIX  .
>> JDK8 is only affected , JDK9 and higher is not affected .
>>
>> Could I get a  review for this change   ?
>>
>>
>> Change :
>>
>> http://cr.openjdk.java.net/~mbaesken/webrevs/8193807/
>>
>> Bug :
>>
>> https://bugs.openjdk.java.net/browse/JDK-8193807
>>
>>
>> Thanks, Matthias
>  Kingfisher plc Registered Office: 3 Sheldon Square, Paddington, 
> London W2 6PX Registered in England, Number 1664812 This e-mail is only 
> intended for the person(s) to whom it is addressed and may contain 
> confidential information. Unless stated to the contrary, any opinions or 
> comments are personal to the writer and do not represent the official view of 
> the company. If you have received this e-mail in error, please notify us 
> immediately by reply e-mail and then delete this message from your system. 
> Please do not copy it or use it for any purpose, or disclose its contents to 
> any other person. Thank you for your co-operation.

Re: Adding SocketChannel toString to connection exception messages

[Remi Forax]

Hi David,
you can "abuse" of the suppressed exception feature for that, i.e add a 
CommentException as suppressed exception to the SocketException with no 
stacktrace and a descriptive error message.

Rémi

- Mail original -
> De: "David Lloyd" 
> À: "David Holmes" 
> Cc: "core-libs-dev" 
> Envoyé: Vendredi 22 Décembre 2017 16:22:41
> Objet: Re: Adding SocketChannel toString to connection exception messages

> On Thu, Dec 21, 2017 at 6:35 PM, David Holmes  wrote:
>> I believe there are concerns with too much information that can be
>> considered "sensitive" (like host names and IP addresses) appearing in error
>> messages due to them ending up in log files and bug reports.
> 
> I tend to agree here.  However - and this is a big "however" - while
> this is a good policy for the JDK, higher-level applications and
> libraries often do not have such concerns, since they can often more
> accurately determine whether a piece of information is actually
> sensitive; the JDK must simply assume that it is.  It should be
> *possible* for libraries and applications to *add* this information.
> Right now this is a very, very difficult task: there is an entire
> hierarchy of exceptions, and there is no easy way to change the
> message of an exception.  The only option is to wrap the exception
> with another, more descriptive exception, which is not great for users
> or for developers.
> 
> It would be nice if there were some way to set some supplementary
> information on SocketException (or even IOException) after the
> exception were constructed, which would be included in getMessage() if
> set.  This would allow frameworks and applications to provide some
> context without causing a security problem for the JDK.
> 
> 
> --
> - DML

Re: Adding SocketChannel toString to connection exception messages

[David Lloyd]

On Thu, Dec 21, 2017 at 6:35 PM, David Holmes  wrote:
> I believe there are concerns with too much information that can be
> considered "sensitive" (like host names and IP addresses) appearing in error
> messages due to them ending up in log files and bug reports.

I tend to agree here.  However - and this is a big "however" - while
this is a good policy for the JDK, higher-level applications and
libraries often do not have such concerns, since they can often more
accurately determine whether a piece of information is actually
sensitive; the JDK must simply assume that it is.  It should be
*possible* for libraries and applications to *add* this information.
Right now this is a very, very difficult task: there is an entire
hierarchy of exceptions, and there is no easy way to change the
message of an exception.  The only option is to wrap the exception
with another, more descriptive exception, which is not great for users
or for developers.

It would be nice if there were some way to set some supplementary
information on SocketException (or even IOException) after the
exception were constructed, which would be included in getMessage() if
set.  This would allow frameworks and applications to provide some
context without causing a security problem for the JDK.


-- 
- DML

Re: Adding SocketChannel toString to connection exception messages

[Chris Hegarty]

On 22/12/17 14:59, Seán Coffey wrote:
As someone who works with alot of log files, I'd like to chime in and 
support Steven's end goal. Looking at a "Connection refused" error in 
the middle of a log file that possibly extends to millions of lines is 
near useless. In the era of cloud compute, diagnosing network issues is 
sure to become a more common task.


While we may not be able to put the sensitive information in an 
exception message, I think we could put it behind a (new?) system 
property which might be able to log this information. Logs contain all 
sorts of sensitive data. Take javax.net.debug=ssl output for example.


I have some sympathy for (capital-L)ogging such detail messages
( given the reasonable restriction on access to log files ), but
a system property that effectively amends exception detail
messages, or prints to stdout/stderr is not a runner in my
opinion.

Maybe we should be looking at instrumentation with JFR events, or
similar. My point being, if someone has time and energy enough
to spend on this, then we can do better than javax.net.debug=ssl.
Also, someone should check that divulging such sensitive information,
even in log files, is acceptable from a security perspective. I'm
personally still dubious.

-Chris.

Re: Adding SocketChannel toString to connection exception messages

[Seán Coffey]

As someone who works with alot of log files, I'd like to chime in and 
support Steven's end goal. Looking at a "Connection refused" error in 
the middle of a log file that possibly extends to millions of lines is 
near useless. In the era of cloud compute, diagnosing network issues is 
sure to become a more common task.


While we may not be able to put the sensitive information in an 
exception message, I think we could put it behind a (new?) system 
property which might be able to log this information. Logs contain all 
sorts of sensitive data. Take javax.net.debug=ssl output for example.


Regards,
Sean.

On 22/12/17 09:57, Chris Hegarty wrote:

On 22/12/17 01:27, David Holmes wrote:

cc'ing net-dev as that might be the more appropriate list.

On 22/12/2017 10:59 AM, Steven Schlansker wrote:


On Dec 21, 2017, at 4:35 PM, David Holmes  
wrote:


On 22/12/2017 10:29 AM, Steven Schlansker wrote:
On Dec 21, 2017, at 11:11 AM, Steven Schlansker 
 wrote:


What if ConnectException included the attempted hostname / IP / 
port SocketAddress?
java.net.ConnectException: Connection to 
'foo.mycorp.com[10.x.x.x]:12345' refused
Much more useful!  This could also be extended to various other 
socket exceptions.


I believe there are concerns with too much information that can be 
considered "sensitive" (like host names and IP addresses) appearing 
in error messages due to them ending up in log files and bug reports.


Unfortunately that's exactly the information that is crucial to 
someone trying to diagnose issues...


And to someone trying to discern private information about a network.


Could it be an opt-in policy?  Perhaps by a system property?


Well, you don't know where a log file or exception will end up.
Most likely on stackoverflow.

The expectation is that such information should be added by layers 
higher in the call chain, rather than the JDK libraries assuming it 
is okay to do.


Agreed. It may be some work, but if the issue is significant
enough to the user then it may be worth their effort, as well
as affording the opportunity to opt-out at any particular
point in the code.

Currently the alternative I'm faced with is going through every 
piece of user code and library that *might*
throw this exception and wrapping it to add this critical diagnostic 
information.  For an application that uses
java.net heavily, you can imagine how that is a tall task and 
possibly even not realistically achievable...


(Is there a written policy regarding this somewhere, or is it up to 
the personal feelings of the contributors?)


This is covered by the secure coding guidelines, under 'Confidential 
information':


http://www.oracle.com/technetwork/java/seccodeguide-139067.html#2

"Guideline 2-1 / CONFIDENTIAL-1: Purge sensitive information from 
exceptions"


Exactly.

I know for a fact that we'd have to scrub this information from test 
failures when putting the info into a bug report.


If net-dev folk can't expand further on this then I suggest filing a 
CSR request so that the CSR group can consider it. But I think this 
will be a no-go (I'm a member of the CSR group).


I have to agree with David here. I don't think that such a request
could be supported.

-Chris.

Re: [10] RFR 8075939: Stream.flatMap() causes breaking of short-circuiting of terminal operations

[forax]

> De: "Paul Sandoz" 
> À: "Remi Forax" 
> Cc: "core-libs-dev" 
> Envoyé: Vendredi 22 Décembre 2017 01:38:37
> Objet: Re: [10] RFR 8075939: Stream.flatMap() causes breaking of
> short-circuiting of terminal operations

>> On 21 Dec 2017, at 15:46, Remi Forax < [ mailto:fo...@univ-mlv.fr |
>> fo...@univ-mlv.fr ] > wrote:

>> Hi Paul,
>> three things:
>> - I think you should add a comment to explain why you have chosen to create a
>> the field downstream* in the primitive implementations,
>> I suppose it's to avoid to allocate a lambda proxy at each call.

> Yes, i included this comment:
> // cache the consumer to avoid creation on every accepted element

>> - the fields in the inner classes cancellationRequested and downstream* 
>> should
>> be private.

> Does it make any difference for such inner classes?

Usually, a stronger encapsulation is better than a weaker one. Being an 
anonymous class only means that you can not access the type, not that the 
fields are not accessible. 

> (I lean towards package private as the default.)

I tend to think that before, but with nestmate around the corner, private now 
really means accessible in the same java file, so i do think that private 
should be the new default in nested classes. 

>> - if you use var, you should use a meaningful name, here, 's' can be 
>> replaced by
>> 'spliterator', making the code more readable.

> Don’t agree in this case, given the locality of use and given the method name 
> on
> the RHS.

It means that when you want to know what a variable is you have to read its 
initialization part, it's a bit of a stretch compare to what Brian said about 
var, we can use var because the variable name is meaningful enough, so the type 
is redundant. I suppose it's not a big deal if you read the code sequentially 
but i tend to read the meat part of the code and extend above and below, so i 
was focus on the code that uses downstreamAsInt when i ask myself what 's' was. 

> Thanks,
> Paul.

cheers, 
Rémi 

Re: RFR [11] JDK-8179424: Remove terminally deprecated sun.reflect.Reflection.getCallerClass

[Chris Hegarty]

On 21/12/17 21:40, mandy chung wrote:

 ... >
ReflectionFactory or other class in sun.reflect package would do it.

and create an issue to follow up sun.reflect.Reflection as flagged as a removed 
API.

That’s what the test now does with my changes. Why separate it
out into a separate issue?   If we need a test for an internal API, I
can add a scenario that uses sun.reflect.ReflectionFactory.


Yes please. That's what this case intends to verify.


Webrev with the test updated as suggested:
  http://cr.openjdk.java.net/~chegar/8179424/webrev.03/

-Chris.

Re: Adding SocketChannel toString to connection exception messages

[Chris Hegarty]

On 22/12/17 01:27, David Holmes wrote:

cc'ing net-dev as that might be the more appropriate list.

On 22/12/2017 10:59 AM, Steven Schlansker wrote:


On Dec 21, 2017, at 4:35 PM, David Holmes  
wrote:


On 22/12/2017 10:29 AM, Steven Schlansker wrote:
On Dec 21, 2017, at 11:11 AM, Steven Schlansker 
 wrote:


What if ConnectException included the attempted hostname / IP / 
port SocketAddress?
java.net.ConnectException: Connection to 
'foo.mycorp.com[10.x.x.x]:12345' refused
Much more useful!  This could also be extended to various other 
socket exceptions.


I believe there are concerns with too much information that can be 
considered "sensitive" (like host names and IP addresses) appearing 
in error messages due to them ending up in log files and bug reports.


Unfortunately that's exactly the information that is crucial to 
someone trying to diagnose issues...


And to someone trying to discern private information about a network.


Could it be an opt-in policy?  Perhaps by a system property?


Well, you don't know where a log file or exception will end up.
Most likely on stackoverflow.

The expectation is that such information should be added by layers 
higher in the call chain, rather than the JDK libraries assuming it is 
okay to do.


Agreed. It may be some work, but if the issue is significant
enough to the user then it may be worth their effort, as well
as affording the opportunity to opt-out at any particular
point in the code.

Currently the alternative I'm faced with is going through every piece 
of user code and library that *might*
throw this exception and wrapping it to add this critical diagnostic 
information.  For an application that uses
java.net heavily, you can imagine how that is a tall task and possibly 
even not realistically achievable...


(Is there a written policy regarding this somewhere, or is it up to 
the personal feelings of the contributors?)


This is covered by the secure coding guidelines, under 'Confidential 
information':


http://www.oracle.com/technetwork/java/seccodeguide-139067.html#2

"Guideline 2-1 / CONFIDENTIAL-1: Purge sensitive information from 
exceptions"


Exactly.

I know for a fact that we'd have to scrub this information from test 
failures when putting the info into a bug report.


If net-dev folk can't expand further on this then I suggest filing a CSR 
request so that the CSR group can consider it. But I think this will be 
a no-go (I'm a member of the CSR group).


I have to agree with David here. I don't think that such a request
could be supported.

-Chris.

Re: RFR 8193832: Performance of InputStream.readAllBytes() could be improved

[Peter Levart]

Hi Brian,

On 12/21/2017 09:44 PM, Brian Burkhalter wrote:

On Dec 21, 2017, at 11:44 AM, Paul Sandoz  wrote:


I concur that this horse is almost dead from the beatings but since I already 
hacked up Peter’s suggestion which eliminates intermediate copies I might as 
well hang it out there (see below).

That looks ok to me, i think keeping the buf allocation at the top of the loop 
tends to simplify the reasoning.

Agreed. Here’s the hopefully final version:

http://cr.openjdk.java.net/~bpb/8193832/webrev.04/

I think the test case all the cases, i.e., possible data lengths, aside from 
the OOME case.


I think this looks good. No more suggestions from my side for this 
patch. As Paul notes, it would be interesting to see if using 
Unsafe.allocateUninitializedArray() to allocate the final array (and 
intermediate buffers too) has a measurable impact.


Regards, Peter




I do have one follow on investigation we discussed off list that is worth 
measuring. At the end use the Unsafe array allocation with no zeroing, since 
the resulting array will be fully written into. This might result in an 
observable improvement.

I’ve not forgotten about that but do not know whether we want to include it as 
part of this issue or a subsequent one.


I suggest a follow on investigation.

I’ll file an issue.

Thanks,

Brian