Re: RFR: 7174936: several String methods claim to always create new String

2013-11-07 Thread Stuart Marks

On 11/6/13 6:15 PM, David Holmes wrote:

On 7/11/2013 11:31 AM, Stuart Marks wrote:

In several places the specs mention returning new strings. This is
overspecified; it's sufficient to return a string that satisfies the
required properties. In some cases the current implementation doesn't
create a new string -- for example, substring(0) returns 'this' -- which
is strictly a violation of the spec. The solution is to relax the spec
requirement to create new strings.


Or modify it as per concat - which is spec'd to return this when you concat an
empty string.

But I suppose in principle this allows for reuse of existing String objects.


I'm mainly interested in relaxing the spec, allowing the implementation maximum 
freedom to choose whether to create a new string or return a pre-existing 
instance such as 'this', instead of requiring one way or the other.


The concat() method is weird in that if the arg is empty, it requires returning 
'this'; but if 'this' is empty, it requires creating a new String instead of 
just returning the arg. I think this is overspecified. But the implementation 
follows this exactly, so I didn't want to change the spec in this case. Maybe later.



Also, this change cleans up the specs for the copyValueOf() overloads to
make them identical to the corresponding valueOf() overloads.
Previously, they were gratuitously different. I think that some time in
the dim, distant past, probably before JDK 1.0, they had different
semantics, but now they're identical. The specs should say they're
identical.


Don't we normally express this as Equivalent to ... rather than Identical to
... - the latter seems like an overspecification.


Yes, Equivalent to... is better; I'll change it.


Surely this change invalidates any tests that check for new strings as per the
spec? They won't start failing but they will no longer be valid test.


For methods where the implementation violates the spec by not returning a new 
instance, any tests that exist for these cases would be failing, so I'd think 
we'd have heard about this already. So perhaps such tests don't exist.


For methods where the implementation does return a new string, relaxing the spec 
may indeed invalidate tests that test for a new string. Such a test wouldn't 
actually fail until some future time when the implementation is changed.


But yes, this change might have some impact on the JCK. This is going through 
the normal internal review process (CCC) though, so the JCK team should be 
properly informed of the change.



That aside subSequence is specified by CharSequence to return a new
CharSequence. So I don't think you can simply remove that requirement.


Oh yes, good catch. I had originally modified subSequence to change all the 
mentions of CharSequence to String -- since String.subSequence() merely calls 
String.substring(). After talking to Mike Duigou about this we decided that it 
would be better to leave these all as CharSequence. I changed them all back, but 
I missed this one in the @return clause. I'll fix this too.


Thanks,

s'marks




Cheers,
David
-


Bug report:

 https://bugs.openjdk.java.net/browse/jdk-7174936

Webrev:

 http://cr.openjdk.java.net/~smarks/reviews/7174936/webrev.0/

Specdiff:


http://cr.openjdk.java.net/~smarks/reviews/7174936/specdiff.0/overview-summary.html



Thanks!

s'marks


Re: hg: jdk8/tl/jdk: 7194897: JSR 292: Cannot create more than 16 instances of an anonymous class; ...

2013-11-07 Thread Peter Levart

On 11/07/2013 02:20 AM, John Rose wrote:
On Nov 6, 2013, at 11:30 AM, Peter Levart peter.lev...@gmail.com 
mailto:peter.lev...@gmail.com wrote:


Well, indexOf(char) or lastIndexOf(char) searches for a single char. 
We can do better searching backwards for two chars at the same time.


If the name of VM-anonymous class is always ending with pattern: 
slash followed by some decimal digits then the following would be 
even faster:


Although this reasoning is plausible, it is not a reliable conclusion, 
and should not drive edits of Java code without careful measurement. 
 The reasoning assumes a performance model based on the interpreter 
and bytecode count.  But performance depends on native code produced 
by the JIT.


I agree. I should have measured it...



An optimizing JIT will usually transform the code deeply.  For string 
scanning, for example, HotSpot has an intrinsic for 
String.indexOf(String) that uses totally different code from a 
user-coded loop.  (It is not currently so for String.indexOf(int), but 
String.indexOf(/) is potentially very fast.)


Also, with your example code, the combined loop may often be faster 
than two back-to-back loops, but simpler loops can sometimes be 
vectorized more robustly, to the point where back-to-back simple 
loops, if vectorized, may be competitive with a hand-fused loop.


So loop complexity and method intrinsics can create surprises for 
those who rely on simple performance modesl


Some day we will get to a world where loops are specified stream-wise, 
and robustly optimized without this sort of manual intervention.  In 
the mean time, be careful about advising hand-optimizations of Java 
code.  They can backfire, by confusing the JIT and preventing 
optimizations that would apply to simpler code.


— John


So I did measure it. I took two classes with the following names:

com.test.pkg.PerfTest$Classic$1
com.test.pkg.PerfTest$$Lambda$1/925858445

Typically package names will be even relatively longer than in this example.

I measured the following implementations:

public static boolean isVMAnonymousClass(Class? cls) {
return cls.getSimpleName().contains(/);
}

public static boolean isVMAnonymousClass_FAST1(Class? cls) {
String name = cls.getName();
for (int i = name.length() - 1; i = 0; i--) {
char c = name.charAt(i);
if (c == '.') return false;
if (c == '/') return true;
}
return false;
}

public static boolean isVMAnonymousClass_FAST2(Class? cls) {
String name = cls.getName();
for (int i = name.length() - 1; i = 0; i--) {
char c = name.charAt(i);
if (c == '/') return true;
if (c  '0' || c  '9') return false;
}
return false;
}

public static boolean isVMAnonymousClass_indexOf(Class? cls) {
return cls.getName().indexOf(/)  -1;
}


I also tried String.lastIndexOf(String) and it is a little faster for 
true return, since it only scans backwards until the / is found, but 
it is slower than String.indexOf(String) for false return. Is it not 
intrinsified?


Here are the results:

http://cr.openjdk.java.net/~plevart/jdk8-tl/isVmAnonymousClass/results.txt

I think that any of the above implementations that doesn't use 
Class.getSimpleName() is good. The FAST2 variant is favourable for false 
return since it typically decides after examining a single character at 
end of class name and is still among the fastest for true return...


Here's the code I used for testing:

http://cr.openjdk.java.net/~plevart/jdk8-tl/isVmAnonymousClass/com/test/pkg/PerfTest.java
http://cr.openjdk.java.net/~plevart/jdk8-tl/isVmAnonymousClass/si/pele/microbench/TestRunner.java


Regards, Peter



Re: RFR for JDK-8019502 some java.util test does not pass VM options when launching java program in script

2013-11-07 Thread Alan Bateman

On 07/11/2013 03:48, Patrick Zhang wrote:

Hi Everyone,

I am working on https://bugs.openjdk.java.net/browse/JDK-8019502. The 
problem is, some java.util test does use ${TESTVMOPTS} to launch java 
program in script. Then it may lose VM setting from high level.


The fix will refer to https://bugs.openjdk.java.net/browse/JDK-8003890 
finished by Mark Sheppard. Just add ${TESTVMOPTS} to launch java.

Did you intend to include a link to a patch?

-Alan.


hg: jdk8/tl/jdk: 8027822: ProblemList.txt Updates (11/2013)

2013-11-07 Thread chris . hegarty
Changeset: f37d62e295c0
Author:chegar
Date:  2013-11-07 08:04 +
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/f37d62e295c0

8027822: ProblemList.txt Updates (11/2013)
Reviewed-by: chegar, alanb
Contributed-by: Amy Lu amy...@oracle.com

! test/ProblemList.txt



Re: RFR: 8027822: ProblemList.txt Updates (11/2013)

2013-11-07 Thread Chris Hegarty

Sorry for the delay Amy.

Trivially, 'windows-amd64, solaris-amd64, linux-amd64, macosx-all' was 
not matching the 64bit versions of these OSes, neither was s/amd64/x64/. 
So I changed it to generic-all for now. I did run into this myself a 
while back, but can't remember the correct labels. Not to worry, these 
are temporary additions and will be removed in a few weeks.


Thanks,
-Chris.

On 11/07/2013 04:29 AM, Amy Lu wrote:


On 11/6/13 10:45 AM, Amy Lu wrote:

On 11/5/13 7:09 PM, Amy Lu wrote:

On 11/5/13 6:01 PM, Alan Bateman wrote:

On 05/11/2013 09:26, Amy Lu wrote:

:

Please review ProblemList.txt changes:
https://dl.dropboxusercontent.com/u/5812451/yl153753/8027822/webrev/index.html


This looks okay to me although I'm not sure about
sun/util/resources/TimeZone/Bug6317929.java (for some reason Aleksej
fixed the test and also excluded it, pending a decision/discussion
on whether this test should be removed).


Aleksej,

Should this test be removed from ProblemList?

Thanks,
Amy



Should we use the opportunity to add all the tests that are failing
due to the exact math intrinsic (JDK-8027622)?

-Alan.


Added 8027622 and affected tests into ProblemList:
https://dl.dropboxusercontent.com/u/5812451/yl153753/8027822/webrev.01/index.html


Thanks,
Amy



If it's pending decision on whether
sun/util/resources/TimeZone/Bug6317929.java should be removed, maybe it
can be updated in separate change.

I'm adding it back to ProblemList and here's the updated changes:
https://dl.dropboxusercontent.com/u/5812451/yl153753/8027822/webrev.02/index.html


Thanks,
Amy



Re: RFR: 8027822: ProblemList.txt Updates (11/2013)

2013-11-07 Thread Amy Lu

Thank you Chris !

/Amy

On 11/7/13 5:09 PM, Chris Hegarty wrote:

Sorry for the delay Amy.

Trivially, 'windows-amd64, solaris-amd64, linux-amd64, macosx-all' was 
not matching the 64bit versions of these OSes, neither was 
s/amd64/x64/. So I changed it to generic-all for now. I did run into 
this myself a while back, but can't remember the correct labels. Not 
to worry, these are temporary additions and will be removed in a few 
weeks.


Thanks,
-Chris.

On 11/07/2013 04:29 AM, Amy Lu wrote:


On 11/6/13 10:45 AM, Amy Lu wrote:

On 11/5/13 7:09 PM, Amy Lu wrote:

On 11/5/13 6:01 PM, Alan Bateman wrote:

On 05/11/2013 09:26, Amy Lu wrote:

:

Please review ProblemList.txt changes:
https://dl.dropboxusercontent.com/u/5812451/yl153753/8027822/webrev/index.html 




This looks okay to me although I'm not sure about
sun/util/resources/TimeZone/Bug6317929.java (for some reason Aleksej
fixed the test and also excluded it, pending a decision/discussion
on whether this test should be removed).


Aleksej,

Should this test be removed from ProblemList?

Thanks,
Amy



Should we use the opportunity to add all the tests that are failing
due to the exact math intrinsic (JDK-8027622)?

-Alan.


Added 8027622 and affected tests into ProblemList:
https://dl.dropboxusercontent.com/u/5812451/yl153753/8027822/webrev.01/index.html 




Thanks,
Amy



If it's pending decision on whether
sun/util/resources/TimeZone/Bug6317929.java should be removed, maybe it
can be updated in separate change.

I'm adding it back to ProblemList and here's the updated changes:
https://dl.dropboxusercontent.com/u/5812451/yl153753/8027822/webrev.02/index.html 




Thanks,
Amy





hg: jdk8/tl/jdk: 8027961: Inet[4|6]Address native initializing code should check field/MethodID values

2013-11-07 Thread chris . hegarty
Changeset: 82b276590b85
Author:chegar
Date:  2013-11-07 08:23 +
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/82b276590b85

8027961: Inet[4|6]Address native initializing code should check field/MethodID 
values
Reviewed-by: michaelm, rriggs

! src/share/native/java/net/Inet4Address.c
! src/share/native/java/net/Inet6Address.c
! src/share/native/java/net/InetAddress.c
! src/solaris/native/java/net/Inet4AddressImpl.c
! src/solaris/native/java/net/Inet6AddressImpl.c
! src/windows/native/java/net/Inet4AddressImpl.c
! src/windows/native/java/net/Inet6AddressImpl.c



Re: RFR: 8020779 8026988 : Improvements to JTReg executable location handling

2013-11-07 Thread Mike Duigou

On Nov 7 2013, at 01:24 , Erik Joelsson erik.joels...@oracle.com wrote:

 Hello Mike,
 
 I think this looks good. A couple of comments that don't necessarily call for 
 any changes.
 
 The version check is commented out, when is it planned to be activated?

I was working on this piece at the same time as a the make version 4.0 
discussion was happening and I wanted to wait for the solution used there and 
reapply it. I can complete this code before commit.

 
 I see you use 'which' as a fallback in the makefiles. Just be aware that we 
 have had some trouble using the which command in the past. Sometimes on 
 Solaris it gives unexpected output depending on your environment and the 
 behavior on fail is different on various platforms and implementations. It 
 looks like you are trying to handle all the weird cases though.

The pattern I used is copied from hgforest.sh where the problems were indeed 
previously encountered. I would prefer to have no fallback at all and plan to 
eventually remove it in a future changeset. The current fallback behaviour is 
intended to help with the transition from the old build system to the new build 
infrastructure and the variety of usage patterns for test/Makefile which are 
still being used.

It will be a week or two before I try to finalize this changeset.

Thanks,

Mike

 /Erik
 
 On 2013-11-06 21:40, Mike Duigou wrote:
 Hello all;
 
 With JDK-8015068 only very recently pushed it's time for the next set of 
 jtreg test/Makfile changes. These changes improve the configure script 
 detection of jtreg and improve the handling of the specification variables 
 in the jdk and langtools test/Makefiles. This changeset is also the first 
 step in syncing the two test/Makefiles. More will follow which will further 
 simplify the integration of testing into the overall build/test process.
 
 http://cr.openjdk.java.net/~mduigou/JDK-8020779/0/webrev/
 
 Mike
 



RFR: 8022213 Intermittent test failures in java/net/URLClassLoader (Add jdk/testlibrary/FileUtils.java)

2013-11-07 Thread Chris Hegarty
Virus checkers and, on more modern Windows OSes, Windows Experience 
Service, and other external processes, have been causing trouble when it 
comes to tests deleting files.


On a number of occasions in the past retry logic has been added to tests 
that need to delete files. Also, the jtreg harness has similar logic 
when cleaning up temporary files.


This has reared its head again with:
  8027902: TEST_BUG: java/net/URLClassLoader/closetest/CloseTest.java 
and java/net/URLClassLoader/closetest/GetResourceAsStream.java failed 
due to dir operation failed.

  8022213: Intermittent test failures in java/net/URLClassLoader

I'd like to add file utility functions to the common jdk/testlibrary, 
starting with a more reliable delete. I've made minimal changes to the 
URLClassloader tests to use this ( more could be done, but this is just 
a start ).


http://cr.openjdk.java.net/~chegar/fileUtils/webrev/

The important part here for review is the API and impl in FileUtils.

Thanks,
-Chris.


Re: RFR: 8020779 8026988 : Improvements to JTReg executable location handling

2013-11-07 Thread Magnus Ihse Bursie

On 2013-11-06 21:40, Mike Duigou wrote:

Hello all;

With JDK-8015068 only very recently pushed it's time for the next set of jtreg 
test/Makfile changes. These changes improve the configure script detection of 
jtreg and improve the handling of the specification variables in the jdk and 
langtools test/Makefiles. This changeset is also the first step in syncing the 
two test/Makefiles. More will follow which will further simplify the 
integration of testing into the overall build/test process.

http://cr.openjdk.java.net/~mduigou/JDK-8020779/0/webrev/


I don't think the changes in toolchain.m4 does what you want it to do. 
The AC_ARG_WITH function is not really well designed. In particular, it 
is missing a default section. So if you use the functionality to 
execute code on yes and no, then no will be the same wether or not 
you leave the option out alltogether, or try call it like 
--without-jtreg (or --with-jtreg=no, which is the same as --without).


Our typical use pattern, to get around this, is to not execute any code 
in the AC_ARG_WITH function, and instead check the $with_argname option 
afterwards. There you can separate the different use patterns:


* $with_argname=yes -- user explicitely set --with-argname (or 
--with-argname=yes)
* $with_argname=no -- user explicitely set --without-argname (or 
--with-argname=no)
* $with_argname= (empty) -- user did not specify flag at all; do 
default action.
* $with_argname=something else  -- user explicitely set 
--with-argname=something else, most likely trying to override a path.


In this case, it will no longer be possible to explicitely disable jtreg 
with your changes. (If I read the code correctly -- I have not tried 
running it).


Also, the pair AC_MSG_CHECKING and AC_MSG_RESULT needs to be close 
together. If any output arrives inbetween them, the output will look 
garbled, like:


checking for foo... yes.
checking for jtreg... Rewriting path for JT_HOME to /localhome/jthome
yes.
checking for bar... no.

I tend to use the pair just consecutively after I actually found out 
what I was looking for, to avoid this. Then it's really just useful for 
the log output, not to show progress. If the test I'm about to make 
might take a non-significant amount of time, I might start by a 
AC_MSG_NOTICE([Trying to locate package foo]) or similar.


Also, if the search failed, and a AC_MSG_CHECKING had been printed, an 
AC_MSG_RESULT([no]) should be printed before AC_MSG_ERROR.


/Magnus


Re: RFR: 8027822: ProblemList.txt Updates (11/2013)

2013-11-07 Thread Aleksej Efimov

Hi Amy,
This test is currently fixed and working as expected, but I have a 
desire to remove this test. In fact, this test (and my decision to fix 
and exclude it) already brings too many question, it doesn't do any 
helpful testing and all situation with adding fixed test to 
ProblemsList.txt looks awkward. I planned to remove it as part of 
JDK-8025051, but it will be resolved only in next 1-2 weeks. So, I 
suggest to exclude it from problems list and remove it now.


Thank you,
Aleksej

On 11/07/2013 08:29 AM, Amy Lu wrote:


On 11/6/13 10:45 AM, Amy Lu wrote:

On 11/5/13 7:09 PM, Amy Lu wrote:

On 11/5/13 6:01 PM, Alan Bateman wrote:

On 05/11/2013 09:26, Amy Lu wrote:

:

Please review ProblemList.txt changes:
https://dl.dropboxusercontent.com/u/5812451/yl153753/8027822/webrev/index.html 

This looks okay to me although I'm not sure about 
sun/util/resources/TimeZone/Bug6317929.java (for some reason 
Aleksej fixed the test and also excluded it, pending a 
decision/discussion on whether this test should be removed).


Aleksej,

Should this test be removed from ProblemList?

Thanks,
Amy



Should we use the opportunity to add all the tests that are failing 
due to the exact math intrinsic (JDK-8027622)?


-Alan.


Added 8027622 and affected tests into ProblemList:
https://dl.dropboxusercontent.com/u/5812451/yl153753/8027822/webrev.01/index.html 



Thanks,
Amy


If it's pending decision on whether 
sun/util/resources/TimeZone/Bug6317929.java should be removed, maybe 
it can be updated in separate change.


I'm adding it back to ProblemList and here's the updated changes:
https://dl.dropboxusercontent.com/u/5812451/yl153753/8027822/webrev.02/index.html 



Thanks,
Amy





Re: RFR: 8027822: ProblemList.txt Updates (11/2013)

2013-11-07 Thread Amy Lu

Thank you for confirm.
And yes, removed.

Thanks,
Amy

On 11/7/13 6:36 PM, Aleksej Efimov wrote:

Hi Amy,
This test is currently fixed and working as expected, but I have a 
desire to remove this test. In fact, this test (and my decision to fix 
and exclude it) already brings too many question, it doesn't do any 
helpful testing and all situation with adding fixed test to 
ProblemsList.txt looks awkward. I planned to remove it as part of 
JDK-8025051, but it will be resolved only in next 1-2 weeks. So, I 
suggest to exclude it from problems list and remove it now.


Thank you,
Aleksej

On 11/07/2013 08:29 AM, Amy Lu wrote:


On 11/6/13 10:45 AM, Amy Lu wrote:

On 11/5/13 7:09 PM, Amy Lu wrote:

On 11/5/13 6:01 PM, Alan Bateman wrote:

On 05/11/2013 09:26, Amy Lu wrote:

:

Please review ProblemList.txt changes:
https://dl.dropboxusercontent.com/u/5812451/yl153753/8027822/webrev/index.html 

This looks okay to me although I'm not sure about 
sun/util/resources/TimeZone/Bug6317929.java (for some reason 
Aleksej fixed the test and also excluded it, pending a 
decision/discussion on whether this test should be removed).


Aleksej,

Should this test be removed from ProblemList?

Thanks,
Amy



Should we use the opportunity to add all the tests that are 
failing due to the exact math intrinsic (JDK-8027622)?


-Alan.


Added 8027622 and affected tests into ProblemList:
https://dl.dropboxusercontent.com/u/5812451/yl153753/8027822/webrev.01/index.html 



Thanks,
Amy


If it's pending decision on whether 
sun/util/resources/TimeZone/Bug6317929.java should be removed, maybe 
it can be updated in separate change.


I'm adding it back to ProblemList and here's the updated changes:
https://dl.dropboxusercontent.com/u/5812451/yl153753/8027822/webrev.02/index.html 



Thanks,
Amy







hg: jdk8/tl/jdk: 8027881: test/java/net/URLPermission/nstest/LookupTest.java failing intermittently, output insufficient

2013-11-07 Thread michael . x . mcmahon
Changeset: 88d1ed05a246
Author:michaelm
Date:  2013-11-07 10:22 +
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/88d1ed05a246

8027881: test/java/net/URLPermission/nstest/LookupTest.java failing 
intermittently, output insufficient
Reviewed-by: chegar

! test/java/net/URLPermission/URLPermissionTest.java
! test/java/net/URLPermission/nstest/LookupTest.java
+ test/java/net/URLPermission/nstest/lookup.sh
- test/java/net/URLPermission/nstest/policy



Re: RFR: 8022213 Intermittent test failures in java/net/URLClassLoader (Add jdk/testlibrary/FileUtils.java)

2013-11-07 Thread Michael McMahon

Chris,

Would it be useful to add some instrumentation/logging (to System.err) 
if it's taking

more than one iteration to delete a file? We could end up with degraded
test performance if there is a general problem deleting files, and 
otherwise would have
no way of understanding what the problem is (eg. up to 7.5 seconds are 
allowed to delete the file)


Also, just curious what is the value in throwing InterruptedException 
out of the delete() method?

It seems onerous for calling code to have to catch it.

Michael

On 07/11/13 10:05, Chris Hegarty wrote:
Virus checkers and, on more modern Windows OSes, Windows Experience 
Service, and other external processes, have been causing trouble when 
it comes to tests deleting files.


On a number of occasions in the past retry logic has been added to 
tests that need to delete files. Also, the jtreg harness has similar 
logic when cleaning up temporary files.


This has reared its head again with:
  8027902: TEST_BUG: java/net/URLClassLoader/closetest/CloseTest.java 
and java/net/URLClassLoader/closetest/GetResourceAsStream.java failed 
due to dir operation failed.

  8022213: Intermittent test failures in java/net/URLClassLoader

I'd like to add file utility functions to the common jdk/testlibrary, 
starting with a more reliable delete. I've made minimal changes to 
the URLClassloader tests to use this ( more could be done, but this is 
just a start ).


http://cr.openjdk.java.net/~chegar/fileUtils/webrev/

The important part here for review is the API and impl in FileUtils.

Thanks,
-Chris.




Re: [8] Review Request: 8027696 Incorrect copyright header in the tests

2013-11-07 Thread Alan Bateman

On 05/11/2013 16:28, Sergey Bylokhov wrote:

Hello,
Updated version:
http://cr.openjdk.java.net/~serb/8027696/webrev.01/
Dates and spaces were fixed.

Thanks Sergey, I sampled a few and they look correct.

The only thing is that I'm not sure about is test/Makefile, it's just 
not clear whether which header this should have.


-Alan


Re: RFR: 8022213 Intermittent test failures in java/net/URLClassLoader (Add jdk/testlibrary/FileUtils.java)

2013-11-07 Thread Chris Hegarty

On 11/07/2013 11:19 AM, Michael McMahon wrote:

Chris,

Would it be useful to add some instrumentation/logging (to System.err)
if it's taking
more than one iteration to delete a file? We could end up with degraded
test performance if there is a general problem deleting files, and
otherwise would have
no way of understanding what the problem is (eg. up to 7.5 seconds are
allowed to delete the file)


I had an optional PrintStream param on delete/deleteTree, but removed 
it. If the file fails to delete, then you will see all the suppressed 
exceptions, but I agree if it fails a number of times and then succeeds 
you do not see this.


I'm not sure that it is worth adding PrintStream ( and this is why I 
removed it ) since the external interference occurs infrequently and 
does not appear to last long. I just wanted to keep this as simply as 
possible, but I could be convinced to reintroduce it.



Also, just curious what is the value in throwing InterruptedException
out of the delete() method?
It seems onerous for calling code to have to catch it.


Right handling this is a pain. Since we may now be possibly sleeping 
during a delete I wanted jtreg to be able to interrupt the test and have 
the test take correct action, rather than just swallowing it. It is 
really annoying to see hung tests in logs that do not respond to jtregs 
attempts to stop them.


I've also received another comment offline about the method names. They 
should be more descriptive, and highlight the difference between these 
methods and regular delete. So maybe move to public deleteWithRetry?


-Chris.



Michael

On 07/11/13 10:05, Chris Hegarty wrote:

Virus checkers and, on more modern Windows OSes, Windows Experience
Service, and other external processes, have been causing trouble when
it comes to tests deleting files.

On a number of occasions in the past retry logic has been added to
tests that need to delete files. Also, the jtreg harness has similar
logic when cleaning up temporary files.

This has reared its head again with:
  8027902: TEST_BUG: java/net/URLClassLoader/closetest/CloseTest.java
and java/net/URLClassLoader/closetest/GetResourceAsStream.java failed
due to dir operation failed.
  8022213: Intermittent test failures in java/net/URLClassLoader

I'd like to add file utility functions to the common jdk/testlibrary,
starting with a more reliable delete. I've made minimal changes to
the URLClassloader tests to use this ( more could be done, but this is
just a start ).

http://cr.openjdk.java.net/~chegar/fileUtils/webrev/

The important part here for review is the API and impl in FileUtils.

Thanks,
-Chris.




hg: jdk8/tl/jdk: 8027796: Refactor Core Reflection for Type Annotations

2013-11-07 Thread joel . franck
Changeset: 44fa6bf42846
Author:jfranck
Date:  2013-11-07 13:33 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/44fa6bf42846

8027796: Refactor Core Reflection for Type Annotations
Reviewed-by: psandoz

! src/share/classes/sun/reflect/annotation/AnnotatedTypeFactory.java
! src/share/classes/sun/reflect/annotation/TypeAnnotation.java
! src/share/classes/sun/reflect/annotation/TypeAnnotationParser.java



Re: RFR: JDK-8027796: Refactor Core Reflection for Type Annotations

2013-11-07 Thread Joel Borggrén-Franck
Thanks for the review Paul

cheers
/Joel

On 2013-11-05, Paul Sandoz wrote:
 
 On Nov 4, 2013, at 8:36 PM, Joel Borggrén-Franck joel.fra...@oracle.com 
 wrote:
 
  Hi
  
  Please review this small refactoring of the type annotations reflection 
  code. This fix just makes the types final since the code wasn't designed 
  for inheritance.
  
  webrev: http://cr.openjdk.java.net/~jfranck/8027796/webrev.00/
  jbs: https://bugs.openjdk.java.net/browse/JDK-8027796
  
 
 +1
 
 FWIW (take it or leave it) the final on 
 AnnotatedTypeFactory.buildAnnotatedType is redundant since 
 AnnotatedTypeFactory is final and thus cannot be overridden with another 
 class that shadows the static method.
 
 Paul.




Re: RFR: 8022213 Intermittent test failures in java/net/URLClassLoader (Add jdk/testlibrary/FileUtils.java)

2013-11-07 Thread Alan Bateman

On 07/11/2013 11:34, Chris Hegarty wrote:

:

I've also received another comment offline about the method names. 
They should be more descriptive, and highlight the difference between 
these methods and regular delete. So maybe move to public 
deleteWithRetry?
That might be a bit better as otherwise it's not obvious how it differs 
from Files.delete. Also to keep the names consistent then 
deleteTreeWithRetry should probably be deleteFileTreeWithRetry.


On interrupting the sleep then it looks like the methods aren't 
consistent, one will throw InterruptedException, the other will complete 
early without an exception and without the interrupt status set.


If you want then you could extend SimpleFileVisitor instead, that would 
allow you to drop the preVisitDirectory method.


A passing comment on closetest/Command is that the copyFile method could 
use Files.copy with REPLACE_EXISTING.


Also rm_minus_rf (amusing name) doesn't need to use isFile or 
isDirectory as FilesUtil.deleteFIleTree will do the right thing.


-Alan.


Re: [8] Review Request: 8027696 Incorrect copyright header in the tests

2013-11-07 Thread Sergey Bylokhov

On 07.11.2013 15:22, Alan Bateman wrote:

Thanks Sergey, I sampled a few and they look correct.

The only thing is that I'm not sure about is test/Makefile, it's just 
not clear whether which header this should have.

I can skip it this time.

--
Best regards, Sergey.



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

2013-11-07 Thread Rob McKenna

Ah, thanks for catching that David, I've updated the webrev in place.

I believe the reasoning is that we want to minimise any potential impact 
to customers running in production. The general feeling seems to be that 
Mac is a development platform and is less likely to cause us problems in 
that regard. (not that I would anticipate any on Solaris, but I 
understand the desire to be conservative)


-Rob

On 07/11/13 01:51, David Holmes wrote:

On 6/11/2013 10:00 PM, Rob McKenna wrote:

Hi David,

The only difference in 5049299 is the change in the default property
value in Solaris. Apologies for not making that clear.


Given this was primarily aimed at Solaris in the first place it seems 
strange to me to not make the change on Solaris. If this is considered 
risky fr an update release then I would think that applies to all 
platforms equally. ??


If Solaris is not in fact changing then the comment in 
src/solaris/native/java/lang/UNIXProcess_md.c needs to be altered:


   * Based on the above analysis, we are currently using vfork() on
!  * Linux and spawn() on other Unix systems, but the code to use clone()
!  * and fork() remains.

David
-


 -Rob

On 06/11/13 01:09, David Holmes wrote:

Hi Rob,

How different is this to the JDK 8 version?

Thanks,
David

On 6/11/2013 7:24 AM, Rob McKenna wrote:

..

http://cr.openjdk.java.net/~robm/5049299/7/webrev.01/

 -Rob

On 05/11/13 21:23, Rob McKenna wrote:

Hi folks,

I'd like to backport this change to JDK7. Note: this fix uses
posix_spawn by default on Mac OSX only. On Solaris fork will remain
the default behaviour.

I've just noticed that there is a problem with the testcase on 
Solaris

in that it will test fork twice. (and won't test posix_spawn) I'll
figure out a way around this, but in the mean time I'd like to get 
the

ball rolling on this review anyway.

-Rob










hg: jdk8/tl/nashorn: 2 new changesets

2013-11-07 Thread sundararajan . athijegannathan
Changeset: 2f07b4234451
Author:sundar
Date:  2013-11-07 17:26 +0530
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/2f07b4234451

8027828: ClassCastException when converting return value of a Java method to 
boolean
Reviewed-by: jlaskey, attila

! src/jdk/nashorn/api/scripting/ScriptObjectMirror.java
! src/jdk/nashorn/api/scripting/ScriptUtils.java
! src/jdk/nashorn/internal/runtime/JSType.java
! src/jdk/nashorn/internal/runtime/linker/NashornBottomLinker.java
+ test/script/basic/JDK-8027828.js
+ test/script/basic/JDK-8027828.js.EXPECTED
+ test/script/basic/convert.js
+ test/script/basic/convert.js.EXPECTED
! test/src/jdk/nashorn/api/scripting/ScriptObjectMirrorTest.java

Changeset: 3b794f364c77
Author:sundar
Date:  2013-11-07 18:11 +0530
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/3b794f364c77

Merge




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

2013-11-07 Thread Rob McKenna

I've added a review showing the differences to jdk8 to:

http://cr.openjdk.java.net/~robm/5049299/7/webrev.02/

I've updated the previous webrev ( 
http://cr.openjdk.java.net/~robm/5049299/7/webrev.01/ ) in place to 
reflect the comment change.


Note: still putting together a separate test which will call Basic.java 
with the correct parameters on each platform.


-Rob

On 07/11/13 11:46, Alan Bateman wrote:

On 06/11/2013 13:30, Rob McKenna wrote:


Yes, absolutely. I'm looking for approval for the code in principle. 
Particularly the change in the default property value on Solaris 
since thats the only change. Once approved I will of course seek 
approval for integration.


Also, I will also integrate these changes separately and in order. 
8008118 has been sitting in my repository for some time and I totally 
forgot about it until I looked at the webrev.


-Rob
Okay, so will you refresh the webrev and just highlight the 
differences from the jdk8 patch?


-Alan.




Re: RFR: 8022213 Intermittent test failures in java/net/URLClassLoader (Add jdk/testlibrary/FileUtils.java)

2013-11-07 Thread Chris Hegarty

Given both Michael and Alan's comments. I've update the webrev:
  http://cr.openjdk.java.net/~chegar/fileUtils.01/webrev/

1) more descriptive method names
2) deleteXXX methods return if interrupted, leaving the
   interrupt status set
3) Use Files.copy with REPLACE_EXISTING
4) Use SimpleFileVisitor, rather than FileVisitor

Thanks,
-Chris.

On 11/07/2013 01:24 PM, Alan Bateman wrote:

On 07/11/2013 11:34, Chris Hegarty wrote:

:

I've also received another comment offline about the method names.
They should be more descriptive, and highlight the difference between
these methods and regular delete. So maybe move to public
deleteWithRetry?

That might be a bit better as otherwise it's not obvious how it differs
from Files.delete. Also to keep the names consistent then
deleteTreeWithRetry should probably be deleteFileTreeWithRetry.

On interrupting the sleep then it looks like the methods aren't
consistent, one will throw InterruptedException, the other will complete
early without an exception and without the interrupt status set.

If you want then you could extend SimpleFileVisitor instead, that would
allow you to drop the preVisitDirectory method.

A passing comment on closetest/Command is that the copyFile method could
use Files.copy with REPLACE_EXISTING.

Also rm_minus_rf (amusing name) doesn't need to use isFile or
isDirectory as FilesUtil.deleteFIleTree will do the right thing.

-Alan.


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

2013-11-07 Thread Alan Bateman

On 07/11/2013 13:52, Rob McKenna wrote:

I've added a review showing the differences to jdk8 to:

http://cr.openjdk.java.net/~robm/5049299/7/webrev.02/
That makes it easy, thanks. At L100 then I assume you mean posix_spawn 
rather than spawn.


-Alan


Question regarding Entity Expansion in JAXB, -DentityExpansionLimit and 8017298: Better XML support

2013-11-07 Thread Volker Simonis
Hi,

I have a question related to change 8017298: Better XML support
which went into the last security update. Because it was considered a
security fix, there's not much information available (i.e. no webrev,
no bug description, no discussion on the public mailing lists).

As far as I can see, the entityExpansionLimit for JAXB has been
there since Java 5 and according to Blaise Doughan blog at
http://blog.bdoughan.com/2011/03/preventing-entity-expansion-attacks-in.html
it should have been enabled by default together with the
XMLConstants.FEATURE_SECURE_PROCESSING feature.

Now we have a customer who claims that after upgrading to 7u45 he gets
an execption because of too many entity expansions. The customer
explicitly sets -DentityExpansionLimit=1.

For us it seems as if before change 8017298: Better XML support
there must have been places in the libraries which ignored the
entityExpansionLimit setting even if this was explicitly specified
by the user. Can somebody confirm this assumption or is our customer
facing another problem?

Thank you and best regards,
Volker


hg: jdk8/tl/jdk: 8027930: ResourceBundle test failures in fr locale

2013-11-07 Thread naoto . sato
Changeset: fb7abd509bd2
Author:naoto
Date:  2013-11-07 10:03 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/fb7abd509bd2

8027930: ResourceBundle test failures in fr locale
Reviewed-by: smarks

! test/java/util/ResourceBundle/ResourceBundleTest.java
! test/java/util/ResourceBundle/getBaseBundleName/TestGetBaseBundleName.java



Re: Question regarding Entity Expansion in JAXB, -DentityExpansionLimit and 8017298: Better XML support

2013-11-07 Thread Alan Bateman

On 07/11/2013 17:33, Volker Simonis wrote:

Hi,

I have a question related to change 8017298: Better XML support
which went into the last security update. Because it was considered a
security fix, there's not much information available (i.e. no webrev,
no bug description, no discussion on the public mailing lists).

As far as I can see, the entityExpansionLimit for JAXB has been
there since Java 5 and according to Blaise Doughan blog at
http://blog.bdoughan.com/2011/03/preventing-entity-expansion-attacks-in.html
it should have been enabled by default together with the
XMLConstants.FEATURE_SECURE_PROCESSING feature.

Now we have a customer who claims that after upgrading to 7u45 he gets
an execption because of too many entity expansions. The customer
explicitly sets -DentityExpansionLimit=1.

For us it seems as if before change 8017298: Better XML support
there must have been places in the libraries which ignored the
entityExpansionLimit setting even if this was explicitly specified
by the user. Can somebody confirm this assumption or is our customer
facing another problem?

This might be useful:
  http://docs.oracle.com/javase/tutorial/jaxp/limits/index.html

-Alan.


Re: RFR: 8022213 Intermittent test failures in java/net/URLClassLoader (Add jdk/testlibrary/FileUtils.java)

2013-11-07 Thread Alan Bateman

On 07/11/2013 14:59, Chris Hegarty wrote:

Given both Michael and Alan's comments. I've update the webrev:
  http://cr.openjdk.java.net/~chegar/fileUtils.01/webrev/

1) more descriptive method names
2) deleteXXX methods return if interrupted, leaving the
   interrupt status set
3) Use Files.copy with REPLACE_EXISTING
4) Use SimpleFileVisitor, rather than FileVisitor

This looks better although interrupting the sleep means that the 
deleteXXX will quietly terminate with the interrupt status set (which 
could be awkward to diagnose if used with tests that are also using 
Thread.interrupt). An alternative might be to just throw the IOException 
with InterruptedException as the cause.


-Alan.




Simple 1-line fix for 8027943 to fix serial version of com.sun.corba.se.spi.orbutil.proxy.CompositeInvocationHandlerImp

2013-11-07 Thread Mandy Chung
This reverts 
com.sun.corba.se.spi.orbutil.proxy.CompositeInvocationHandlerImpl back 
to the previous serial version.


diff --git 
a/src/share/classes/com/sun/corba/se/spi/orbutil/proxy/CompositeInvocationHandlerImpl.java
 
b/src/share/classes/com/sun/corba/se/spi/orbutil/proxy/CompositeInvocationHandlerImpl.java
--- 
a/src/share/classes/com/sun/corba/se/spi/orbutil/proxy/CompositeInvocationHandlerImpl.java
+++ 
b/src/share/classes/com/sun/corba/se/spi/orbutil/proxy/CompositeInvocationHandlerImpl.java
@@ -89,4 +89,6 @@
 sm.checkPermission(perm);
 }
 }
+
+private static final long serialVersionUID = 4571178305984833743L;
 }

Webrev:
 http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8027943/webrev.00/

Mandy



Re: RFR: 7174936: several String methods claim to always create new String

2013-11-07 Thread Brent Christian

Hi, Stuart

On 11/6/13 5:31 PM, Stuart Marks wrote:


In several places the specs mention returning new strings. This is
overspecified; it's sufficient to return a string that satisfies the
required properties. In some cases the current implementation doesn't
create a new string -- for example, substring(0) returns 'this' -- which
is strictly a violation of the spec. The solution is to relax the spec
requirement to create new strings.


I like it.

I'd like to suggest a doc tweak.

For concat() we have:

--- 1997,2008 
   * If the length of the argument string is {@code 0}, then this
!  * {@code String} object is returned. Otherwise, a
!  * {@code String} object is returned, representing a character
   * sequence that is the concatenation of the character sequence

With new removed, Otherwise, a String object is returned, 
representing... to me somewhat implies that in the other case, 
something other than a String object is returned.


I prefer the way replace() is worded:

--- 2025,2041 
   * then a reference to this {@code String} object is returned.
!  * Otherwise, a {@code String} object is returned that
   * represents a character sequence identical to the character

I would change concat() to be similar:
Otherwise, a String object is returned that represents a character 
sequence that is the concatenation of...


Thanks,
-Brent


Re: Simple 1-line fix for 8027943 to fix serial version of com.sun.corba.se.spi.orbutil.proxy.CompositeInvocationHandlerImp

2013-11-07 Thread Lance Andersen - Oracle
+1
On Nov 7, 2013, at 4:59 PM, Mandy Chung wrote:

 This reverts 
 com.sun.corba.se.spi.orbutil.proxy.CompositeInvocationHandlerImpl back to the 
 previous serial version.
 
 diff --git 
 a/src/share/classes/com/sun/corba/se/spi/orbutil/proxy/CompositeInvocationHandlerImpl.java
  
 b/src/share/classes/com/sun/corba/se/spi/orbutil/proxy/CompositeInvocationHandlerImpl.java
 --- 
 a/src/share/classes/com/sun/corba/se/spi/orbutil/proxy/CompositeInvocationHandlerImpl.java
 +++ 
 b/src/share/classes/com/sun/corba/se/spi/orbutil/proxy/CompositeInvocationHandlerImpl.java
 @@ -89,4 +89,6 @@
 sm.checkPermission(perm);
 }
 }
 +
 +private static final long serialVersionUID = 4571178305984833743L;
 }
 
 Webrev:
 http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8027943/webrev.00/
 
 Mandy
 


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



Re: RFR doc only typo in java.io.DataInput

2013-11-07 Thread Lance Andersen - Oracle
+1
On Nov 7, 2013, at 5:29 PM, roger riggs wrote:

 Please review this straightforward typo correction:
 
 Webrev:
   http://cr.openjdk.java.net/~rriggs/webrev-doc-readlong-8024458/
 
 Thanks, Roger
 


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



RFR doc only typo in java.io.DataInput

2013-11-07 Thread roger riggs

Please review this straightforward typo correction:

Webrev:
   http://cr.openjdk.java.net/~rriggs/webrev-doc-readlong-8024458/

Thanks, Roger



Re: RFR doc only typo in java.io.DataInput

2013-11-07 Thread Chris Hegarty
Looks fine to me Roger.

-Chris

 On 7 Nov 2013, at 22:29, roger riggs roger.ri...@oracle.com wrote:
 
 Please review this straightforward typo correction:
 
 Webrev:
   http://cr.openjdk.java.net/~rriggs/webrev-doc-readlong-8024458/
 
 Thanks, Roger
 


Re: RFR: 7174936: several String methods claim to always create new String

2013-11-07 Thread Stuart Marks

On 11/7/13 12:12 AM, Stuart Marks wrote:

The concat() method is weird in that if the arg is empty, it requires returning
'this'; but if 'this' is empty, it requires creating a new String instead of
just returning the arg. I think this is overspecified. But the implementation
follows this exactly, so I didn't want to change the spec in this case. Maybe
later.


Woops, I forgot, I did change the concat() spec to no longer require creating a 
new string in the case where 'this' is empty. I think it's still overspecified, 
in that it requires returning 'this' if the arg is empty, though this doesn't 
seem to do much harm.


s'marks


Re: RFR: 7174936: several String methods claim to always create new String

2013-11-07 Thread Stuart Marks

On 11/7/13 2:13 PM, Brent Christian wrote:

I would change concat() to be similar:
Otherwise, a String object is returned that represents a character sequence
that is the concatenation of...


Yes, that's nice. Thanks for the suggestion. I'll put it in.

s'marks


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

2013-11-07 Thread David Holmes

On 7/11/2013 11:53 PM, Rob McKenna wrote:

Ah, thanks for catching that David, I've updated the webrev in place.

I believe the reasoning is that we want to minimise any potential impact
to customers running in production. The general feeling seems to be that
Mac is a development platform and is less likely to cause us problems in
that regard. (not that I would anticipate any on Solaris, but I
understand the desire to be conservative)


And linux? It has changed to vfork right?

So OSX has changed, linux has changed, but Solaris remains as-is. All 
platforms allow selecting the mechanism via the property 
jdk.lang.Process.launchMechanism


Thanks,
David


 -Rob

On 07/11/13 01:51, David Holmes wrote:

On 6/11/2013 10:00 PM, Rob McKenna wrote:

Hi David,

The only difference in 5049299 is the change in the default property
value in Solaris. Apologies for not making that clear.


Given this was primarily aimed at Solaris in the first place it seems
strange to me to not make the change on Solaris. If this is considered
risky fr an update release then I would think that applies to all
platforms equally. ??

If Solaris is not in fact changing then the comment in
src/solaris/native/java/lang/UNIXProcess_md.c needs to be altered:

   * Based on the above analysis, we are currently using vfork() on
!  * Linux and spawn() on other Unix systems, but the code to use clone()
!  * and fork() remains.

David
-


 -Rob

On 06/11/13 01:09, David Holmes wrote:

Hi Rob,

How different is this to the JDK 8 version?

Thanks,
David

On 6/11/2013 7:24 AM, Rob McKenna wrote:

..

http://cr.openjdk.java.net/~robm/5049299/7/webrev.01/

 -Rob

On 05/11/13 21:23, Rob McKenna wrote:

Hi folks,

I'd like to backport this change to JDK7. Note: this fix uses
posix_spawn by default on Mac OSX only. On Solaris fork will remain
the default behaviour.

I've just noticed that there is a problem with the testcase on
Solaris
in that it will test fork twice. (and won't test posix_spawn) I'll
figure out a way around this, but in the mean time I'd like to get
the
ball rolling on this review anyway.

-Rob










Re: RFR doc only typo in java.io.DataInput

2013-11-07 Thread Dan Xu

Hi Roger,

The change looks good.

-Dan

On 11/07/2013 02:29 PM, roger riggs wrote:

Please review this straightforward typo correction:

Webrev:
   http://cr.openjdk.java.net/~rriggs/webrev-doc-readlong-8024458/

Thanks, Roger





Re: RFR: 8022213 Intermittent test failures in java/net/URLClassLoader (Add jdk/testlibrary/FileUtils.java)

2013-11-07 Thread Dan Xu


On 11/07/2013 11:04 AM, Alan Bateman wrote:

On 07/11/2013 14:59, Chris Hegarty wrote:

Given both Michael and Alan's comments. I've update the webrev:
  http://cr.openjdk.java.net/~chegar/fileUtils.01/webrev/

1) more descriptive method names
2) deleteXXX methods return if interrupted, leaving the
   interrupt status set
3) Use Files.copy with REPLACE_EXISTING
4) Use SimpleFileVisitor, rather than FileVisitor

This looks better although interrupting the sleep means that the 
deleteXXX will quietly terminate with the interrupt status set (which 
could be awkward to diagnose if used with tests that are also using 
Thread.interrupt). An alternative might be to just throw the 
IOException with InterruptedException as the cause.


-Alan.



Hi Chris,

In the method, deleteFileWithRetry0(), it assumes that if any other 
process is accessing the same file, the delete operation, 
Files.delete(), will throw out IOException on Windows. But I don't see 
this assumption is always true when I investigated this issue on 
intermittent test failures.


When Files.delete() method is called, it finally calls DeleteFile or 
RemoveDirectory functions based on whether the target is a file or 
directory. And these Windows APIs only mark the target for deletion on 
close and return immediately without waiting the operation to be 
completed. If another process is accessing the file in the meantime, the 
delete operation does not occur and the target file stays at 
delete-pending status until that open handle is closed. It basically 
implies that DeleteFile and RemoveDirectory is like an async operation. 
Therefore, we cannot assume that the file/directory is deleted after 
Files.delete() returns or File.delete() returns true.


When checking those intermittently test failures, I find the test 
normally succeeds on the Files.delete() call. But due to the 
interference of Anti-virus or other Windows daemon services, the target 
file changes to delete-pending status. And the immediately following 
operation fails due the target file still exists, but our tests assume 
the target file is already gone. Because the delete-pending status of a 
file usually last for a very short time which depends on the 
interference source, such failures normally happens when we recursively 
delete a folder or delete-and-create a file with the same file name at a 
high frequency.


It is basically a Windows API design or implementation issue. I have 
logged an enhancement, JDK-8024496, to solve it from Java library layer. 
Currently, I have two strategies in mind. One is to make the delete 
operation blocking, which means to make sure the file/directory is 
deleted before the return. The other is to make sure the delete-pending 
file does not lead to a failure of subsequent file operations. But they 
both has pros and cons.


Thank!

-Dan


RFR: 8028027: serialver should emit declaration with the 'private' modifier

2013-11-07 Thread Stuart Marks

Hi all,

Please review this quick one-liner to change the serialver tool so that it emits 
a serialVersionUID declaration with the 'private' modifier, which comports with 
the recommendation in the java.io.Serializable page.


Bug:

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

Patch appended below.

Thanks,

s'marks


--


# HG changeset patch
# User smarks
# Date 1383868023 28800
# Node ID 1e1088bfea50d7d3cc7cfdce2b0085b7adc6a180
# Parent  f18b60bd22a1be988d329960c46d504f4b75000f
8028027: serialver should emit declaration with the 'private' modifier
Reviewed-by: XXX

diff -r f18b60bd22a1 -r 1e1088bfea50 
src/share/classes/sun/tools/serialver/SerialVer.java
--- a/src/share/classes/sun/tools/serialver/SerialVer.java	Thu Nov 07 15:45:21 
2013 -0800
+++ b/src/share/classes/sun/tools/serialver/SerialVer.java	Thu Nov 07 15:47:03 
2013 -0800

@@ -211,7 +211,7 @@
 Class? cl = Class.forName(classname, false, loader);
 ObjectStreamClass desc = ObjectStreamClass.lookup(cl);
 if (desc != null) {
-return static final long serialVersionUID =  +
+return private static final long serialVersionUID =  +
 desc.getSerialVersionUID() + L;;
 } else {
 return null;


Re: RFR: 8028027: serialver should emit declaration with the 'private' modifier

2013-11-07 Thread Joseph Darcy

Approved!!

-Joe

On 11/7/2013 7:02 PM, Stuart Marks wrote:

Hi all,

Please review this quick one-liner to change the serialver tool so 
that it emits a serialVersionUID declaration with the 'private' 
modifier, which comports with the recommendation in the 
java.io.Serializable page.


Bug:

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

Patch appended below.

Thanks,

s'marks


--


# HG changeset patch
# User smarks
# Date 1383868023 28800
# Node ID 1e1088bfea50d7d3cc7cfdce2b0085b7adc6a180
# Parent  f18b60bd22a1be988d329960c46d504f4b75000f
8028027: serialver should emit declaration with the 'private' modifier
Reviewed-by: XXX

diff -r f18b60bd22a1 -r 1e1088bfea50 
src/share/classes/sun/tools/serialver/SerialVer.java
--- a/src/share/classes/sun/tools/serialver/SerialVer.javaThu Nov 
07 15:45:21 2013 -0800
+++ b/src/share/classes/sun/tools/serialver/SerialVer.javaThu Nov 
07 15:47:03 2013 -0800

@@ -211,7 +211,7 @@
 Class? cl = Class.forName(classname, false, loader);
 ObjectStreamClass desc = ObjectStreamClass.lookup(cl);
 if (desc != null) {
-return static final long serialVersionUID =  +
+return private static final long serialVersionUID =  +
 desc.getSerialVersionUID() + L;;
 } else {
 return null;




Re: RFR: 8028027: serialver should emit declaration with the 'private' modifier

2013-11-07 Thread Mandy Chung

Looks good.
Mandy

On 11/7/2013 7:02 PM, Stuart Marks wrote:

Hi all,

Please review this quick one-liner to change the serialver tool so 
that it emits a serialVersionUID declaration with the 'private' 
modifier, which comports with the recommendation in the 
java.io.Serializable page.


Bug:

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

Patch appended below.

Thanks,

s'marks


--


# HG changeset patch
# User smarks
# Date 1383868023 28800
# Node ID 1e1088bfea50d7d3cc7cfdce2b0085b7adc6a180
# Parent  f18b60bd22a1be988d329960c46d504f4b75000f
8028027: serialver should emit declaration with the 'private' modifier
Reviewed-by: XXX

diff -r f18b60bd22a1 -r 1e1088bfea50 
src/share/classes/sun/tools/serialver/SerialVer.java
--- a/src/share/classes/sun/tools/serialver/SerialVer.javaThu Nov 
07 15:45:21 2013 -0800
+++ b/src/share/classes/sun/tools/serialver/SerialVer.javaThu Nov 
07 15:47:03 2013 -0800

@@ -211,7 +211,7 @@
 Class? cl = Class.forName(classname, false, loader);
 ObjectStreamClass desc = ObjectStreamClass.lookup(cl);
 if (desc != null) {
-return static final long serialVersionUID =  +
+return private static final long serialVersionUID =  +
 desc.getSerialVersionUID() + L;;
 } else {
 return null;




Re: RFR for JDK-8019502 some java.util test does not pass VM options when launching java program in script

2013-11-07 Thread Patrick Zhang

Sorry, I have some problems to connect to cr.openjdk.java.net yesterday.
Now the webrev  and result are attached. Please help to review.
After checking the scripts in java.util, most of scripts have been 
finished in 8003890. So the webrev only change 2 rest files.


webrev:
http://cr.openjdk.java.net/~pzhang/8019502/webrev/

result:
http://cr.openjdk.java.net/~pzhang/8019502/GenericTimeZoneNamesTest.jtr
http://cr.openjdk.java.net/~pzhang/8019502/NarrowNamesTest.jtr

Regards
Patrick

On 11/7/13 5:07 PM, Alan Bateman wrote:

On 07/11/2013 03:48, Patrick Zhang wrote:

Hi Everyone,

I am working on https://bugs.openjdk.java.net/browse/JDK-8019502. The 
problem is, some java.util test does use ${TESTVMOPTS} to launch java 
program in script. Then it may lose VM setting from high level.


The fix will refer to 
https://bugs.openjdk.java.net/browse/JDK-8003890 finished by Mark 
Sheppard. Just add ${TESTVMOPTS} to launch java.

Did you intend to include a link to a patch?

-Alan.


hg: jdk8/tl/langtools: 8027730: Fix release-8 type visitors to support intersection types

2013-11-07 Thread joe . darcy
Changeset: e39bd9401ea5
Author:darcy
Date:  2013-11-07 20:11 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/e39bd9401ea5

8027730: Fix release-8 type visitors to support intersection types
Reviewed-by: jjg, jlahoda, sogoel

! src/share/classes/javax/lang/model/util/SimpleTypeVisitor8.java
! src/share/classes/javax/lang/model/util/TypeKindVisitor8.java
+ test/tools/javac/processing/model/util/TestIntersectionTypeVisitors.java



Re: RFR for JDK-8019502 some java.util test does not pass VM options when launching java program in script

2013-11-07 Thread David Holmes

On 8/11/2013 1:41 PM, Patrick Zhang wrote:

Sorry, I have some problems to connect to cr.openjdk.java.net yesterday.
Now the webrev  and result are attached. Please help to review.
After checking the scripts in java.util, most of scripts have been
finished in 8003890. So the webrev only change 2 rest files.

webrev:
http://cr.openjdk.java.net/~pzhang/8019502/webrev/


Looks fine to me.

Thanks,
David


result:
http://cr.openjdk.java.net/~pzhang/8019502/GenericTimeZoneNamesTest.jtr
http://cr.openjdk.java.net/~pzhang/8019502/NarrowNamesTest.jtr

Regards
Patrick

On 11/7/13 5:07 PM, Alan Bateman wrote:

On 07/11/2013 03:48, Patrick Zhang wrote:

Hi Everyone,

I am working on https://bugs.openjdk.java.net/browse/JDK-8019502. The
problem is, some java.util test does use ${TESTVMOPTS} to launch java
program in script. Then it may lose VM setting from high level.

The fix will refer to
https://bugs.openjdk.java.net/browse/JDK-8003890 finished by Mark
Sheppard. Just add ${TESTVMOPTS} to launch java.

Did you intend to include a link to a patch?

-Alan.


hg: jdk8/tl/corba: 8027943: serial version of com.sun.corba.se.spi.orbutil.proxy.CompositeInvocationHandlerImpl changed in 7u45

2013-11-07 Thread mandy . chung
Changeset: b99535e22efe
Author:mchung
Date:  2013-11-07 20:48 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/corba/rev/b99535e22efe

8027943: serial version of 
com.sun.corba.se.spi.orbutil.proxy.CompositeInvocationHandlerImpl changed in 
7u45
Reviewed-by: msheppar, alanb, lancea

! 
src/share/classes/com/sun/corba/se/spi/orbutil/proxy/CompositeInvocationHandlerImpl.java



hg: jdk8/tl/jdk: 8007984: Null pointer dereference in jdk/linux-amd64/democlasses/demo/jvmti/heapTracker/src/java_crw_demo.c

2013-11-07 Thread jaroslav . bachorik
Changeset: b5748857ef42
Author:jbachorik
Date:  2013-11-08 08:47 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b5748857ef42

8007984: Null pointer dereference in 
jdk/linux-amd64/democlasses/demo/jvmti/heapTracker/src/java_crw_demo.c
Reviewed-by: dholmes

! src/share/demo/jvmti/java_crw_demo/java_crw_demo.c