Re: JDK 9 RFR of 4026465: Provide more byte array constructors for BigInteger

[Brian Burkhalter]

Hello all,

Thanks for the comments. A new patch is here:

http://cr.openjdk.java.net/~bpb/4026465/webrev.02/

On Dec 30, 2014, at 6:15 PM, joe darcy  wrote:

> The new changes generally look good. A few comments, for the new code like
> 
> 291 } else if ((off < 0) || (off > val.length) || (len < 0) ||
> 292((off + len) > val.length) || ((off + len) < 0)) {
> 293 throw new IndexOutOfBoundsException();
> 
> it is not immediately clear that the arithmetic on line 292 won't have some 
> inappropriate overflow behavior. A comment stating why "off + len" will 
> behave appropriately (assuming it does behave appropriately ;-) would help. 
> (By line 292, both off and len are non-negative so that should limit the case 
> analysis.)

Logic updated.

> It might be worthwhile for all the BigInteger constructors which take array 
> arguments to state something about the thread-safely behavior ("arrays are 
> assumed to be unchanged for the duration of the constructor call...”).

Verbiage added.

> Do have have any code coverage information for the new code by the regression 
> test? It would be good to know whether or not all the guard conditions are 
> properly being executed.

No coverage information, but I added some tests for the guard conditions and 
slightly changed the correct-value part of the test.

On Dec 30, 2014, at 6:42 PM, Paul Benedict  wrote:

> Please add @since 1.9 to the new constructors.

Done.

On Jan 2, 2015, at 1:57 AM, Alan Bateman  wrote:

> I assume this can be reduced down to:
>  if (off < 0 || len < 0 || (off > val.length - len)) { ... }


But then if len > val.length, the third inequality tests ‘off’ for being 
greater than a negative value. Please see the updated patch.

Thanks,

Brian

Re: RFR: JDK-8047769 SecureRandom should be more frugal with file descriptors

[Bradford Wetmore]

On 1/1/2015 12:22 PM, Peter Levart wrote:

Hi Brad,

Here's next webrev which tries to cover all your comments:

http://cr.openjdk.java.net/~plevart/jdk9-dev/FileInputStreamPool.8047769/webrev.04/


I downloaded the webrev.05 (Chris' later followup email) and ran it 
through JPRT.  The only error was the previously seen -Dseed which is 
not your problem.


Again, I only ran:

jdk_lang, jdk_math, jdk_util, jdk_io, jdk_net, jdk_nio,
jdk_security*, jdk_rmi, jdk_text, jdk_time, jdk_other, core_tools.

If you need anything else run, let me know.


Looks like you have a committer status, will you be pushing this?


I can, yes. As soon as we clear-out the remaining questions, right?


Yes.  The comments below are minor and shouldn't need another review
cycle.


I'm worried about the failure of the test you observed while running
from NetBeans. Perhaps a 0.5s wait is sometimes not enough for
ReferenceHandler thread to process (enqueue) a WeakReference. Since
there is already a facility in place to help ReferenceHandler thread
instead of wait for it, I used it in new version of the test.


BTW, there's now an unnecessary import from java.lang.AssertionError 
added in webrev.04.



TEST RESULT: Failed. Compilation failed: Compilation failed


I changed the test to be self-contained now so one can run it without
testlib in classpath.


Thanks.  It's compiling fine now.


Two minor nits?   SeedGenerator.java:  Lines 507/518


Done that too.


Thanks.


Maybe issue multiple reads to exercise in1 and in2?  e.g. 2 bytes of
in1, 4 bytes of in2, then 2 bytes of in1?


The 1st assert makes sure in1 == in2, so there's no point in invoking
the same instance via two aliases.


True.


IIRC, when I ran this under NetBeans last week, the second testCaching
didn't clear the WeakReference.


This should not happen any more now that the test is helping to enqueue
the WeakReferences instead of waiting for ReferenceHandler to enqueue
them.


Yes, that hit the refQueue.poll().

It's always interesting to work with core-libs folks, learn something 
new everyday.  Mixing Lambdas/try-with.


I need a time-machine for your CFV/jdk8 Committer:

http://mail.openjdk.java.net/pipermail/jdk8-dev/2013-August/002896.html

I vote yes.


The test can now fail only if System.gc() does not trigger
WeakReference processing in the VM. Can you give it a try on your
NetBeans environment?


One last comment.  It's now 2015.  ;)

Brad

Re: Explicit Serialization API and Security

[Brian Goetz]

Overall the direction seems promising.  Poking at it a bit...

 - ReadSerial methods are caller-sensitive and only show a class a view 
of its own fields.
 - Invariant checking is separate from deserialization, and does not 
seem entirely built-in -- subclass constructors seem responsible for 
asking parents to do validity-checking?
 - I don't see how this invariant-checking mechanism can enforce 
invariants between superclass fields and subclass fields.  For example:


class A {
int lower, upper;  // invariant: lower <= upper
}

class B extends A {
int cur;  // invariant: lower <= cur <= upper
}

To check such an invariant, the serialization library would have to 
construct the object (in a potentially bad state), invoke the checker at 
each layer, and then fail deserialization if any checker said no.  But, 
an evil checker could still squirrel away a reference under the carpet.


Another challenge in invariant checking is circular data structures.  If 
you have two objects:


class Brother {
final Brother brother;
}

that refer to each other, an invariant you might want to check after 
deserialization is that

  this.brother.brother == this

Obviously you have to patch one or the other instance after construction 
to retain the circular references; at what point do you do invariant 
checking?


On 1/1/2015 7:43 AM, Peter Firmstone wrote:

Subclass example:

class SubFoo extends BaseFoo {

public static ReadSerial check(ReadSerial rs){
if (rs.getInt("y") > 128) throw Exception("too big");
return rs;
}

private final int y;

public SubFoo( int x , int y) {
super(x);
this.y = y;
}

public SubFoo( ReadSerial rs ){
super(BaseFoo.check(check(rs)));
// SubFoo can't get at BaseFoo's rs.getInt("x"),
// it's visible only to BaseFoo. Instead SubFoo would get
// the default int value of 0. Just in case both classes have
// private fields named "x".
// ReadSerial is caller sensitive.
this.y = rs.getInt("y");
}
}

Classes in the heirarchy can provide a static method that throws an
exception to check invarients while preventing a finaliser attack. We'd
want to check invarients for other constructors also, but for berevity...

Eg:

class BaseFoo implements Serializable{

public static ReadSerial check(ReadSerial rs) throws Exception
{
if (rs.getInt("x") < 1)
throw IllegalArgumentException("message");
return rs;
}



Sent from my Nokia N900.

- Original message -
 > So, if I understand this correctly, the way this would get used is:
 >
 > class BaseFoo implements Serializable {
 >  private final int x;
 >
 >  public BaseFoo(ReadSerial rs) {
 >  this(rs.getInt("x"));
 >  }
 >
 >  public BaseFoo(int x) {
 >  this.x = x;
 >  }
 > }
 >
 > Right?
 >
 > What happens with subclasses?  I think then I need an extra RS arg in my
 > constructors:
 >
 > class SubFoo extends BaseFoo {
 >  private final int y;
 >
 >  public SubFoo(ReadSerial rs) {
 >  this(rs.getInt("y"));
 >  }
 >
 >  public BaseFoo(ReadSerial rs, int y) {
 >  super(rs);
 >  this.y = y;
 >  }
 > }
 >
 > Is this what you envision?
 >
 >
 >
 >
 >
 > On 12/27/2014 8:03 PM, Peter Firmstone wrote:
 > > Is there any interest in developing an explicit API for
Serialization?:
 > >
 > > 1. Use a public constructor signature with a single argument,
 > > ReadSerialParameters (read only, writable only by the
 > > serialization framework) to recreate objects, subclasses (when
 > > permitted) call this first from their own constructor, they have
 > > an identical constructor signature.  ReadSerialParameters that are
 > > null may contain a circular reference and will be available after
 > > construction, see #3 below.
 > > 2. Use a factory method (defined by an interface) with one parameter,
 > > WriteSerialParameters (write only, readable only by the
 > > serialization framework), this method can be overridden by
 > > subclasses (when permitted)
 > > 3. For circular links, a public method (defined by an interface) that
 > > accepts one argument, ReadSerialParameters, this method is called
 > > after the constructor completes, subclasses overriding this should
 > > call the superclass method.  If this method is not called, an
 > > implementation, if known to possibly contain circular links,
 > > should check it has been fully initialized in each object method
 > > called.
 > > 4. Retains compatibility with current serialization stream format.
 > > 5. Each serial field has a name, calling class and object reference,
 > > similar to explicitly declaring "private static final
 > > ObjectStreamField[] serialPersistentFields ".
 > >
 > > Benefits:
 > >
 > > 1. An object's internal form is not publicised.
 > > 2. Each class in an object's heirarchy can use a static method to
 > > check invarients and throw an exception, prior to
 > > java.lang.Object's constructor being called, preventing
 > > constructio

Re: RFR 8066397 Remove network-related seed initialization code in ThreadLocal/SplittableRandom

[Bradford Wetmore]

the Microsoft C Runtime Library makes use of this function in its
implementation of "rand_s", so it's removal would break a lot of
programs. I think this is a relative guarantee that the function is here
to stay.


Ok.


What are the fallbacks for SystemRandomImpl if /dev/urandom or the
rtlGenRandomFN/CryptGenRandom aren't available?  Is that something
you'll bake into TLR or will you do it here?

>

I think it's better to leave it to consumers (TLR/SplittableRandom) as
they know what's good-enough for them. The API allows for arbitrary
number of bytes to be generated and I don't have an easy means of
generating more than 8 "random" bytes just from System.nanoTime() and
System.currentTimeMillis() short of using SecureRandom as a fall-back.


webrev.03 only has new code, no changes yet to TLR/SplittableRandom,
so I'm not yet quite following where TLR/SR will be changing.  Also,
what is proposed for platforms that aren't Unix/Windows?  Should there
be a generic fallback mechanism like ThreadedSeedGenerator?  (Note,
I'm not suggesting using it, it's rather...SSLLLOOO...)


Are there other non-Unix and non-Windows platforms?


I'm not an expert in non-Oracle Java offerings, but what about the 
various Java ME devices?  Some of the embedded devices are 
Linux/Windows, but are there others?  JavaCard?


I know IBM has a large number of OS's they support, for example IBM i.

http://en.wikipedia.org/wiki/IBM_i
https://www.ibm.com/developerworks/community/wikis/home?lang=en#/wiki/IBM%20i%20Technology%20Updates/page/Java%20on%20IBM%20i

I know IBM supports Java on z/OS, but apparently that uses a Linux-style 
filesystem via UNIX_System_Services. http://en.wikipedia.org/?title=Z/OS


Not sure about others.

I saw a couple of other non-unix/windows OSs in a wikipedia comparison 
of JVM's, but most of them seem to be discontinued.



I think the planned
fall-back for TLR/SplittableRandom is to just use
System.currentTimeMillis() & System.nanoTime() - these are the defaults
now unless SecureRandom is requested.


Ok.  Since this isn't security-critical, IMHO there just needs to be a 
"reasonable" fall-back option in case /dev/urandom or rtlGenRandomFN 
can't be used.


I don't always closely monitor core-libs-dev, so please cc me if further 
discussion occurs in a different thread.


Cheers, and HNY to you and everyone else here,

Brad

Re: RFR: JDK-8051563: Convert JAXP function tests in xslt components: : org.apache.qetest.dtm package, org.apache.qetest.trax package to openjdk

[Tristan Yan]

Hi Joe and Lance
Sorry for my late reply. I just uploaded the new webrev which is trying to 
limit minimal permissions for most of tests. The new changeset has two base 
classes; class JAXPBaseTest has only minimal set permissions; class 
JAXPFileBaseTest adds two permissions that need access local files in the 
directory directory test.src and test.classes. Most of tests only inherit 
JAXPBaseTest that provides only minimal set permissions. Some tests will 
inherit JAXPFileBaseTest because tests need access local files.
Please help to review the code.
http://cr.openjdk.java.net/~tyan/8051563/webrev.00/ 


Thank you
Tristan


> On Jan 2, 2015, at 10:39 AM, huizhe wang  wrote:
> 
> Lance,
> 
> Tristan is looking into adding an extension base class for about 60 tests 
> that require file permission, then the current base class would indeed set 
> "minimal" permission. So please wait for his update :-)
> 
> Best,
> Joe
> 
> On 12/30/2014 3:07 PM, Lance @ Oracle wrote:
>> Hi Tristan,
>> 
>> I will look at this but doubt I will get to this 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 
>> Sent from my iPad
>> 
>> On Dec 30, 2014, at 5:28 PM, Tristan Yan > > wrote:
>> 
>>> Hi All
>>> 
>>> Can I get your review on this change.
>>> 
>>> http://cr.openjdk.java.net/~tyan/8051563/webrev.00/ 
>>>  
>>> >> >
>>> 
>>> These fixes originally come from bug 
>>> https://bugs.openjdk.java.net/browse/JDK-8051563 
>>>  
>>> >> > as part of our XML test 
>>> colocation work. ThIS change-set mainly covers tests for package 
>>> org.apache.qetest.dtm and org.apache.qetest.trax.
>>> 
>>> In the meantime I took steps at fixing some of our existed test code on 
>>> below issues:
>>> 
>>> 1. Add a base test class for all functional tests that enable security 
>>> manager running. A limited minimal permissions set have been set for every 
>>> test.
>>> 2. Remove all unnecessary exception capture for functional tests that we’re 
>>> using testng to handle all the exceptions.
>>> 3. Use try-resource block to solve all possible resource leaks (including 
>>> InputStream, OutputStream, Writer, Reader). 
>>> 
>>> Thanks a lot.
>>> Tristan
> 

Re: RFR: JDK-8051563: Convert JAXP function tests in xslt components: : org.apache.qetest.dtm package, org.apache.qetest.trax package to openjdk

[huizhe wang]

Lance,

Tristan is looking into adding an extension base class for about 60 
tests that require file permission, then the current base class would 
indeed set "minimal" permission. So please wait for his update :-)


Best,
Joe

On 12/30/2014 3:07 PM, Lance @ Oracle wrote:

Hi Tristan,

I will look at this but doubt I will get to this 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 
Sent from my iPad

On Dec 30, 2014, at 5:28 PM, Tristan Yan > wrote:



Hi All

Can I get your review on this change.

http://cr.openjdk.java.net/~tyan/8051563/webrev.00/ 
 
>


These fixes originally come from bug 
https://bugs.openjdk.java.net/browse/JDK-8051563 
 as part of our XML 
test colocation work. ThIS change-set mainly covers tests for package 
org.apache.qetest.dtm and org.apache.qetest.trax.


In the meantime I took steps at fixing some of our existed test code 
on below issues:


1. Add a base test class for all functional tests that enable 
security manager running. A limited minimal permissions set have been 
set for every test.
2. Remove all unnecessary exception capture for functional tests that 
we’re using testng to handle all the exceptions.
3. Use try-resource block to solve all possible resource leaks 
(including InputStream, OutputStream, Writer, Reader).


Thanks a lot.
Tristan

Re: RFR: 5049299 - (process) Use,posix_spawn, not fork, on S10 to avoid swap,exhaustion (jdk7u-dev)

[Volker Simonis]

Hi Amit,

I think this is the wrong place to ask. This list is about OpenJDK/OracleJDK.

As you can read from the bug report (better look at:
https://bugs.openjdk.java.net/browse/JDK-5049299) there's a list in
which OpenJDK/OracleJDK versions this bug has been fixed.
It describes that this bug hasn't been fixed in JDK 6 (only 7 and 8).

No idea where you can get this information for JRockit - you'd
probably have to ask the one from which you bought it.

By the way, in Oracle/OpenJDK 7 and 8 you can control the launch behaviour with:

-Djdk.lang.Process.launchMechanism={fork, vfork, posix_spawn}

Notice that not every platform supports every setting and different
platforms have different default launching mechanisms. You'll have to
look at the sources to see what's the default (i.e. the files
jdk/src/solaris/classes/java/lang/UNIXProcess.java.{bsd, linux,
solaris}').

Regards,
Volker



On Mon, Dec 29, 2014 at 3:51 PM, Amit Baxi  wrote:
> Hi Folks,
>
> We are facing issue with process builder on JRockit JVM with solaris 5.10
> machine. That is realted to following bug:
>
>http://bugs.java.com/view_bug.do?bug_id=5049299
>
>
>
> Currently we are using JRockit JDK 1.6.0_37-R28.2.5-4.1.0 version. Please
> let me know  following :
>
>  * Which version of JRockit this bug is fixed ? (as it is fixed in JAVA
>8u 15)
>  * If yes then  is it also fixed for solaris 5.10 or In solaris process
>builder still uses fork() call ?
>
>
> As per this page link below Mr. Rob wrote that for solaris it still using
> fork which is confusing weather fixed for solaris or not:
>
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-November/022909.html
>
> Please confirm for this fix and thanks in advance.
>
> Waiting for your response,
>
> Thanks!
> *Amit Baxi
>
>
>
> *
>
>
>
>
>

Re: 5049299 - (process) Use,posix_spawn, not fork, on S10 to avoid swap,exhaustion (jdk7u-dev)

[Rob McKenna]

Hi Amit,

As per https://bugs.openjdk.java.net/browse/JDK-5049299, this is fixed 
in 7u55+ (including 8 fcs). There are some peculiarities however:


On JDK8 posix_spawn is the default on Solaris & Mac. vfork is the 
default on Linux.
On JDK7u55+ posix_spawn is the default on Mac only. fork is the default 
on Solaris and vfork is the default on Linux.


On 7u55+ you can change the default launch mechanism using the 
jdk.lang.Process.launchMechanism startup flag. E.g.:


java -Djdk.lang.Process.launchMechanism=posix_spawn ...

-Rob


On 29/12/14 14:51, Amit Baxi wrote:

Hi Folks,

We are facing issue with process builder on JRockit JVM with solaris 
5.10 machine. That is realted to following bug:


   http://bugs.java.com/view_bug.do?bug_id=5049299



Currently we are using JRockit JDK 1.6.0_37-R28.2.5-4.1.0 version. 
Please let me know  following :


 * Which version of JRockit this bug is fixed ? (as it is fixed in JAVA
   8u 15)
 * If yes then  is it also fixed for solaris 5.10 or In solaris process
   builder still uses fork() call ?


As per this page link below Mr. Rob wrote that for solaris it still 
using fork which is confusing weather fixed for solaris or not:


http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-November/022909.html 



Please confirm for this fix and thanks in advance.

Waiting for your response,

Thanks!
*Amit Baxi



*

RFR: 5049299 - (process) Use,posix_spawn, not fork, on S10 to avoid swap,exhaustion (jdk7u-dev)

[Amit Baxi]

Hi Folks,

We are facing issue with process builder on JRockit JVM with solaris 
5.10 machine. That is realted to following bug:


   http://bugs.java.com/view_bug.do?bug_id=5049299



Currently we are using JRockit JDK 1.6.0_37-R28.2.5-4.1.0 version. 
Please let me know  following :


 * Which version of JRockit this bug is fixed ? (as it is fixed in JAVA
   8u 15)
 * If yes then  is it also fixed for solaris 5.10 or In solaris process
   builder still uses fork() call ?


As per this page link below Mr. Rob wrote that for solaris it still 
using fork which is confusing weather fixed for solaris or not:


http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-November/022909.html

Please confirm for this fix and thanks in advance.

Waiting for your response,

Thanks!
*Amit Baxi



*

pre-RFR (s): 8049847: Enhance PrintWriter line.separator handling

[Claes Redestad]

Hi,

 this is a proposal to resolve concerns that PrintWriter/BufferedWriter 
behave
inconsistently with j.u.Formatter when setting the line.separator 
property to override

system-appropriate line breaks.

 Instead of providing a set of new constructors, I propose to simply 
add a new default
method j.l.Appendable#lineSeparator, which by default delegates to 
System.lineSeparator().
This allows classes such as PrintWriter/BufferedWriter to provide 
overrides which

communicate to j.u.Formatter their intended behavior.

 This indirectly provides a way to control the lineSeparator by 
allowing users to
override PrintWriter or similar in custom classes which override 
lineSeparator, e.g.:


 PrintWriter unixPrintWriter = new PrintWriter(out) {
 @Override
 public String lineSeparator() { return "\n"; }
 };

 PrintWriter windowsPrintWriter = new PrintWriter(out) {
 @Override
 public String lineSeparator() { return "\r\n"; }
 };

 Bonus: This approach can be used to optimize j.l.Throwable to get rid 
of all static
inner classes to wrap PrintWriter/PrintStream and instead use 
j.l.Appendable, which
more than enough will mitigate the startup hit adding a method to 
Appendable will incur.


bug: https://bugs.openjdk.java.net/browse/JDK-8049847
webrev: http://cr.openjdk.java.net/~redestad/8049847/webrev.00/

 I've not filed requests to change the public API just yet, rather 
wanted to throw this out

for preview to see if there's some concern and feedback first.

/Claes

Re: JDK 9 RFR of 4026465: Provide more byte array constructors for BigInteger

[Alan Bateman]

On 31/12/2014 02:15, joe darcy wrote:

Hi Brian,

The new changes generally look good. A few comments, for the new code 
like


 291 } else if ((off < 0) || (off > val.length) || (len < 0) ||
 292((off + len) > val.length) || ((off + len) < 
0)) {

 293 throw new IndexOutOfBoundsException();

it is not immediately clear that the arithmetic on line 292 won't have 
some inappropriate overflow behavior. A comment stating why "off + 
len" will behave appropriately (assuming it does behave appropriately 
;-) would help. (By line 292, both off and len are non-negative so 
that should limit the case analysis.)



I assume this can be reduced down to:
  if (off < 0 || len < 0 || (off > val.length - len)) { ... }

-Alan