Re: RFR: 8017513: Support for closeable streams

[Alan Bateman]

On 28/08/2013 22:10, Henry Jen wrote:

Hi,

Please review the webrev at
http://cr.openjdk.java.net/~henryjen/ccc/8017513/1/webrev/

Based on the feedback/discussion from last time, the EG decided to
weaken AutoCloseable contract(see RFR 8022176[1]), and have Stream
extend AutoCloseable.

A quick briefing of the webrev,
- Remove CloseableStream and DelegatingStream, which was previous used
to implement close of resource
- BaseStream extends AutoCloseable, thus all Stream implementation will
implements close()
- onClose() method allows to chain up close handlers which are invoked
by close()
- Change use of CloseableStream to Stream, mainly java.nio.file.Files
and various tests.

The specdiff is also available at
http://cr.openjdk.java.net/~henryjen/ccc/8017513/1/specdiff/overview-summary.html

Cheers,
Henry

[1]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-August/020236.html

In Files then you've expanded the wildcard on the imports but left 
java.nio.file.attribute.*; (this seems a long list, I think one or two 
may be javadoc references only).


I think I mentioned this previously but in the Files.list/walk/etc. 
methods where you close the resource (on error|runtimeexception) then 
it's probably best to catch the IOException and add it as a suppressed 
exception.


Minor nits in Files but all the  usages after a space before the 
wording. Also can you combine L123-124 on the same line so that the 
formatting is locally consistent.


StreamCloseTest.java has the GPL+CP copyright, I assume you'll replace 
that before pushing.


Otherwise looks good to me.

-Alan.

Re: RFR: JDK-4792059 -- test/java/io/pathNames/GeneralSolaris.java fails on symbolic links

[Alan Bateman]

On 28/08/2013 23:18, Dan Xu wrote:

Thank you, Alan.

I have updated my webrev to 
http://cr.openjdk.java.net/~dxu/4792059/webrev.01/.


-Dan

Looks fine.

-Alan

Re: RFR: 5047859 : (reflect) Class.getField can't find String[].length

[David Holmes]

On 29/08/2013 6:26 AM, Joel Borggrén-Franck wrote:

Hi David,

On Aug 27, 2013, at 5:21 AM, David Holmes  wrote:


Hi Joel,

On 26/08/2013 10:39 PM, Joel Borggren-Franck wrote:

Hi,

Please review doc fix and test for 
http://bugs.sun.com/view_bug.do?bug_id=5047859

http://cr.openjdk.java.net/~jfranck/5047859/webrev.00/

This is a spec change to update the spec to match the long-standing 
implementation.


Have to wonder why reflection chose to ignore 'length' when the JLS is so clear 
that it is a field ??



This is way before my time but my guess is that hotspot got this wrong way back 
when. From my experience with how length was implemented in JRockit it wasn't 
obvious that it should be reflected as a Field.


Anyway ... I think the added wording to getField() is okay but:

1604  * follows.  Let C be the class or interface represented by this 
object:

I don't think it necessary, or desirable to replace 'class' with 'class or interface' here. If you 
do then you should do it everywhere - which would be bad. Plus does it really mean "class, or 
interface, or enum, or annotation" ? Sometimes "class" just means the thing a Class 
represents.


I think it is a reasonable compromise between legalese precision and brevity. 
The javadoc for Class is already inconsistent, using 'class or interface' in 
some parts and a sole 'class' in others where 'class or interface' would be 
more appropriate. I don't think it is realistic to expect a big rewrite of our 
API doc for consistency, but that shouldn't stop us from improving things 
locally. There are a few more clarification to the reflective parts of the  
Class docs in in the pipe.


Even locally there are other uses of "class" which really mean "class or 
interface" (or maybe even Class object?)-


1623  * @return the {@code Field} object of this class specified by

1631  * ancestor of the class loader for the current class and

1634  * of this class.

But changing these would not improve clarity at all IMHO. Using "class" 
to mean "class or interface" is extremely common. Once the method specs, 
as in this case, have stated that the method applies to "class or 
interface" then I think it is perfectly fine to simply say "class" 
thereafter unless you really need to distinguish between classes and 
interfaces.


Cheers,
David
--



Annotation types are interface types and enum types are class types so it 
rarely makes sense to single them out unless they are a special case. Interface 
types are not class types.


There is also a clarification of getFields() javadoc without changing the
spec.


I don't think this change make sense given it already says it returns a zero-length array 
for array classes - as Mandy pointed out. I don't think this "clarification" is 
needed.


Agreed. I plan to revisit this part.

cheers
/Joel

Re: Java 8 RFR 8010430: Math.round has surprising behavior for odd values of ulp 1

[Brian Burkhalter]

I will update the javadoc in the webrev and repost tomorrow.

Brian

On Aug 28, 2013, at 7:03 PM, Guy Steele wrote:

> In any case, I think everyone is now agreed on "the right thing" for going 
> forward.

Re: RFR: 7057785 : (xs) Add note to hashCode() that support for self referential is optional

[Guy Steele]

What Mike said.  It's basically the same problem as for serialization,
so you keep a hashtable of all objects traversed---but you need not record
objects that have no subobjects and have "simple" or "short" printed 
representations
(such as numbers and maybe short strings).

In full generality it requires two traversals.  Then as you format each object
on the second traversal, objects encountered more than once can be given
labels on first occurrence and references on later occurrences.  Common Lisp
uses "#nnn=" for labels and "#nnn# for references, for integers nnn, so a list
containing three occurrences of the same sublist as well as two self-references
and the symbol BAZ might print in this way:

  #1=(#1# #2=(a b c) #2# #1# BAZ #2#)

Common Lisp has a global (dynamically scoped) variable *print-circle* that
controls whether to use the circularity-detection algorithm.

--Guy

On Aug 28, 2013, at 6:54 PM, Alan Eliasen  wrote:

> On 08/28/2013 04:47 PM, Guy Steele wrote:
>>  *ahem* Y'know, Common Lisp had a good solution for
>> printing self-referential structures (by a user-extensible print
>> function) back in 1984.
>> 
>> It leaned on the solution that had been provided in Interlisp in
>> 1974.  On a machine with one megabyte of main memory and a speed of 1
>> MIPS.
>> 
>> This is not rocket science. 
> 
>   Hehe.  So could you please describe the solution?
> 
> -- 
>  Alan Eliasen
>  elia...@mindspring.com
>  http://futureboy.us/

Re: Java 8 RFR 8010430: Math.round has surprising behavior for odd values of ulp 1

[Guy Steele]

Thanks for this context, Joe.  And, truth be told, the fact there was a 
discrepancy
between the textual and code descriptions of the operation may well have been
my error.  (I don't have a clear memory either way, but it's the sort of text
I would have worked on rather than Bill.)

In any case, I think everyone is now agreed on "the right thing" for going 
forward.

--Guy

On Aug 28, 2013, at 9:48 PM, Joseph Darcy  wrote:

> Hello,
> 
> On 8/23/2013 1:36 PM, Guy Steele wrote:
>> The specification of java.lang.Math.round in the first edition of the
>> Java Language Specification is quite clear:
>> 
>> public static int round(float a)
>>   The result is rounded to an integer by adding 1/2, taking the floor of
>>   the result, and casting the result to type int.
>>   In other words, the result is equal to the value of the expression
>>  (int)Math.floor(a + 0.5f)
>> 
>> and a similar statement for the case where the type of the argument is 
>> double.
>> 
>> This does not correspond to "rounding away from zero" as in IEEE754.
>> 
>> The phrase "ties rounding up" entered the Java documentation later on
>> as a (perhaps unfortunately worded) shorthand for the original specification.
> 
> To give some context, the original specification was changed in JDK 7 under 
> bug
> 
>JDK-6430675 Math.round has surprising behavior for 0x1.fp-2
>http://bugs.sun.com/view_bug.do?bug_id=6430675
> 
> At the time, it was making the rounds that Math.round gave a "wrong" answer 
> for an input value equal to the largest floating-point value less than 0.5: 
> Math.round(0.49994) returned 1 rather than 0. The result is wrong 
> in terms of being unexpected; it was the result mandated by the operational 
> definition of the specification.
> 
> I addressed JDK-6430675 by changing the implementation and removing the 
> operational definition of Math.round. While I thought I had ruled out 
> unexpected round-off behavior at the other end of the range, mea culpa, I did 
> not account for the issues reported in 8010430.
> 
> -Joe
> 
>> 
>> --Guy Steele
>> 
>> 
>> On Aug 23, 2013, at 4:24 PM, Dmitry Nadezhin  
>> wrote:
>> 
>>> The specification of java.lang.Math.round() says
>>>* Returns the closest {@code int} to the argument, with ties
>>> * rounding up.
>>> 
>>> It is not clarified what is "ties rounding up".
>>> I guess that it should correspond to the direction "roundTiesToAway" of
>>> IEEE 754-2008
>>> and to the java.math.RoundingMode.HALF_UP .
>>> 
>>> They round
>>> +0.5 -> +1
>>> -0.5 -> -1
>>> 
>>> The current implementation of java.lang.Math.round() rounds
>>> +0.5 -> +1
>>> -0.5 -> 0
>>> 
>>> "ties rounding up" should match IEEE754 standard and other JDK math class,
>>> shouldn't it ?
>>> 
>>> 
>>> On Fri, Aug 23, 2013 at 10:32 PM, Brian Burkhalter <
>>> brian.burkhal...@oracle.com> wrote:
>>> 
 This message follows the RFC
 http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-August/019560.htmlposted
  on August 2.
 
 The issue is http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8010430.
 
 The proposed patch http://cr.openjdk.java.net/~bpb/8010430/ has the
 effect of option (A) in the aforementioned RFC.
 
 Thanks,
 
 Brian
> 

Re: RFR: 5047859 : (reflect) Class.getField can't find String[].length

[Mandy Chung]

On 8/28/2013 6:39 AM, Joel Borggrén-Franck wrote:

Hi Mandy,

Thanks for your comments,

On 2013-08-26, Mandy Chung wrote:

Joel,

The spec of the getFields and getDeclaredFields() methods both states this:

   This method returns an array of length 0 if the class
   or interface declares no fields, or if this|Class|  object
   represents a primitive type, an array class, or void.

The spec of the getDeclaredField() method has this sentence:

   Note that this method will not reflect the {@code length}
   field of an array class.

Your change is okay and it would be good to keep the getField(s)
and getDeclaredField(s) methods be consistent and states its
return value "if this|Class|  object represents a primitive type,
an array class, or void"

I agree the javadoc for the 4 methods should be more uniform. The note
in getDeclaredFields() isn't that good IMHO as for example the term
'array class' in not used in JLS.

I'll be back shortly with an updated webrev.



You may also want to check out getConstructor(s) and 
getDeclaredConstructor(s) that seem to have similar inconsistency issue.


Mandy

Re: Java 8 RFR 8010430: Math.round has surprising behavior for odd values of ulp 1

[Joseph Darcy]

Hello,

On 8/23/2013 1:36 PM, Guy Steele wrote:

The specification of java.lang.Math.round in the first edition of the
Java Language Specification is quite clear:

public static int round(float a)
   The result is rounded to an integer by adding 1/2, taking the floor of
   the result, and casting the result to type int.
   In other words, the result is equal to the value of the expression
(int)Math.floor(a + 0.5f)

and a similar statement for the case where the type of the argument is double.

This does not correspond to "rounding away from zero" as in IEEE754.

The phrase "ties rounding up" entered the Java documentation later on
as a (perhaps unfortunately worded) shorthand for the original specification.


To give some context, the original specification was changed in JDK 7 
under bug


JDK-6430675 Math.round has surprising behavior for 0x1.fp-2
http://bugs.sun.com/view_bug.do?bug_id=6430675

At the time, it was making the rounds that Math.round gave a "wrong" 
answer for an input value equal to the largest floating-point value less 
than 0.5: Math.round(0.49994) returned 1 rather than 0. The 
result is wrong in terms of being unexpected; it was the result mandated 
by the operational definition of the specification.


I addressed JDK-6430675 by changing the implementation and removing the 
operational definition of Math.round. While I thought I had ruled out 
unexpected round-off behavior at the other end of the range, mea culpa, 
I did not account for the issues reported in 8010430.


-Joe



--Guy Steele


On Aug 23, 2013, at 4:24 PM, Dmitry Nadezhin  wrote:


The specification of java.lang.Math.round() says
* Returns the closest {@code int} to the argument, with ties
 * rounding up.

It is not clarified what is "ties rounding up".
I guess that it should correspond to the direction "roundTiesToAway" of
IEEE 754-2008
and to the java.math.RoundingMode.HALF_UP .

They round
+0.5 -> +1
-0.5 -> -1

The current implementation of java.lang.Math.round() rounds
+0.5 -> +1
-0.5 -> 0

"ties rounding up" should match IEEE754 standard and other JDK math class,
shouldn't it ?


On Fri, Aug 23, 2013 at 10:32 PM, Brian Burkhalter <
brian.burkhal...@oracle.com> wrote:


This message follows the RFC
http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-August/019560.htmlposted
 on August 2.

The issue is http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8010430.

The proposed patch http://cr.openjdk.java.net/~bpb/8010430/ has the
effect of option (A) in the aforementioned RFC.

Thanks,

Brian

Re: RFR: JDK-8023765 -- Improve MaxPathLength.java testcase and reduce its test load

[Dan Xu]

On 08/27/2013 07:15 AM, Dan Xu wrote:

On 08/27/2013 12:12 AM, Alan Bateman wrote:

On 27/08/2013 01:18, Dan Xu wrote:

Hi All,

MaxPathLength.javais a troublesome testcase, and fails 
intermittently in the nightly test. And it also runs for a long 
time, especially on Windows platforms. Inorder to improve the test 
stability, I remove its unnecessary test iterations, and use 
NIOdelete method todo the clean-up to make the potential 
failureseasier for diagnosis. Please review thechanges. Thanks!


bug: https://bugs.openjdk.java.net/browse/JDK-8023765
webrev: http://cr.openjdk.java.net/~dxu/8023765/webrev/

-Dan
The double to quickly skip over the names to MAX_LENGTH/2 looks 
reasonable.


I guess fileCreated should really be fileExists as it may be deleted 
and then deleted. An alternative here would be to use 
Files.deleteIfExists as that would avoid the need to introduce flags 
to track whether the directory and exist exists.


-Alan

Hi Alan,

Those flag names are a little misleading. Sorry about that.

fileCreated and dirCreated are actually tracking the existence of new 
file and directories. If the new file gets deleted, I marked the flag 
to false in the code. And at the end, I also change the recorded file 
path after the rename operation.


I agree that using deleteIfExists is a good alternative. In my 
original thought, I plan to monitor every step and make sure all file 
operations happen as expected. Thanks!


-Dan


Stuart brought a very good point for this test failure in the main bug, 
JDK-7160013. According to his comment, "Bottom line is that a Windows 
daemon (not an anti-virus scanner) may hold a file in delete-pending 
state for a short amount of time after it's been deleted. While it's in 
this state, if you ask if the file exists, the system will say that it 
does not. However, if you try to create a new file with the same name, 
you get an access denied error."


And I re-checked remarks in the Windows APIs used for deleting file in 
[1] and directory in [2]. According to the spec, these APIs only mark a 
file/directory for deletion on close. And the delete operation does not 
occur until the last handle to it is closed. Therefore, if a Windows 
daemon opens a handle towards those newly-created directories (I think 
anti-virus on-access scanner may do a similar thing), the delete 
operation will be put in delete-pending state, and the next immediate 
call to create the same directory structure will fail.


Based on the above information, I updated this testcase to make it 
create different directory structure in each loop, so that the 
next-create operation will not conflict with previous pending delete 
operation. I also follow Alan's suggestion to use Files.deleteIfExists 
in the latest change. Please review it at 
http://cr.openjdk.java.net/~dxu/8023765/webrev.01/. Thanks!


-Dan


[1] 
http://msdn.microsoft.com/en-us/library/windows/desktop/aa363915%28v=vs.85%29.aspx
[2] 
http://msdn.microsoft.com/en-us/library/windows/desktop/aa365488%28v=vs.85%29.aspx

Re: RFR: 7057785 : (xs) Add note to hashCode() that support for self referential is optional

[Mike Duigou]

On Aug 28 2013, at 15:54 , Alan Eliasen wrote:

> On 08/28/2013 04:47 PM, Guy Steele wrote:
>>  *ahem* Y'know, Common Lisp had a good solution for
>> printing self-referential structures (by a user-extensible print
>> function) back in 1984.
>> 
>> It leaned on the solution that had been provided in Interlisp in
>> 1974.  On a machine with one megabyte of main memory and a speed of 1
>> MIPS.
>> 
>> This is not rocket science. 
> 
>   Hehe.  So could you please describe the solution?

I am unsure what solution was used in Common Lisp (I think I knew once). The 
most likely solution for Java Collections would be to use a 
ThreadLocal which potentially recursive methods could check/set on 
entry and avoid recursing. Explicitly concurrent hostile implementations could 
use a plain boolean field rather than a ThreadLocal.

So toString() becomes:

public String toString() {
   if(recursed.get()) {
   return "<>"; // self-referential
   }
   
   recursed(true);
   try {
   // ... the normal toString() ...
   } finally {
 recursed.set(true);
   }
}

For performance and footprints reasons the Java Collections chose not to do 
something like this. Graph preserving operations like serialization will 
generally use a hash table with objects added by identity as keys to the table 
before they are traversed. Any objects re-encountered during traversal will be 
not-retraversed and are replaced in the stream with their identity.

Mike

> 
> -- 
>  Alan Eliasen
>  elia...@mindspring.com
>  http://futureboy.us/

Re: RFR: 7057785 : (xs) Add note to hashCode() that support for self referential is optional

[Alan Eliasen]

On 08/28/2013 04:47 PM, Guy Steele wrote:
>  *ahem* Y'know, Common Lisp had a good solution for
> printing self-referential structures (by a user-extensible print
> function) back in 1984.
> 
> It leaned on the solution that had been provided in Interlisp in
> 1974.  On a machine with one megabyte of main memory and a speed of 1
> MIPS.
> 
> This is not rocket science. 

   Hehe.  So could you please describe the solution?

-- 
  Alan Eliasen
  elia...@mindspring.com
  http://futureboy.us/

Re: RFR: 7057785 : (xs) Add note to hashCode() that support for self referential is optional

[Guy Steele]

On Aug 28, 2013, at 6:13 PM, Mike Duigou  wrote:

> 
> On Aug 28 2013, at 11:48 , Martin Buchholz wrote:
> 
>> This isn't just about hashCode - I'm not sure why you are singling it out.  
>> What about toString?
> 
> A reasonable point. The bug reports are just about as common for toString() 
> being "broken" for self-referential collections.


*ahem* Y'know, Common Lisp had a good solution for printing self-referential 
structures (by a user-extensible print function) back in 1984.

It leaned on the solution that had been provided in Interlisp in 1974.  On a 
machine with one megabyte of main memory and a speed of 1 MIPS.

This is not rocket science.

hg: jdk8/tl/langtools: 8010310: [javadoc] Error processing sources with -private

[jonathan . gibbons]

Changeset: 189942cdf585
Author:jjg
Date:  2013-08-28 15:40 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/189942cdf585

8010310: [javadoc] Error processing sources with -private
Reviewed-by: vromero, mcimadamore

! src/share/classes/com/sun/tools/javac/code/Symbol.java
! src/share/classes/com/sun/tools/javadoc/JavadocMemberEnter.java
+ test/tools/javadoc/nonConstExprs/Test.java

Re: RFR: JDK-4792059 -- test/java/io/pathNames/GeneralSolaris.java fails on symbolic links

[Dan Xu]

Thank you, Alan.

I have updated my webrev to 
http://cr.openjdk.java.net/~dxu/4792059/webrev.01/.


-Dan


On 08/28/2013 04:05 AM, Alan Bateman wrote:

On 28/08/2013 00:20, Dan Xu wrote:

Hi,

When GeneralSolaris testcase follows symbolic link to pick up an 
existing file or directory for testing, it will fail the assertion in 
check()method because the file path canonicalization process will 
result in the real path not a path containing symbolic link. I 
enforce this test not to use any symbolic link as a testing file 
ordirectory to avoid this problem. Please help review my fix. Thanks!


bug: https://bugs.openjdk.java.net/browse/JDK-4792059
webrev: http://cr.openjdk.java.net/~dxu/4792059/webrev/

-Dan
You can drop the exists check as Files.is can't return true if the 
file doesn't exist. Otherwise looks okay to me.


-Alan.

Re: RFR: 7057785 : (xs) Add note to hashCode() that support for self referential is optional

[Mike Duigou]

On Aug 28 2013, at 11:48 , Martin Buchholz wrote:

> This isn't just about hashCode - I'm not sure why you are singling it out.  
> What about toString?

A reasonable point. The bug reports are just about as common for toString() 
being "broken" for self-referential collections.

>  Or really, any recursive ("deep") algorithm.

A class level note then for specific classes? A general warning on the 
interfaces regarding self-referential being optional?

Mike

>  And the typical failure mode is inflooping or stack overflow. 
> 
> 
> On Wed, Aug 28, 2013 at 8:49 AM, Mike Duigou  wrote:
> Thanks Stephen,
> 
> I am fine with your wording. Any other votes or suggested wordings?
> 
> Mike
> 
> On Aug 28 2013, at 02:55 , Stephen Colebourne wrote:
> 
> > I lke the idea, but the wording feels a little opaque as the result is
> > typically StackOverflow.
> >
> > Also, I prefer a style with the @apiNote on a line of its own, rather
> > like a heading. It makes the documentation easier to read in source
> > code, and has no effect on the output Javadoc.
> >
> > @apiNote
> > If the Collection is self-referential, where it directly or indirectly
> > contains itself, then the calculation of hashCode may fail with an
> > exception. Implementations may optionally try to handle this scenario,
> > however most current implementations do not do so.
> >
> > Stephen
> >
> > On 28 August 2013 03:06, Mike Duigou  wrote:
> >> Hello all;
> >>
> >> Fairly frequently it is reported that various Collection/Map 
> >> implementations of hashCode() fail when the instance directly or 
> >> indirectly contains itself. For a variety of reasons, mostly performance 
> >> and resource related, most implementations choose not to support 
> >> calculation of hash codes for self-referential collections. This is not 
> >> likely to change. So to reduce confusion and "bug" reports I am proposing 
> >> a non-normative @apiNote be added to Collection and HashMap. The text of 
> >> the proposed note is:
> >>
> >>> Support for calculation of hash code by self referential 
> >>> {Collection|Map}s (they either directly or indirectly contain themselves) 
> >>> is optional. Few Collection implementations support calculation of hash 
> >>> code for self referential instances.
> >>
> >>
> >> http://cr.openjdk.java.net/~mduigou/JDK-7057785/0/webrev/
> >>
> >> Cheers,
> >>
> >> Mike
> 
> 

RFR: JDK-6341345: (spec) Console.reader() should make it clear that the reader requires line termination

[Xueming Shen]

Alan,

My mailbox suggests that you have already reviewed this one back to the
past May, somehow I dropped the ball some where. Here is the webrev and
CCC again (the change is a trivial spec clarification, which I should have
done 8 years ago).

http://cr.openjdk.java.net/~sherman/6341345
http://ccc.us.oracle.com/6341345

Thanks
-Sherman

Re: RFR: 8022176: Weaken contract of java.lang.AutoCloseable

[Henry Jen]

On 08/24/2013 01:40 PM, Alan Bateman wrote:
> On 24/08/2013 14:09, Doug Lea wrote:
>>
>> "resource specification", while accurate, looked confusing here.
>> But we could include both terms, which seems like an improvement.
>> See below.
> This looks better to me, thanks.
> 

Updated webrev as concluded on this email thread, please review at

http://cr.openjdk.java.net/~henryjen/ccc/8022176/1/webrev/

Cheers,
Henry

RFR: 8017513: Support for closeable streams

[Henry Jen]

Hi,

Please review the webrev at
http://cr.openjdk.java.net/~henryjen/ccc/8017513/1/webrev/

Based on the feedback/discussion from last time, the EG decided to
weaken AutoCloseable contract(see RFR 8022176[1]), and have Stream
extend AutoCloseable.

A quick briefing of the webrev,
- Remove CloseableStream and DelegatingStream, which was previous used
to implement close of resource
- BaseStream extends AutoCloseable, thus all Stream implementation will
implements close()
- onClose() method allows to chain up close handlers which are invoked
by close()
- Change use of CloseableStream to Stream, mainly java.nio.file.Files
and various tests.

The specdiff is also available at
http://cr.openjdk.java.net/~henryjen/ccc/8017513/1/specdiff/overview-summary.html

Cheers,
Henry

[1]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-August/020236.html

Re: java.util.Locale changes

[Alan Bateman]

On 28/08/2013 14:25, Masayoshi Okutsu wrote:

(adding core-libs-dev)

Hi Christian,

RFC 4647 defines The Language Priority List [1]. The use of 
java.util.List would be a natural fit, which is the reason why List is 
used. But use of Iterable does work (as API design). The parameter 
name `priorityIterable' sounds a bit odd, though.


Does use of Iterable add much value to the API? (Please note that List 
is also used in Locale.LanguageRange.)


It depends on the usages but Iterable would be a bit more flexible. Does 
the locale matcher require to do anything except iterate over the 
elements? The return could be looked at it but it depends on the typical 
usage -- would the user of the API typically just iterate over the 
matched Locales?


-Alan.

Re: RFR: 5047859 : (reflect) Class.getField can't find String[].length

[Joel Borggrén-Franck]

Hi David,

On Aug 27, 2013, at 5:21 AM, David Holmes  wrote:

> Hi Joel,
> 
> On 26/08/2013 10:39 PM, Joel Borggren-Franck wrote:
>> Hi,
>> 
>> Please review doc fix and test for 
>> http://bugs.sun.com/view_bug.do?bug_id=5047859
>> 
>> http://cr.openjdk.java.net/~jfranck/5047859/webrev.00/
>> 
>> This is a spec change to update the spec to match the long-standing 
>> implementation.
> 
> Have to wonder why reflection chose to ignore 'length' when the JLS is so 
> clear that it is a field ??
> 

This is way before my time but my guess is that hotspot got this wrong way back 
when. From my experience with how length was implemented in JRockit it wasn't 
obvious that it should be reflected as a Field.

> Anyway ... I think the added wording to getField() is okay but:
> 
> 1604  * follows.  Let C be the class or interface represented by this 
> object:
> 
> I don't think it necessary, or desirable to replace 'class' with 'class or 
> interface' here. If you do then you should do it everywhere - which would be 
> bad. Plus does it really mean "class, or interface, or enum, or annotation" ? 
> Sometimes "class" just means the thing a Class represents.

I think it is a reasonable compromise between legalese precision and brevity. 
The javadoc for Class is already inconsistent, using 'class or interface' in 
some parts and a sole 'class' in others where 'class or interface' would be 
more appropriate. I don't think it is realistic to expect a big rewrite of our 
API doc for consistency, but that shouldn't stop us from improving things 
locally. There are a few more clarification to the reflective parts of the  
Class docs in in the pipe.

Annotation types are interface types and enum types are class types so it 
rarely makes sense to single them out unless they are a special case. Interface 
types are not class types.

>> There is also a clarification of getFields() javadoc without changing the
>> spec.
> 
> I don't think this change make sense given it already says it returns a 
> zero-length array for array classes - as Mandy pointed out. I don't think 
> this "clarification" is needed.

Agreed. I plan to revisit this part.

cheers
/Joel

hg: jdk8/tl/jdk: 8023155: Ensure functional consistency across Random, ThreadLocalRandom, SplittableRandom

[paul . sandoz]

Changeset: b1f41565b806
Author:psandoz
Date:  2013-08-28 22:11 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b1f41565b806

8023155: Ensure functional consistency across Random, ThreadLocalRandom, 
SplittableRandom
Reviewed-by: mduigou
Contributed-by: Doug Lea , Paul Sandoz 


! src/share/classes/java/util/Random.java
! src/share/classes/java/util/concurrent/ThreadLocalRandom.java
! test/java/util/Random/RandomStreamTest.java
+ test/java/util/Random/RandomTest.java
! test/java/util/SplittableRandom/SplittableRandomTest.java
+ test/java/util/concurrent/ThreadLocalRandom/ThreadLocalRandomTest.java

hg: jdk8/tl/langtools: 8014566: Remove @ignore tags from MethodReference66 and InInterface when 8013875 is fixed

[henry . jen]

Changeset: 7de7100c30ce
Author:henryjen
Date:  2013-08-28 10:17 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/7de7100c30ce

8014566: Remove @ignore tags from MethodReference66 and InInterface when 
8013875 is fixed
Reviewed-by: briangoetz, jjg

! test/tools/javac/lambda/MethodReference66.java
! test/tools/javac/lambda/lambdaExecution/InInterface.java

Re: Java 8 RFR 8010430: Math.round has surprising behavior for odd values of ulp 1

[Dmitry Nadezhin]

TLS 1 specification contained verbal statement
"The result is rounded to an integer by adding , taking the floor of the
result, and casting the result to type int".
and Java code
Math.floor(f + 0.5).

It seems to me that the verbal statement says about mathematical "+" .
It maps a pair of reals to there exact sum without rounding.

"+" in Java code is machine adding that is mathematic adding followed by
rounding.

In my opionion this is the source of mismatch.
The verbal TLS 1 statement  is equivalent to
"Returns the closest {@code int} to the argument, with ties rounding to
positive infinity".
The Java code "Math.floor(f + 0.5) is not.

This is just a remark.
I agree that formulation "Returns the closest {@code int} to the argument,
with ties rounding to positive infinity" is clear.


On Wed, Aug 28, 2013 at 6:48 PM, Brian Burkhalter <
brian.burkhal...@oracle.com> wrote:

> On Aug 27, 2013, at 7:44 PM, Dmitry Nadezhin wrote:
>
> Is it reasonable to make specification clearer ?
>
> Either to return JLS 1 specification:
> <<<
> The result is rounded to an integer by adding , taking the floor of the
> result, and casting the result to type long.
> >>>
>
>
> This verbiage would imply no change to the implementation with the odd ULP
> unity cases continuing to exhibit the problem described in the issue.
>
> or to replace "rounding up" with "rounding to positive infinity":
> <<<
> Returns the closest {@code int} to the argument, with ties rounding to
> positive infinity.
> >>>
>
>
> This change could accompany the implementation change I posted in the
> updated webrev.
>
> Thanks,
>
> Brian
>

Re: Java 8 RFR 8010430: Math.round has surprising behavior for odd values of ulp 1

[Brian Burkhalter]

On Aug 28, 2013, at 10:55 AM, Dmitry Nadezhin wrote:

> TLS 1 specification contained verbal statement
> "The result is rounded to an integer by adding , taking the floor of the 
> result, and casting the result to typeint".
> and Java code
> Math.floor(f + 0.5).
> 
> It seems to me that the verbal statement says about mathematical "+" .
> It maps a pair of reals to there exact sum without rounding.
> 
> "+" in Java code is machine adding that is mathematic adding followed by 
> rounding.
> 
> In my opionion this is the source of mismatch.

Agreed.

> The verbal TLS 1 statement  is equivalent to
> "Returns the closest {@code int} to the argument, with ties rounding to 
> positive infinity".
> The Java code "Math.floor(f + 0.5) is not.
> 
> This is just a remark.
> I agree that formulation "Returns the closest {@code int} to the argument, 
> with ties rounding to positive infinity" is clear.

If others agree, I can update the webrev to use this verbiage. A CCC might be 
in order as well.

Brian

Re: RFR JDK-8023713: ZipFileSystem has compatiable issue to handle old zip file.

[Xueming Shen]

On 08/28/2013 10:45 AM, Martin Buchholz wrote:

get16 can not return negative values, right?

correct. will remove it in a separate push. thanks!


So elide the
sz < 0 below
 666 if (sz < 0 || (off + 4 + sz) > len) {
 667 break;
 668 }

Otherwise, looks good to me.



On Tue, Aug 27, 2013 at 4:09 PM, Xueming Shen mailto:xueming.s...@oracle.com>> wrote:

On 08/27/2013 03:07 PM, Martin Buchholz wrote:

It does seem vaguely reasonable to support any extra data.

Don't you want to also handle arbitrary byte arrays, if e.g. one the 16-bit 
size fields overflows the extra data?
It looks to me like getExtraLen could return a negative number.



probably I should. The webrev has been updated to simply
copy the rest of the extra data, if the "sz" is either < 0 or
out of the range.

http://cr.openjdk.java.net/~sherman/8023713/webrev/ 





And put a SPACE after "if".


updated.

Thanks!
-Sherman





On Tue, Aug 27, 2013 at 2:42 PM, Xueming Shen mailto:xueming.s...@oracle.com>> wrote:

Hi,

Please help review the change for #8023713

http://cr.openjdk.java.net/~sherman/8023713/webrev 


The root cause is that the newly introduced ZOS.writeExtra() (for
#8015666) fails to handle "irregular" extra data field. The zip spec
requires the the extra data stars with 4 bytes of "tag + size" pair
and then followed by the actual "extra data". The "offending" zip
file actually has the "irregular" extra data field with 1 single byte
as the extra data. That said, the implementation (ZOS) should still
be able handle this kind of zip entry correctly and appropriately.

The proposed solution is to simply copy the specified extra data
into the output stream.

Thanks!
-Sherman

hg: jdk8/tl/jdk: 8023713: ZipFileSystem crashes on old zip file

[xueming . shen]

Changeset: 690b2931baef
Author:sherman
Date:  2013-08-28 09:46 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/690b2931baef

8023713: ZipFileSystem crashes on old zip file
Summary: to handle extra data field copy correctly even the extra data does not 
follow the spec
Reviewed-by: alanb, martin, chegar

! src/share/classes/java/util/zip/ZipOutputStream.java
! test/java/util/zip/TestExtraTime.java

Re: RFR JDK-8023713: ZipFileSystem has compatiable issue to handle old zip file.

[Xueming Shen]

On 08/28/2013 03:56 AM, Alan Bateman wrote:

On 28/08/2013 00:09, Xueming Shen wrote:

On 08/27/2013 03:07 PM, Martin Buchholz wrote:

It does seem vaguely reasonable to support any extra data.

Don't you want to also handle arbitrary byte arrays, if e.g. one the 16-bit 
size fields overflows the extra data?
It looks to me like getExtraLen could return a negative number.



probably I should. The webrev has been updated to simply
copy the rest of the extra data, if the "sz" is either < 0 or
out of the range.

http://cr.openjdk.java.net/~sherman/8023713/webrev/


The updated webrev looks okay, just odd to see an entry named " TestExtreTime" 
(which may have been a continuation of a typo in the original test).



Thanks Alan! That was indeed a typo:-) fixed in updated webrev.

-Sherman

Re: RFR: 7057785 : (xs) Add note to hashCode() that support for self referential is optional

[Mike Duigou]

Thanks Stephen,

I am fine with your wording. Any other votes or suggested wordings?

Mike

On Aug 28 2013, at 02:55 , Stephen Colebourne wrote:

> I lke the idea, but the wording feels a little opaque as the result is
> typically StackOverflow.
> 
> Also, I prefer a style with the @apiNote on a line of its own, rather
> like a heading. It makes the documentation easier to read in source
> code, and has no effect on the output Javadoc.
> 
> @apiNote
> If the Collection is self-referential, where it directly or indirectly
> contains itself, then the calculation of hashCode may fail with an
> exception. Implementations may optionally try to handle this scenario,
> however most current implementations do not do so.
> 
> Stephen
> 
> On 28 August 2013 03:06, Mike Duigou  wrote:
>> Hello all;
>> 
>> Fairly frequently it is reported that various Collection/Map implementations 
>> of hashCode() fail when the instance directly or indirectly contains itself. 
>> For a variety of reasons, mostly performance and resource related, most 
>> implementations choose not to support calculation of hash codes for 
>> self-referential collections. This is not likely to change. So to reduce 
>> confusion and "bug" reports I am proposing a non-normative @apiNote be added 
>> to Collection and HashMap. The text of the proposed note is:
>> 
>>> Support for calculation of hash code by self referential {Collection|Map}s 
>>> (they either directly or indirectly contain themselves) is optional. Few 
>>> Collection implementations support calculation of hash code for self 
>>> referential instances.
>> 
>> 
>> http://cr.openjdk.java.net/~mduigou/JDK-7057785/0/webrev/
>> 
>> Cheers,
>> 
>> Mike

Re: Java 8 RFR 8010430: Math.round has surprising behavior for odd values of ulp 1

[Brian Burkhalter]

On Aug 27, 2013, at 7:44 PM, Dmitry Nadezhin wrote:

> Is it reasonable to make specification clearer ?
> 
> Either to return JLS 1 specification:
> <<<
> The result is rounded to an integer by adding , taking the floor of the 
> result, and casting the result to type long.
> >>>

This verbiage would imply no change to the implementation with the odd ULP 
unity cases continuing to exhibit the problem described in the issue.

> or to replace "rounding up" with "rounding to positive infinity":
> <<<
> Returns the closest {@code int} to the argument, with ties rounding to 
> positive infinity.
> >>>

This change could accompany the implementation change I posted in the updated 
webrev.

Thanks,

Brian

hg: jdk8/tl/jdk: 8022594: Potential deadlock in of sun.nio.ch.Util/IOUtil

[alan . bateman]

Changeset: 378acd4d03c8
Author:alanb
Date:  2013-08-28 15:50 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/378acd4d03c8

8022594: Potential deadlock in  of sun.nio.ch.Util/IOUtil
Reviewed-by: chegar

! src/macosx/classes/sun/nio/ch/KQueueArrayWrapper.java
! src/macosx/classes/sun/nio/ch/KQueueSelectorImpl.java
! src/share/classes/sun/nio/ch/AbstractPollSelectorImpl.java
! src/share/classes/sun/nio/ch/DatagramChannelImpl.java
! src/share/classes/sun/nio/ch/FileChannelImpl.java
! src/share/classes/sun/nio/ch/IOUtil.java
! src/share/classes/sun/nio/ch/Net.java
! src/share/classes/sun/nio/ch/ServerSocketChannelImpl.java
! src/share/classes/sun/nio/ch/SocketChannelImpl.java
! src/share/classes/sun/nio/ch/Util.java
! src/solaris/classes/sun/nio/ch/DatagramDispatcher.java
! src/solaris/classes/sun/nio/ch/DevPollArrayWrapper.java
! src/solaris/classes/sun/nio/ch/DevPollSelectorImpl.java
! src/solaris/classes/sun/nio/ch/EPoll.java
! src/solaris/classes/sun/nio/ch/EPollArrayWrapper.java
! src/solaris/classes/sun/nio/ch/EPollPort.java
! src/solaris/classes/sun/nio/ch/EPollSelectorImpl.java
! src/solaris/classes/sun/nio/ch/FileDispatcherImpl.java
! src/solaris/classes/sun/nio/ch/InheritedChannel.java
! src/solaris/classes/sun/nio/ch/KQueue.java
! src/solaris/classes/sun/nio/ch/KQueuePort.java
! src/solaris/classes/sun/nio/ch/NativeThread.java
! src/solaris/classes/sun/nio/ch/PollArrayWrapper.java
! src/solaris/classes/sun/nio/ch/SinkChannelImpl.java
! src/solaris/classes/sun/nio/ch/SolarisEventPort.java
! src/solaris/classes/sun/nio/ch/SourceChannelImpl.java
! src/solaris/classes/sun/nio/ch/UnixAsynchronousServerSocketChannelImpl.java
! src/solaris/classes/sun/nio/ch/UnixAsynchronousSocketChannelImpl.java
! src/solaris/classes/sun/nio/ch/sctp/SctpChannelImpl.java
! src/solaris/classes/sun/nio/ch/sctp/SctpMultiChannelImpl.java
! src/solaris/classes/sun/nio/ch/sctp/SctpServerChannelImpl.java
! src/windows/classes/sun/nio/ch/DatagramDispatcher.java
! src/windows/classes/sun/nio/ch/FileDispatcherImpl.java
! src/windows/classes/sun/nio/ch/FileKey.java
! src/windows/classes/sun/nio/ch/Iocp.java
! src/windows/classes/sun/nio/ch/PipeImpl.java
! src/windows/classes/sun/nio/ch/SocketDispatcher.java
! src/windows/classes/sun/nio/ch/WindowsAsynchronousFileChannelImpl.java
! src/windows/classes/sun/nio/ch/WindowsAsynchronousServerSocketChannelImpl.java
! src/windows/classes/sun/nio/ch/WindowsAsynchronousSocketChannelImpl.java
! src/windows/classes/sun/nio/ch/WindowsSelectorImpl.java

hg: jdk8/tl/jdk: 8023717: (process) ProcessBuilder should catch SecurityException rather than AccessControlException

[alan . bateman]

Changeset: 2efa310226f7
Author:alanb
Date:  2013-08-28 14:07 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2efa310226f7

8023717: (process) ProcessBuilder should catch SecurityException rather than 
AccessControlException
Reviewed-by: wetmore, alanb
Contributed-by: marti...@google.com

! src/share/classes/java/lang/ProcessBuilder.java

Re: RFR: 5047859 : (reflect) Class.getField can't find String[].length

[Joel Borggrén-Franck]

Hi Mandy,

Thanks for your comments,

On 2013-08-26, Mandy Chung wrote:
> Joel,
> 
> The spec of the getFields and getDeclaredFields() methods both states this:
> 
>   This method returns an array of length 0 if the class
>   or interface declares no fields, or if this|Class|  object
>   represents a primitive type, an array class, or void.
> 
> The spec of the getDeclaredField() method has this sentence:
> 
>   Note that this method will not reflect the {@code length}
>   field of an array class.
> 
> Your change is okay and it would be good to keep the getField(s)
> and getDeclaredField(s) methods be consistent and states its
> return value "if this|Class|  object represents a primitive type,
> an array class, or void"

I agree the javadoc for the 4 methods should be more uniform. The note
in getDeclaredFields() isn't that good IMHO as for example the term
'array class' in not used in JLS.

I'll be back shortly with an updated webrev.

cheers
/Joel

Re: java.util.Locale changes

[Masayoshi Okutsu]

(adding core-libs-dev)

Hi Christian,

RFC 4647 defines The Language Priority List [1]. The use of 
java.util.List would be a natural fit, which is the reason why List is 
used. But use of Iterable does work (as API design). The parameter name 
`priorityIterable' sounds a bit odd, though.


Does use of Iterable add much value to the API? (Please note that List 
is also used in Locale.LanguageRange.)


Thanks,
Masayoshi

[1] http://tools.ietf.org/html/rfc4647#section-2.3

On 2013/08/28 18:31, Christian Beikov wrote:

Hello there,

I have just seen the changes you want to apply to java.util.Locale in 
JDK 8 and was wondering why you are forcing the use of a 
java.util.List in the lookup and filter methods. The related methods 
of java.util.Locale are


public static Locale lookup(List priorityList, 
Collection locales)
public static String lookupTag(List 
priorityList, Collection tags)
public static List filterTags(List 
priorityList, Collection tags)
public static List filterTags(List 
priorityList, Collection tags, Locale.FilteringMode mode)
public static List filter(List 
priorityList, Collection locales)
public static List filter(List 
priorityList, Collection locales, Locale.FilteringMode mode)


which could be changed to

public static Locale lookup(Iterable 
priorityIterable, Collection locales)
public static String lookupTag(Iterable 
priorityIterable, Collection tags)
public static List filterTags(Iterable 
priorityIterable, Collection tags)
public static List filterTags(Iterable 
priorityIterable, Collection tags, Locale.FilteringMode mode)
public static List filter(Iterable 
priorityIterable, Collection locales)
public static List filter(Iterable 
priorityIterable, Collection locales, Locale.FilteringMode mode)


The use of java.util.Collection would also be ok.
One could also use a java.util.Set or something similar to represent 
the language ranges. I just wanted to provide that feedback if anyone 
cares. I am also ok with java.util.List but since you are only relying 
on the iteration order of the priorityList I was curious about the 
reason.

Re: Additional source to guess the local timezone ID on linux

[Andrew Hughes]

- Original Message -
> On 08/27/2013 03:00 AM, Masayoshi Okutsu wrote:
> > /etc/sysconfig/clock used to be supported, but it was removed in JDK 7.
> > The problem is discussed in bug #6456628.
> > 
> > http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6456628
> 
> Thanks for that link. I did not know that this support was present in
> JDK7 but then removed. I did not expect various distributions and
> versions to have significant differences in that file either.
> 

To bring things full circle:

https://bugzilla.redhat.com/show_bug.cgi?id=489586

"BTW: I'll change the warning boilerplate to this to make it more clear:

# The time zone of the system is defined by the contents of /etc/localtime.
# This file is only for evaluation by system-config-date, do not rely on its
# contents elsewhere."

https://bugzilla.redhat.com/show_bug.cgi?id=489586#c2

> > Have you tested your fix on all Red Hat-like distros, including some
> > older releases, with all the time zone IDs?
> 
> No, I tested this with Red Hat Enterprise Linux 6. I did not try it on
> older versions. I know newer versions of Fedora don't ship with this
> file any more.
> 
> Given that this support was removed because there were significant
> differences in that file making maintenance harder in OpenJDK, I am
> going to withdraw this patch.
> 
> Thanks to everyone who reviewed and commented on it.
> 
> Omair
> 
> --
> PGP Key: 66484681 (http://pgp.mit.edu/)
> Fingerprint = F072 555B 0A17 3957 4E95  0056 F286 F14F 6648 4681
> 

-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: 248BDC07 (https://keys.indymedia.org/)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F  8F91 3B96 A578 248B DC07

Re: Potential issue with CHM.toArray

[Peter Levart]

On 08/28/2013 12:10 PM, Paul Sandoz wrote:

Hi,

Intermittent failures were reported with the CHM toArray test in the JDK.

I updated the tests to increase the number of runs and the number of workers 
and i can reliably reproduce on my laptop, see below.

The test presumes the size should always increase but fails when it observes a 
new new size less than the previously observed size.

Is that a valid assumption?


I guess it should be a valid assumption. If a thread observes some 
entries in the Map at a particular time and the same thread observes 
less entries at a later time it means that some entries have 
disappeared. That observation is surprising if the only modifications to 
the Map are concurrent insertions of new entries...



especially when resizing is in progress and bins are transferred from the old 
to the new table e.g. if one sets the initial capacity of the CHM in the test 
to say 8 * 1024 (double the number of entries)  there is no issue.

The toArray implementation essentially uses the iterator, with some use of size 
estimates to create arrays with hopefully minimal re-sizing. So there is 
nothing particular about toArray regarding traversal.


The same can be observed by just counting the elements returned from 
Iterators of various Collection views...


It's interesting to print out the two consecutive arrays when this 
happens. Here's one such occurrence:


prevArray: [0, 1024, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 
16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 
34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47]
nextArray: [0, 1024, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 
16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32]


I modified the CHM.Traverser class (a base for all Iterators) to collect 
all tables that it traverses via ForwardingNode.nextTable links during 
iteration. Here's one such occurrence printing also the state of 
Traverser after the iteration:


prevIterator(iterations=62): tab lengths/hashCodes=[ 32/119d7047 
64/776ec8df 32/119d7047 64/776ec8df], index=32, baseIndex=32, 
baseLimit=32, baseSize=32
nextIterator(iterations=36): tab lengths/hashCodes=[ 32/119d7047 
64/776ec8df 32/119d7047], index=48, baseIndex=16, baseLimit=32, baseSize=32



It seems that the iteration can traverse through the same tables 
multiple times (back and forth) by following ForwardingNode.nextTable 
links. Aren't nextTable links supposed to be only in "forward" 
direction, leading to from smaller to larger tables?



Regards, Peter



Paul.

Re: Potential issue with CHM.toArray

[Doug Lea]

On 08/28/2013 06:10 AM, Paul Sandoz wrote:

Hi,

Intermittent failures were reported with the CHM toArray test in the JDK.

I updated the tests to increase the number of runs and the number of workers 
and i can reliably reproduce on my laptop, see below.

The test presumes the size should always increase


CHM has disclaimers (that possibly could be better emphasized) that
size() and related methods return estimates that may not be accurate
during concurrent updates. So the behavior you see doesn't seem
to be forbidden, but is surprising enough that it might be worth
addressing by restarting if post-check of size doesn't match array size.
I will make sure this doesn't induce hidden livelocks etc, and if all
is well, submit update.

-Doug

Re: RFR: 8015068 : (m) Use jtreg -exclude for problemlist.txt processing

[Alan Bateman]

On 28/08/2013 00:17, Mike Duigou wrote:

Hello all;

I have updated the changeset for this issue based upon feedback from the 
earlier version. As a result of intervening work this version contains even 
more cleanup.

http://cr.openjdk.java.net/~mduigou/JDK-8015068/1/webrev/

Since the last revision:

- One open issue remains--handling of shared_library_permissions. The proposed patch uses 
a pattern rule, "jdk_%", rather than explicit make targets. This means the 
knowledge of which targets required shared_library_permissions has been lost. The 
shared_library_permissions target could be run unconditionally as part of prep but I am 
uncertain if this is reasonable. Alternatives?

- The previously proposed behaviour of writing testoutput to the current 
directory if not ALT_OUTPUTDIR is provided has been changed in response to 
feedback. Output will now be written to jdk/testoutput and this directory has 
been added to the .hgignore. Since most people are expected to use only the 
root repo make which sets ALT_OUTPUTDIR it is not expected that this change 
will make any difference for most users.

As before, testing of this patch requires using a source build of JTReg as it 
requires one fix that is not in the promoted builds. This changeset will not be 
integrated until after the next JTReg promotion.

Mike

The updated webrev looks good to me too although I expect the output 
location will be visited again in the future.


(I like Kelly's suggestion for hazard pay for those working on these 
make files).


-Alan.

Re: RFR: JDK-4792059 -- test/java/io/pathNames/GeneralSolaris.java fails on symbolic links

[Alan Bateman]

On 28/08/2013 00:20, Dan Xu wrote:

Hi,

When GeneralSolaris testcase follows symbolic link to pick up an 
existing file or directory for testing, it will fail the assertion in 
check()method because the file path canonicalization process will 
result in the real path not a path containing symbolic link. I enforce 
this test not to use any symbolic link as a testing file ordirectory 
to avoid this problem. Please help review my fix. Thanks!


bug: https://bugs.openjdk.java.net/browse/JDK-4792059
webrev: http://cr.openjdk.java.net/~dxu/4792059/webrev/

-Dan
You can drop the exists check as Files.is can't return true if the 
file doesn't exist. Otherwise looks okay to me.


-Alan.

Re: RFR JDK-8023713: ZipFileSystem has compatiable issue to handle old zip file.

[Alan Bateman]

On 28/08/2013 00:09, Xueming Shen wrote:

On 08/27/2013 03:07 PM, Martin Buchholz wrote:

It does seem vaguely reasonable to support any extra data.

Don't you want to also handle arbitrary byte arrays, if e.g. one the 
16-bit size fields overflows the extra data?

It looks to me like getExtraLen could return a negative number.



probably I should. The webrev has been updated to simply
copy the rest of the extra data, if the "sz" is either < 0 or
out of the range.

http://cr.openjdk.java.net/~sherman/8023713/webrev/

The updated webrev looks okay, just odd to see an entry named " 
TestExtreTime" (which may have been a continuation of a typo in the 
original test).


-Alan.

Re: Take 2 Re: RFR 8023155: Ensure functional consistency across Random, ThreadLocalRandom, SplittableRandom

[Paul Sandoz]

On Aug 27, 2013, at 11:47 PM, Mike Duigou  wrote:
> ThreadLocalRandom::
> 
> - I don't understand the point of having a writeObject() if the readResolve() 
> ignores the result. My expectation for a serialized TLR might be that upon 
> de-serialization the seeding state is restored. If that isn't provided, why 
> offer any serialization at all? Alternately we should be more explicit that 
> the seeding state is not part of the serialization.
> 

My understanding is it preserves compatibility when deserializing, as indicated 
by the internal comment:

 * still allowing a call from constructor.  Note that
 * serialization is completely unnecessary because there is only a
 * static singleton.  But we generate a serial form containing
 * "rnd" and "initialized" fields to ensure compatibility across
 * versions.


> - There's no test for the serialization behaviour.
> 

Right, neither for the TCK AFAICT. (There is a serialization test for Random in 
the JCK.) Issue logged, 8023896.


> SplittableRandomTest::
> 
> - executeAndCatch -> assertThrows perhaps? There are a few implementations of 
> assertThrows around in other tests (which haven't been collected into a 
> library yet).
> 

There are also a few implementations of executeAndCatch lying around too caused 
by me :-) I agree it is a bad name. We should change all of 'em. I logged 
another issue, 8023897.

I wish there was a common library of additional functionality that is missing 
from TestNG itself.

Paul.

Potential issue with CHM.toArray

[Paul Sandoz]

Hi,

Intermittent failures were reported with the CHM toArray test in the JDK.

I updated the tests to increase the number of runs and the number of workers 
and i can reliably reproduce on my laptop, see below.

The test presumes the size should always increase but fails when it observes a 
new new size less than the previously observed size. 

Is that a valid assumption? especially when resizing is in progress and bins 
are transferred from the old to the new table e.g. if one sets the initial 
capacity of the CHM in the test to say 8 * 1024 (double the number of entries)  
there is no issue. 

The toArray implementation essentially uses the iterator, with some use of size 
estimates to create arrays with hopefully minimal re-sizing. So there is 
nothing particular about toArray regarding traversal.

Paul.

public class toArray {

public static void main(String[] args) throws Throwable {
for (int i = 0; i < 100; i++) {
main();
}
}

public static void main() throws Throwable {
final Throwable throwable[] = new Throwable[1];
final ConcurrentHashMap m
= new ConcurrentHashMap();

BiConsumer r = (o, b) -> {
for (int i = o; i < b; i++)
m.put(i, i);
};

final int nWorkers = 4;
final int sizePerWorker = 1024;
final int maxSize = nWorkers * sizePerWorker;
List workers = IntStream.range(0, nWorkers).
map(w -> w * sizePerWorker).
mapToObj(w -> (Runnable )() -> r.accept(w, w + sizePerWorker)).
map(Thread::new).collect(toList());

final Thread foreman = new Thread() {
public Throwable exception = null;
private int prevSize = 0;

private boolean checkProgress(Object[] a) {
int size = a.length;
System.out.println(size);
if (size < prevSize) throw new RuntimeException("WRONG WAY");
if (size > maxSize)  throw new RuntimeException("OVERSHOOT");
if (size == maxSize) return true;
prevSize = size;
return false;
}

public void run() {
try {
Integer[] empty = new Integer[0];
while (true) {
if (checkProgress(m.values().toArray())) return;
if (checkProgress(m.keySet().toArray())) return;
if (checkProgress(m.values().toArray(empty))) return;
if (checkProgress(m.keySet().toArray(empty))) return;
}
} catch (Throwable t) {
   throwable[0] = t;
}}};

foreman.start();
workers.stream().forEach(Thread::start);

workers.stream().forEach(toArray::join);
foreman.join();

if (throwable[0] != null)
throw throwable[0];
}

static void join(Thread t) {
try {
t.join();
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
}
}

Re: Simple RFR: 8203311 Clean up profile-includes.txt

[David Holmes]

Sorry about that. I thought the two fixes were travelling together else 
I would have taken steps to put them together. :(


David

On 28/08/2013 8:04 PM, Dawid Weiss wrote:

Thanks Andreas, I'll switch to that revision.

On Wed, Aug 28, 2013 at 12:00 PM, Andreas Rieber
 wrote:

Hi Dawid,

the fix is in jdk8tl already, just missed to come into jdk8. It's only a few
lines.

Andreas

Here the changeset:

Changeset: 3b8fed46b2a8
Author:dholmes
Date:  2013-08-21 05:56 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3b8fed46b2a8

8023460: OPENJDK build fails due to missing jfr.jar
Reviewed-by: alanb

! makefiles/Profiles.gmk



On 28.08.13 11:44, Dawid Weiss wrote:


David,

Is the fix for this committed anywhere? I've just tried building from
scratch and I'm getting the same error.

Dawid

On Wed, Aug 21, 2013 at 10:55 AM, David Holmes 
wrote:


Fix on the way. This exposed another piece that was missing from the
original change. But mea culpa for not doing an OPENJDK build. Sorry
about
that.

Thanks,
David


On 21/08/2013 6:31 PM, Andreas Rieber wrote:



Hi David,

On 21.08.13 09:53, David Holmes wrote:



Hi Andreas,

On 21/08/2013 4:44 PM, Andreas Rieber wrote:



Hi David,

your change causes build problems with ubuntu 12.04. Result is:

make[2]: Entering directory `/usr/local/src/jdk8tl/jdk/makefiles'
make[2]: *** No rule to make target


`/usr/local/src/jdk8tl/build/linux-x86-normal-server-release/images/lib/jfr.jar',


needed by `all'.  Stop.
make[2]: Leaving directory `/usr/local/src/jdk8tl/jdk/makefiles'
make[1]: *** [images] Error 2
make[1]: Leaving directory `/usr/local/src/jdk8tl/jdk/makefiles'
make: *** [images-only] Error 2

I tried several make clean, configure and so on but only when i
rollback
your change i can compile.




I presume this is an OpenJDK build?




Yes, openJDK build. Just tried clean fresh build on ubuntu 13.04 with
jdk8tl, same result:

## Starting images
Creating images/lib/charsets.jar
make[2]: *** No rule to make target


`/usr/local/src/jdk8tl/build/linux-x86_64-normal-server-release/images/lib/jfr.jar',
needed by `all'.  Stop.
make[2]: *** Waiting for unfinished jobs
Creating images/lib/ext/dnsns.jar
Creating images/lib/ext/cldrdata.jar
make[1]: *** [images] Error 2
make: *** [images-only] Error 2



That is quite puzzling  I will investigate.




thanks for puzzling
Andreas



Thanks,
David


Andreas


On 20.08.13 01:33, David Holmes wrote:



http://cr.openjdk.java.net/~dholmes/8023311/webrev/

Patch inlined below

This is a trivial cleanup following on from an earlier change under
8017570. JFR was moved from profile compact3 to the full JRE but not
all
the variables in profile-includes.txt were updated as needed. jfr.jar
needed to removed from the profile3 jar list and added to the jre jar
list.

This has no impact on the actual build artifacts - JFR was, and
remains,
in the full JRE only.

Thanks,
David

--- old/makefiles/profile-includes.txt2013-08-19
19:10:17.554355677
-0400
+++ new/makefiles/profile-includes.txt2013-08-19
19:10:14.586187808
-0400
@@ -102,6 +102,7 @@
security/US_export_policy.jar \
security/local_policy.jar

+
PROFILE_2_JRE_BIN_FILES := \
rmid$(EXE_SUFFIX) \
rmiregistry$(EXE_SUFFIX)
@@ -140,7 +141,6 @@
PROFILE_3_JRE_OTHER_FILES :=

PROFILE_3_JRE_JAR_FILES := \
-jfr.jar \
management-agent.jar


@@ -253,6 +253,6 @@
ext/cldrdata.jar \
ext/dnsns.jar \
ext/nashorn.jar \
-ext/zipfs.jar
-
+ext/zipfs.jar \
+jfr.jar

Re: Simple RFR: 8203311 Clean up profile-includes.txt

[Dawid Weiss]

Thanks Andreas, I'll switch to that revision.

On Wed, Aug 28, 2013 at 12:00 PM, Andreas Rieber
 wrote:
> Hi Dawid,
>
> the fix is in jdk8tl already, just missed to come into jdk8. It's only a few
> lines.
>
> Andreas
>
> Here the changeset:
>
> Changeset: 3b8fed46b2a8
> Author:dholmes
> Date:  2013-08-21 05:56 -0400
> URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3b8fed46b2a8
>
> 8023460: OPENJDK build fails due to missing jfr.jar
> Reviewed-by: alanb
>
> ! makefiles/Profiles.gmk
>
>
>
> On 28.08.13 11:44, Dawid Weiss wrote:
>>
>> David,
>>
>> Is the fix for this committed anywhere? I've just tried building from
>> scratch and I'm getting the same error.
>>
>> Dawid
>>
>> On Wed, Aug 21, 2013 at 10:55 AM, David Holmes 
>> wrote:
>>>
>>> Fix on the way. This exposed another piece that was missing from the
>>> original change. But mea culpa for not doing an OPENJDK build. Sorry
>>> about
>>> that.
>>>
>>> Thanks,
>>> David
>>>
>>>
>>> On 21/08/2013 6:31 PM, Andreas Rieber wrote:


 Hi David,

 On 21.08.13 09:53, David Holmes wrote:
>
>
> Hi Andreas,
>
> On 21/08/2013 4:44 PM, Andreas Rieber wrote:
>>
>>
>> Hi David,
>>
>> your change causes build problems with ubuntu 12.04. Result is:
>>
>> make[2]: Entering directory `/usr/local/src/jdk8tl/jdk/makefiles'
>> make[2]: *** No rule to make target
>>
>>
>> `/usr/local/src/jdk8tl/build/linux-x86-normal-server-release/images/lib/jfr.jar',
>>
>>
>> needed by `all'.  Stop.
>> make[2]: Leaving directory `/usr/local/src/jdk8tl/jdk/makefiles'
>> make[1]: *** [images] Error 2
>> make[1]: Leaving directory `/usr/local/src/jdk8tl/jdk/makefiles'
>> make: *** [images-only] Error 2
>>
>> I tried several make clean, configure and so on but only when i
>> rollback
>> your change i can compile.
>
>
>
> I presume this is an OpenJDK build?



 Yes, openJDK build. Just tried clean fresh build on ubuntu 13.04 with
 jdk8tl, same result:

 ## Starting images
 Creating images/lib/charsets.jar
 make[2]: *** No rule to make target


 `/usr/local/src/jdk8tl/build/linux-x86_64-normal-server-release/images/lib/jfr.jar',
 needed by `all'.  Stop.
 make[2]: *** Waiting for unfinished jobs
 Creating images/lib/ext/dnsns.jar
 Creating images/lib/ext/cldrdata.jar
 make[1]: *** [images] Error 2
 make: *** [images-only] Error 2

>
> That is quite puzzling  I will investigate.



 thanks for puzzling
 Andreas

>
> Thanks,
> David
>
>> Andreas
>>
>>
>> On 20.08.13 01:33, David Holmes wrote:
>>>
>>>
>>> http://cr.openjdk.java.net/~dholmes/8023311/webrev/
>>>
>>> Patch inlined below
>>>
>>> This is a trivial cleanup following on from an earlier change under
>>> 8017570. JFR was moved from profile compact3 to the full JRE but not
>>> all
>>> the variables in profile-includes.txt were updated as needed. jfr.jar
>>> needed to removed from the profile3 jar list and added to the jre jar
>>> list.
>>>
>>> This has no impact on the actual build artifacts - JFR was, and
>>> remains,
>>> in the full JRE only.
>>>
>>> Thanks,
>>> David
>>>
>>> --- old/makefiles/profile-includes.txt2013-08-19
>>> 19:10:17.554355677
>>> -0400
>>> +++ new/makefiles/profile-includes.txt2013-08-19
>>> 19:10:14.586187808
>>> -0400
>>> @@ -102,6 +102,7 @@
>>>security/US_export_policy.jar \
>>>security/local_policy.jar
>>>
>>> +
>>>PROFILE_2_JRE_BIN_FILES := \
>>>rmid$(EXE_SUFFIX) \
>>>rmiregistry$(EXE_SUFFIX)
>>> @@ -140,7 +141,6 @@
>>>PROFILE_3_JRE_OTHER_FILES :=
>>>
>>>PROFILE_3_JRE_JAR_FILES := \
>>> -jfr.jar \
>>>management-agent.jar
>>>
>>>
>>> @@ -253,6 +253,6 @@
>>>ext/cldrdata.jar \
>>>ext/dnsns.jar \
>>>ext/nashorn.jar \
>>> -ext/zipfs.jar
>>> -
>>> +ext/zipfs.jar \
>>> +jfr.jar
>>>
>>

>>>
>

Re: Simple RFR: 8203311 Clean up profile-includes.txt

[Andreas Rieber]

Hi Dawid,

the fix is in jdk8tl already, just missed to come into jdk8. It's only a 
few lines.


Andreas

Here the changeset:

Changeset: 3b8fed46b2a8
Author:dholmes
Date:  2013-08-21 05:56 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3b8fed46b2a8

8023460: OPENJDK build fails due to missing jfr.jar
Reviewed-by: alanb

! makefiles/Profiles.gmk


On 28.08.13 11:44, Dawid Weiss wrote:

David,

Is the fix for this committed anywhere? I've just tried building from
scratch and I'm getting the same error.

Dawid

On Wed, Aug 21, 2013 at 10:55 AM, David Holmes  wrote:

Fix on the way. This exposed another piece that was missing from the
original change. But mea culpa for not doing an OPENJDK build. Sorry about
that.

Thanks,
David


On 21/08/2013 6:31 PM, Andreas Rieber wrote:


Hi David,

On 21.08.13 09:53, David Holmes wrote:


Hi Andreas,

On 21/08/2013 4:44 PM, Andreas Rieber wrote:


Hi David,

your change causes build problems with ubuntu 12.04. Result is:

make[2]: Entering directory `/usr/local/src/jdk8tl/jdk/makefiles'
make[2]: *** No rule to make target

`/usr/local/src/jdk8tl/build/linux-x86-normal-server-release/images/lib/jfr.jar',


needed by `all'.  Stop.
make[2]: Leaving directory `/usr/local/src/jdk8tl/jdk/makefiles'
make[1]: *** [images] Error 2
make[1]: Leaving directory `/usr/local/src/jdk8tl/jdk/makefiles'
make: *** [images-only] Error 2

I tried several make clean, configure and so on but only when i rollback
your change i can compile.



I presume this is an OpenJDK build?



Yes, openJDK build. Just tried clean fresh build on ubuntu 13.04 with
jdk8tl, same result:

## Starting images
Creating images/lib/charsets.jar
make[2]: *** No rule to make target

`/usr/local/src/jdk8tl/build/linux-x86_64-normal-server-release/images/lib/jfr.jar',
needed by `all'.  Stop.
make[2]: *** Waiting for unfinished jobs
Creating images/lib/ext/dnsns.jar
Creating images/lib/ext/cldrdata.jar
make[1]: *** [images] Error 2
make: *** [images-only] Error 2



That is quite puzzling  I will investigate.



thanks for puzzling
Andreas



Thanks,
David


Andreas


On 20.08.13 01:33, David Holmes wrote:


http://cr.openjdk.java.net/~dholmes/8023311/webrev/

Patch inlined below

This is a trivial cleanup following on from an earlier change under
8017570. JFR was moved from profile compact3 to the full JRE but not
all
the variables in profile-includes.txt were updated as needed. jfr.jar
needed to removed from the profile3 jar list and added to the jre jar
list.

This has no impact on the actual build artifacts - JFR was, and
remains,
in the full JRE only.

Thanks,
David

--- old/makefiles/profile-includes.txt2013-08-19 19:10:17.554355677
-0400
+++ new/makefiles/profile-includes.txt2013-08-19 19:10:14.586187808
-0400
@@ -102,6 +102,7 @@
   security/US_export_policy.jar \
   security/local_policy.jar

+
   PROFILE_2_JRE_BIN_FILES := \
   rmid$(EXE_SUFFIX) \
   rmiregistry$(EXE_SUFFIX)
@@ -140,7 +141,6 @@
   PROFILE_3_JRE_OTHER_FILES :=

   PROFILE_3_JRE_JAR_FILES := \
-jfr.jar \
   management-agent.jar


@@ -253,6 +253,6 @@
   ext/cldrdata.jar \
   ext/dnsns.jar \
   ext/nashorn.jar \
-ext/zipfs.jar
-
+ext/zipfs.jar \
+jfr.jar

Re: RFR: 7057785 : (xs) Add note to hashCode() that support for self referential is optional

[Stephen Colebourne]

I lke the idea, but the wording feels a little opaque as the result is
typically StackOverflow.

Also, I prefer a style with the @apiNote on a line of its own, rather
like a heading. It makes the documentation easier to read in source
code, and has no effect on the output Javadoc.

@apiNote
If the Collection is self-referential, where it directly or indirectly
contains itself, then the calculation of hashCode may fail with an
exception. Implementations may optionally try to handle this scenario,
however most current implementations do not do so.

Stephen

On 28 August 2013 03:06, Mike Duigou  wrote:
> Hello all;
>
> Fairly frequently it is reported that various Collection/Map implementations 
> of hashCode() fail when the instance directly or indirectly contains itself. 
> For a variety of reasons, mostly performance and resource related, most 
> implementations choose not to support calculation of hash codes for 
> self-referential collections. This is not likely to change. So to reduce 
> confusion and "bug" reports I am proposing a non-normative @apiNote be added 
> to Collection and HashMap. The text of the proposed note is:
>
>> Support for calculation of hash code by self referential {Collection|Map}s 
>> (they either directly or indirectly contain themselves) is optional. Few 
>> Collection implementations support calculation of hash code for self 
>> referential instances.
>
>
> http://cr.openjdk.java.net/~mduigou/JDK-7057785/0/webrev/
>
> Cheers,
>
> Mike

Re: Simple RFR: 8203311 Clean up profile-includes.txt

[Dawid Weiss]

David,

Is the fix for this committed anywhere? I've just tried building from
scratch and I'm getting the same error.

Dawid

On Wed, Aug 21, 2013 at 10:55 AM, David Holmes  wrote:
> Fix on the way. This exposed another piece that was missing from the
> original change. But mea culpa for not doing an OPENJDK build. Sorry about
> that.
>
> Thanks,
> David
>
>
> On 21/08/2013 6:31 PM, Andreas Rieber wrote:
>>
>> Hi David,
>>
>> On 21.08.13 09:53, David Holmes wrote:
>>>
>>> Hi Andreas,
>>>
>>> On 21/08/2013 4:44 PM, Andreas Rieber wrote:

 Hi David,

 your change causes build problems with ubuntu 12.04. Result is:

 make[2]: Entering directory `/usr/local/src/jdk8tl/jdk/makefiles'
 make[2]: *** No rule to make target

 `/usr/local/src/jdk8tl/build/linux-x86-normal-server-release/images/lib/jfr.jar',


 needed by `all'.  Stop.
 make[2]: Leaving directory `/usr/local/src/jdk8tl/jdk/makefiles'
 make[1]: *** [images] Error 2
 make[1]: Leaving directory `/usr/local/src/jdk8tl/jdk/makefiles'
 make: *** [images-only] Error 2

 I tried several make clean, configure and so on but only when i rollback
 your change i can compile.
>>>
>>>
>>> I presume this is an OpenJDK build?
>>
>>
>> Yes, openJDK build. Just tried clean fresh build on ubuntu 13.04 with
>> jdk8tl, same result:
>>
>> ## Starting images
>> Creating images/lib/charsets.jar
>> make[2]: *** No rule to make target
>>
>> `/usr/local/src/jdk8tl/build/linux-x86_64-normal-server-release/images/lib/jfr.jar',
>> needed by `all'.  Stop.
>> make[2]: *** Waiting for unfinished jobs
>> Creating images/lib/ext/dnsns.jar
>> Creating images/lib/ext/cldrdata.jar
>> make[1]: *** [images] Error 2
>> make: *** [images-only] Error 2
>>
>>>
>>> That is quite puzzling  I will investigate.
>>
>>
>> thanks for puzzling
>> Andreas
>>
>>>
>>> Thanks,
>>> David
>>>
 Andreas


 On 20.08.13 01:33, David Holmes wrote:
>
> http://cr.openjdk.java.net/~dholmes/8023311/webrev/
>
> Patch inlined below
>
> This is a trivial cleanup following on from an earlier change under
> 8017570. JFR was moved from profile compact3 to the full JRE but not
> all
> the variables in profile-includes.txt were updated as needed. jfr.jar
> needed to removed from the profile3 jar list and added to the jre jar
> list.
>
> This has no impact on the actual build artifacts - JFR was, and
> remains,
> in the full JRE only.
>
> Thanks,
> David
>
> --- old/makefiles/profile-includes.txt2013-08-19 19:10:17.554355677
> -0400
> +++ new/makefiles/profile-includes.txt2013-08-19 19:10:14.586187808
> -0400
> @@ -102,6 +102,7 @@
>   security/US_export_policy.jar \
>   security/local_policy.jar
>
> +
>   PROFILE_2_JRE_BIN_FILES := \
>   rmid$(EXE_SUFFIX) \
>   rmiregistry$(EXE_SUFFIX)
> @@ -140,7 +141,6 @@
>   PROFILE_3_JRE_OTHER_FILES :=
>
>   PROFILE_3_JRE_JAR_FILES := \
> -jfr.jar \
>   management-agent.jar
>
>
> @@ -253,6 +253,6 @@
>   ext/cldrdata.jar \
>   ext/dnsns.jar \
>   ext/nashorn.jar \
> -ext/zipfs.jar
> -
> +ext/zipfs.jar \
> +jfr.jar
>

>>
>

Re: RFR JDK-8023713: ZipFileSystem has compatiable issue to handle old zip file.

[Chris Hegarty]

On 28/08/2013 00:09, Xueming Shen wrote:

On 08/27/2013 03:07 PM, Martin Buchholz wrote:

It does seem vaguely reasonable to support any extra data.

Don't you want to also handle arbitrary byte arrays, if e.g. one the
16-bit size fields overflows the extra data?
It looks to me like getExtraLen could return a negative number.



probably I should. The webrev has been updated to simply
copy the rest of the extra data, if the "sz" is either < 0 or
out of the range.

http://cr.openjdk.java.net/~sherman/8023713/webrev/


Looks ok to me Sherman.

-Chris.





And put a SPACE after "if".


updated.

Thanks!
-Sherman




On Tue, Aug 27, 2013 at 2:42 PM, Xueming Shen mailto:xueming.s...@oracle.com>> wrote:

Hi,

Please help review the change for #8023713

http://cr.openjdk.java.net/~sherman/8023713/webrev


The root cause is that the newly introduced ZOS.writeExtra() (for
#8015666) fails to handle "irregular" extra data field. The zip spec
requires the the extra data stars with 4 bytes of "tag + size" pair
and then followed by the actual "extra data". The "offending" zip
file actually has the "irregular" extra data field with 1 single byte
as the extra data. That said, the implementation (ZOS) should still
be able handle this kind of zip entry correctly and appropriately.

The proposed solution is to simply copy the specified extra data
into the output stream.

Thanks!
-Sherman