Re: review request for 7021582, use try-with-resources in jar/zip implementation and tests

[David Holmes]

Hi Stuart,

Just taking a look as a curious observer ...

I'm somewhat dismayed by the lack of exception handling in the original 
code.


Stuart Marks said the following on 02/24/11 16:26:


* src/share/classes/com/sun/java/util/jar/pack/Driver.java

I narrowed the scope of the open resource. No sense keeping it open any 
longer than necessary. This occurs in several other places as well.


You also changed from using a BufferedStream to just the FileInputStream 
- was that intentional?



* src/share/classes/com/sun/java/util/jar/pack/PropMap.java

Narrowed the scope of catch IOException; should be OK since the code 
that was migrated out cannot throw IOException.


So in this one:

 128 boolean propsLoaded = false;
 129 try (InputStream propStr = 
PackerImpl.class.getResourceAsStream(propFile)) {

 130 props.load(propStr);
 131 propsLoaded = true;
 132 } catch (IOException ee) {
 133 // ignore exception if it came from the close()
 134 if (!propsLoaded) {
 135 throw new RuntimeException(ee);
 136 }
 137 }

The RuntimeException has a cause of ee, which in turn may have a 
suppressed IOException from the close().



test/java/util/zip/Available.java

Don't you need to unroll the constructors here too:

  47 try (ZipInputStream z = new ZipInputStream(new 
FileInputStream(f))) {

  48 z.getNextEntry();
  49 tryAvail(z);
  50 }


test/java/util/zip/GZIP/GZIPInputStreamRead.java

Unroll here too:

 78 try (GZIPInputStream gzis = new GZIPInputStream(
 79 new ByteArrayInputStream(dst),
 80 gzisBufSize))
 81 {


Cheers,
David
-



* src/share/classes/com/sun/java/util/jar/pack/UnpackerImpl.java

This closes its input after successful processing. I changed this so 
that it also closes its input if an exception is thrown.


* test/java/util/zip/LargeZip.java

I've "unrolled" a cascade of constructors into separate resource 
variables. This also occurs in several other places. Basically code that 
used to look like this:


ZipOutputStream zos = new ZipOutputStream(
new BufferedOutputStream(
new FileOutputStream(largeFile)));
// process zos
zos.close();

is converted to this:

try (FileOutputStream fos = new FileOutputStream(largeFile);
 BufferedOutputStream bos = new BufferedOutputStream(fos);
 ZipOutputStream zos = new ZipOutputStream(bos))
{
// process zos
}

I think this more robust, since it closes the FileOutputStream if an 
exception occurs during the construction of one of the stacked streams, 
which the original code did not handle. Since the wrapper streams will 
close their underlying streams, this will result in redundant close() 
calls. However, close() is supposed to be idempotent so this should be OK.


* test/java/util/zip/ZipFile/DeleteTempJar.java

I'm not sure if this properly handles an IOException caused by 
HttpExchange.close(). Funny, the method isn't declared to throw IOE, but 
this test did compile and pass.


* test/java/util/zip/ZipFile/LargeZipFile.java

I changed this to fail the test if close() were to throw IOE. I think 
this is proper for test code.


* test/java/util/zip/ZipFile/ReadZip.java

I took the liberty of converting the file copying code to use the new 
java.nio.file.Files utilities. Well, I'm really following Alan's lead 
here since he's prompted me to do so in other places a couple times 
already. :-)


Thanks for reviewing!

s'marks

hg: jdk7/tl/jdk: 6844879: Source distribution should be more robustly built without the security source distribution

[bradford . wetmore]

Changeset: 0f0d6b8f98cc
Author:wetmore
Date:  2011-02-23 22:54 -0800
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/0f0d6b8f98cc

6844879: Source distribution should be more robustly built without the security 
source distribution
Reviewed-by: ohair

! make/common/shared/Defs-java.gmk

Re: review request for 7021582, use try-with-resources in jar/zip implementation and tests

[Xueming Shen]

 Kumar,

Would you please help review the change in your pack code?

Thanks,
-Sherman

On 2011-2-23 22:26, Stuart Marks wrote:

Hi Sherman, all,

Here's a webrev to convert code in the jar/zip implementation files 
and tests to use the new Java 7 try-with-resources construct.


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

There are rather a lot of files, however, most of the changes are 
pretty straightforward. A typical conversion changes code like this:


FileInputStream fis = new FileInputStream(filename);
// use fis
fis.close();

to this:

try (FileInputStream fis = new FileInputStream(filename)) {
// use fis
}

The majority of the conversions are like the above. However, there are 
a several places where I had to rearrange things either in order to 
get them to work at all, to improve robustness, or for general cleanup.


Some of these are marked with "TODO". I will of course remove these 
comments before committing the changes. Where you see these comments, 
you might want to give the code a bit more scrutiny.


Specific examples of things to look at follow:

* test/java/util/jar/JarEntry/GetMethodsReturnClones.java

I had to change the ordering of local variable declarations so that 
the variable was visible where it's used. TWR introduces a nested 
block, so obviously a local declared within will have to be moved 
outside in order to be used outside. This occurs in several other 
places as well. In some cases initialization order was changed. This 
shouldn't matter, though, since it's things like opening a file vs. 
allocating an array.


* src/share/classes/com/sun/java/util/jar/pack/Driver.java

I narrowed the scope of the open resource. No sense keeping it open 
any longer than necessary. This occurs in several other places as well.


* src/share/classes/com/sun/java/util/jar/pack/PackageReader.java
* src/share/classes/com/sun/java/util/jar/pack/PackageWriter.java

These changes rely on recent changes to TWR's handling of null 
resources. Currently, TWR will avoid calling close if the resource is 
null. Joe checked in this change just last week. Before that, a null 
resource would generate an unavoidable NPE when it attempted to call 
close(). Handling of non-null resources is unchanged.


I don't think the change to null handling is in a promoted build yet. 
Is it OK to check in code that depends on it? All tests pass, but that 
just means that the path where the resource is null isn't tested.


* src/share/classes/com/sun/java/util/jar/pack/PropMap.java

Narrowed the scope of catch IOException; should be OK since the code 
that was migrated out cannot throw IOException.


* src/share/classes/com/sun/java/util/jar/pack/UnpackerImpl.java

This closes its input after successful processing. I changed this so 
that it also closes its input if an exception is thrown.


* test/java/util/zip/LargeZip.java

I've "unrolled" a cascade of constructors into separate resource 
variables. This also occurs in several other places. Basically code 
that used to look like this:


ZipOutputStream zos = new ZipOutputStream(
new BufferedOutputStream(
new FileOutputStream(largeFile)));
// process zos
zos.close();

is converted to this:

try (FileOutputStream fos = new FileOutputStream(largeFile);
 BufferedOutputStream bos = new BufferedOutputStream(fos);
 ZipOutputStream zos = new ZipOutputStream(bos))
{
// process zos
}

I think this more robust, since it closes the FileOutputStream if an 
exception occurs during the construction of one of the stacked 
streams, which the original code did not handle. Since the wrapper 
streams will close their underlying streams, this will result in 
redundant close() calls. However, close() is supposed to be idempotent 
so this should be OK.


* test/java/util/zip/ZipFile/DeleteTempJar.java

I'm not sure if this properly handles an IOException caused by 
HttpExchange.close(). Funny, the method isn't declared to throw IOE, 
but this test did compile and pass.


* test/java/util/zip/ZipFile/LargeZipFile.java

I changed this to fail the test if close() were to throw IOE. I think 
this is proper for test code.


* test/java/util/zip/ZipFile/ReadZip.java

I took the liberty of converting the file copying code to use the new 
java.nio.file.Files utilities. Well, I'm really following Alan's lead 
here since he's prompted me to do so in other places a couple times 
already. :-)


Thanks for reviewing!

s'marks

review request for 7021582, use try-with-resources in jar/zip implementation and tests

[Stuart Marks]

Hi Sherman, all,

Here's a webrev to convert code in the jar/zip implementation files and tests 
to use the new Java 7 try-with-resources construct.


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

There are rather a lot of files, however, most of the changes are pretty 
straightforward. A typical conversion changes code like this:


FileInputStream fis = new FileInputStream(filename);
// use fis
fis.close();

to this:

try (FileInputStream fis = new FileInputStream(filename)) {
// use fis
}

The majority of the conversions are like the above. However, there are a 
several places where I had to rearrange things either in order to get them to 
work at all, to improve robustness, or for general cleanup.


Some of these are marked with "TODO". I will of course remove these comments 
before committing the changes. Where you see these comments, you might want to 
give the code a bit more scrutiny.


Specific examples of things to look at follow:

* test/java/util/jar/JarEntry/GetMethodsReturnClones.java

I had to change the ordering of local variable declarations so that the 
variable was visible where it's used. TWR introduces a nested block, so 
obviously a local declared within will have to be moved outside in order to be 
used outside. This occurs in several other places as well. In some cases 
initialization order was changed. This shouldn't matter, though, since it's 
things like opening a file vs. allocating an array.


* src/share/classes/com/sun/java/util/jar/pack/Driver.java

I narrowed the scope of the open resource. No sense keeping it open any longer 
than necessary. This occurs in several other places as well.


* src/share/classes/com/sun/java/util/jar/pack/PackageReader.java
* src/share/classes/com/sun/java/util/jar/pack/PackageWriter.java

These changes rely on recent changes to TWR's handling of null resources. 
Currently, TWR will avoid calling close if the resource is null. Joe checked in 
this change just last week. Before that, a null resource would generate an 
unavoidable NPE when it attempted to call close(). Handling of non-null 
resources is unchanged.


I don't think the change to null handling is in a promoted build yet. Is it OK 
to check in code that depends on it? All tests pass, but that just means that 
the path where the resource is null isn't tested.


* src/share/classes/com/sun/java/util/jar/pack/PropMap.java

Narrowed the scope of catch IOException; should be OK since the code that was 
migrated out cannot throw IOException.


* src/share/classes/com/sun/java/util/jar/pack/UnpackerImpl.java

This closes its input after successful processing. I changed this so that it 
also closes its input if an exception is thrown.


* test/java/util/zip/LargeZip.java

I've "unrolled" a cascade of constructors into separate resource variables. 
This also occurs in several other places. Basically code that used to look like 
this:


ZipOutputStream zos = new ZipOutputStream(
new BufferedOutputStream(
new FileOutputStream(largeFile)));
// process zos
zos.close();

is converted to this:

try (FileOutputStream fos = new FileOutputStream(largeFile);
 BufferedOutputStream bos = new BufferedOutputStream(fos);
 ZipOutputStream zos = new ZipOutputStream(bos))
{
// process zos
}

I think this more robust, since it closes the FileOutputStream if an exception 
occurs during the construction of one of the stacked streams, which the 
original code did not handle. Since the wrapper streams will close their 
underlying streams, this will result in redundant close() calls. However, 
close() is supposed to be idempotent so this should be OK.


* test/java/util/zip/ZipFile/DeleteTempJar.java

I'm not sure if this properly handles an IOException caused by 
HttpExchange.close(). Funny, the method isn't declared to throw IOE, but this 
test did compile and pass.


* test/java/util/zip/ZipFile/LargeZipFile.java

I changed this to fail the test if close() were to throw IOE. I think this is 
proper for test code.


* test/java/util/zip/ZipFile/ReadZip.java

I took the liberty of converting the file copying code to use the new 
java.nio.file.Files utilities. Well, I'm really following Alan's lead here 
since he's prompted me to do so in other places a couple times already. :-)


Thanks for reviewing!

s'marks

Re: Review request for 7020513 : Add com.sun.xml.internal to the "package.access" property in java.security

[Alan Bateman]

Rama Pulavarthi wrote:

:

Just porting the fix along with tests from Open JDK 6 workspace, 
that's why I kept the old date. Does it need to be changed? This test 
was added then following the convention of other tests. I will check 
other tests in JDK 7 to see if it needs any update.


Sorry Rama, it wasn't obvious that this was a forward-port, in which 
case ignore my comment on the year.


On the test, we regularly have issues with tests that are scripts. From 
a brief glance it doesn't appear to be needed and instead just requires 
changing the @run tag to @run/othervm (might want to give it a better 
name too as "Test" doesn't convey much).


-Alan.

hg: jdk7/tl/jdk: 40 new changesets

[lana . steuck]

Changeset: bad0ddc6f573
Author:prr
Date:  2011-01-26 11:46 -0800
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/bad0ddc6f573

7014738: Update jdk repo application manifests with Windows 7 compatibility 
section.
Reviewed-by: bae, igor

! src/windows/resource/java.manifest

Changeset: d0e158473b6f
Author:prr
Date:  2011-01-26 13:26 -0800
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/d0e158473b6f

6940890: Java doesn't pick up the correct fontconfig files in latest Solaris 
Next builds
Reviewed-by: bae, igor

! src/share/classes/sun/java2d/SunGraphicsEnvironment.java

Changeset: 4cf20706dbfa
Author:dlila
Date:  2011-01-27 16:43 -0500
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/4cf20706dbfa

4645692: solveCubic does not return all solutions.
Summary: more robust solveCubic implementation.
Reviewed-by: flar

! src/share/classes/java/awt/geom/CubicCurve2D.java
+ test/java/awt/geom/CubicCurve2D/ContainsTest.java
+ test/java/awt/geom/CubicCurve2D/IntersectsTest.java
+ test/java/awt/geom/CubicCurve2D/SolveCubicTest.java

Changeset: 21621a756b32
Author:lana
Date:  2011-02-03 19:15 -0800
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/21621a756b32

Merge

- make/java/hpi/Makefile
- make/java/hpi/hpi_common.gmk
- make/java/hpi/native/Makefile
- make/java/hpi/native/mapfile-vers
- make/java/hpi/native/reorder-i586
- make/java/hpi/native/reorder-sparc
- make/java/hpi/native/reorder-sparcv9
- make/java/hpi/windows/Makefile
- src/share/hpi/export/bool.h
- src/share/hpi/export/dll.h
- src/share/hpi/export/hpi.h
- src/share/hpi/include/hpi_impl.h
- src/share/hpi/include/vm_calls.h
- src/share/hpi/src/hpi.c
- src/solaris/hpi/export/byteorder_md.h
- src/solaris/hpi/export/hpi_md.h
- src/solaris/hpi/export/io_md.h
- src/solaris/hpi/export/path_md.h
- src/solaris/hpi/export/timeval_md.h
- src/solaris/hpi/include/hpi_init.h
- src/solaris/hpi/include/interrupt.h
- src/solaris/hpi/include/largefile.h
- src/solaris/hpi/include/largefile_linux.h
- src/solaris/hpi/include/largefile_solaris.h
- src/solaris/hpi/native_threads/include/condvar_md.h
- src/solaris/hpi/native_threads/include/monitor_md.h
- src/solaris/hpi/native_threads/include/mutex_md.h
- src/solaris/hpi/native_threads/include/np.h
- src/solaris/hpi/native_threads/include/porting.h
- src/solaris/hpi/native_threads/include/threads_md.h
- src/solaris/hpi/native_threads/src/condvar_md.c
- src/solaris/hpi/native_threads/src/interrupt_md.c
- src/solaris/hpi/native_threads/src/monitor_md.c
- src/solaris/hpi/native_threads/src/mutex_md.c
- src/solaris/hpi/native_threads/src/sys_api_td.c
- src/solaris/hpi/native_threads/src/threads_linux.c
- src/solaris/hpi/native_threads/src/threads_md.c
- src/solaris/hpi/native_threads/src/threads_solaris.c
- src/solaris/hpi/src/interrupt.c
- src/solaris/hpi/src/linker_md.c
- src/solaris/hpi/src/memory_md.c
- src/solaris/hpi/src/system_md.c
- src/windows/hpi/export/byteorder_md.h
- src/windows/hpi/export/hpi_md.h
- src/windows/hpi/export/io_md.h
- src/windows/hpi/export/path_md.h
- src/windows/hpi/export/timeval_md.h
- src/windows/hpi/include/monitor_md.h
- src/windows/hpi/include/mutex_md.h
- src/windows/hpi/include/threads_md.h
- src/windows/hpi/src/linker_md.c
- src/windows/hpi/src/memory_md.c
- src/windows/hpi/src/monitor_md.c
- src/windows/hpi/src/path_md.c
- src/windows/hpi/src/socket_md.c
- src/windows/hpi/src/sys_api_md.c
- src/windows/hpi/src/system_md.c
- src/windows/hpi/src/threads_md.c
- test/java/net/InetAddress/B4762344.java
- 
test/java/net/InetAddress/META-INF/services/sun.net.spi.nameservice.NameServiceDescriptor
- test/java/net/InetAddress/Simple1NameServiceDescriptor.java
- test/java/net/InetAddress/Simple2NameServiceDescriptor.java
- test/java/net/InetAddress/SimpleNameService.java
- test/sun/net/InetAddress/nameservice/B6442088.java
- test/sun/net/InetAddress/nameservice/CacheTest.java
- 
test/sun/net/InetAddress/nameservice/META-INF/services/sun.net.spi.nameservice.NameServiceDescriptor
- test/sun/net/InetAddress/nameservice/SimpleNameService.java
- test/sun/net/InetAddress/nameservice/SimpleNameServiceDescriptor.java

Changeset: 5e624003e622
Author:dlila
Date:  2011-02-08 09:22 -0500
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/5e624003e622

7016856: dashing performance was reduced during latest changes to the OpenJDK 
rasterizer
Summary: Optimized dashing, rasterizing, and the flow of transformed coordinates
Reviewed-by: flar

! src/share/classes/sun/java2d/pisces/Curve.java
! src/share/classes/sun/java2d/pisces/Dasher.java
! src/share/classes/sun/java2d/pisces/Helpers.java
! src/share/classes/sun/java2d/pisces/PiscesCache.java
! src/share/classes/sun/java2d/pisces/PiscesRenderingEngine.java
! src/share/classes/sun/java2d/pisces/PiscesTileGenerator.java
! src/share/classes/sun/java2d/pisces/Renderer.java
! src/share/classes/sun/java2d/pisces/Stroker.java
! src/share/classes/sun/java2d/pisces/TransformingPathConsumer2D.java

Changeset: 577c5d

hg: jdk7/tl/hotspot: 26 new changesets

[lana . steuck]

Changeset: b7a938236e43
Author:tonyp
Date:  2011-01-31 16:28 -0500
URL:   http://hg.openjdk.java.net/jdk7/tl/hotspot/rev/b7a938236e43

7014679: G1: deadlock during concurrent cleanup
Summary: There's a potential deadlock between the concurrent cleanup thread and 
the GC workers that are trying to allocate and waiting for more free regions to 
be made available.
Reviewed-by: iveresov, jcoomes

! src/share/vm/gc_implementation/g1/concurrentMarkThread.cpp

Changeset: e49cfa28f585
Author:ysr
Date:  2011-02-01 10:02 -0800
URL:   http://hg.openjdk.java.net/jdk7/tl/hotspot/rev/e49cfa28f585

688: CMS: Increased fragmentation leading to promotion failure after 
CR#6631166 got implemented
Summary: Fix calculation of _desired, in free list statistics, which was 
missing an intended set of parentheses.
Reviewed-by: poonam, jmasa

! src/share/vm/gc_implementation/shared/allocationStats.hpp

Changeset: 986b2844f7a2
Author:brutisso
Date:  2011-02-01 14:05 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/hotspot/rev/986b2844f7a2

6789220: CMS: intermittent timeout running nsk/regression/b4796926
Summary: The reference handler java thread and the GC could dead lock
Reviewed-by: never, johnc, jcoomes

! src/share/vm/compiler/compileBroker.cpp

Changeset: c33825b68624
Author:johnc
Date:  2011-02-02 10:41 -0800
URL:   http://hg.openjdk.java.net/jdk7/tl/hotspot/rev/c33825b68624

6923430: G1: assert(res != 0,"This should have worked.")
7007446: G1: expand the heap with a single step, not one region at a time
Summary: Changed G1CollectedHeap::expand() to expand the committed space by 
calling VirtualSpace::expand_by() once rather than for every region in the 
expansion amount. This allows the success or failure of the expansion to be 
determined before creating any heap regions. Introduced a develop flag 
G1ExitOnExpansionFailure (false by default) that, when true, will exit the VM 
if the expansion of the committed space fails. Finally 
G1CollectedHeap::expand() returns a status back to it's caller so that the 
caller knows whether to attempt the allocation.
Reviewed-by: brutisso, tonyp

! src/share/vm/gc_implementation/g1/concurrentG1Refine.cpp
! src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
! src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp
! src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp
! src/share/vm/gc_implementation/g1/g1RemSet.cpp
! src/share/vm/gc_implementation/g1/g1_globals.hpp

Changeset: 176d0be30214
Author:phh
Date:  2011-02-03 16:06 -0500
URL:   http://hg.openjdk.java.net/jdk7/tl/hotspot/rev/176d0be30214

7016998: gcutil class LinearLeastSquareFit doesn't initialize some of its fields
Summary: Initialize _sum_x_squared, _intercept and _slope in constructor.
Reviewed-by: bobv, coleenp

! src/share/vm/gc_implementation/shared/gcUtil.cpp

Changeset: c6bf3ca2bb31
Author:trims
Date:  2011-02-04 16:29 -0800
URL:   http://hg.openjdk.java.net/jdk7/tl/hotspot/rev/c6bf3ca2bb31

Merge


Changeset: d70fe6ab4436
Author:coleenp
Date:  2011-02-01 11:23 -0500
URL:   http://hg.openjdk.java.net/jdk7/tl/hotspot/rev/d70fe6ab4436

6588413: Use -fvisibility=hidden for gcc compiles
Summary: Add option for gcc 4 and above, define JNIEXPORT and JNIIMPORT to 
visibility=default, add for jio_snprintf and others since -fvisibility=hidden 
overrides --version-script definitions.
Reviewed-by: kamg, never

! make/linux/makefiles/gcc.make
! make/linux/makefiles/mapfile-vers-debug
! make/linux/makefiles/mapfile-vers-product
! src/cpu/sparc/vm/jni_sparc.h
! src/cpu/x86/vm/jni_x86.h
! src/cpu/zero/vm/jni_zero.h
! src/os/linux/vm/jvm_linux.cpp
! src/os/linux/vm/os_linux.cpp
! src/os/solaris/vm/os_solaris.cpp
! src/os_cpu/linux_sparc/vm/os_linux_sparc.cpp
! src/os_cpu/linux_x86/vm/os_linux_x86.cpp
! src/os_cpu/linux_zero/vm/os_linux_zero.cpp
! src/os_cpu/solaris_sparc/vm/os_solaris_sparc.cpp
! src/os_cpu/solaris_x86/vm/os_solaris_x86.cpp
! src/share/vm/prims/forte.cpp
! src/share/vm/prims/jvm.cpp
! src/share/vm/prims/jvm.h

Changeset: b92c45f2bc75
Author:bobv
Date:  2011-02-02 11:35 -0500
URL:   http://hg.openjdk.java.net/jdk7/tl/hotspot/rev/b92c45f2bc75

7016023: Enable building ARM and PPC from src/closed repository
Reviewed-by: dholmes, bdelsart

! make/Makefile
+ make/closed.make
! make/jprt.properties
! make/linux/Makefile
! make/linux/makefiles/adlc.make
+ make/linux/makefiles/arm.make
! make/linux/makefiles/buildtree.make
+ make/linux/makefiles/ppc.make
! make/linux/makefiles/rules.make
! make/linux/makefiles/top.make
! make/linux/makefiles/vm.make
+ make/linux/platform_arm
+ make/linux/platform_ppc
! src/os/linux/vm/osThread_linux.cpp
! src/os/linux/vm/os_linux.cpp
! src/os/linux/vm/os_linux.inline.hpp
! src/os/linux/vm/thread_linux.inline.hpp
! src/share/vm/asm/assembler.cpp
! src/share/vm/asm/assembler.hpp
! src/share/vm/asm/codeBuffer.hpp
! src/share/vm/c1/c1_Defs.hpp
! src/share/vm/c1/c1_FpuStackSim.hpp
! src/share/vm/

hg: jdk7/tl/jaxp: Added tag jdk7-b130 for changeset ab107c1bc4b9

[lana . steuck]

Changeset: f2ad604323c0
Author:cl
Date:  2011-02-18 14:23 -0800
URL:   http://hg.openjdk.java.net/jdk7/tl/jaxp/rev/f2ad604323c0

Added tag jdk7-b130 for changeset ab107c1bc4b9

! .hgtags

hg: jdk7/tl: 9 new changesets

[lana . steuck]

Changeset: 995077c73fbb
Author:cl
Date:  2011-02-18 14:23 -0800
URL:   http://hg.openjdk.java.net/jdk7/tl/rev/995077c73fbb

Added tag jdk7-b130 for changeset cc58c11af154

! .hgtags

Changeset: 0fd0aeb592cb
Author:jqzuo
Date:  2010-12-09 10:58 -0500
URL:   http://hg.openjdk.java.net/jdk7/tl/rev/0fd0aeb592cb

Merge


Changeset: 20955959b7b7
Author:jqzuo
Date:  2010-12-22 15:55 -0500
URL:   http://hg.openjdk.java.net/jdk7/tl/rev/20955959b7b7

Merge


Changeset: 08fe18caf411
Author:jqzuo
Date:  2011-01-10 13:45 -0500
URL:   http://hg.openjdk.java.net/jdk7/tl/rev/08fe18caf411

Merge


Changeset: aee1b0183364
Author:jqzuo
Date:  2011-01-24 17:14 -0500
URL:   http://hg.openjdk.java.net/jdk7/tl/rev/aee1b0183364

Merge


Changeset: 12764a5a3aec
Author:jqzuo
Date:  2011-02-01 15:03 -0500
URL:   http://hg.openjdk.java.net/jdk7/tl/rev/12764a5a3aec

Merge


Changeset: df3abd560cbd
Author:jqzuo
Date:  2011-02-09 16:05 -0500
URL:   http://hg.openjdk.java.net/jdk7/tl/rev/df3abd560cbd

Merge


Changeset: e2370dfcc721
Author:paulk
Date:  2011-02-14 14:29 -0500
URL:   http://hg.openjdk.java.net/jdk7/tl/rev/e2370dfcc721

7019371: JDK7 is not building UPX. IFTW wrappers are not compressed.
Reviewed-by: billyh, jqzuo

! make/deploy-rules.gmk

Changeset: 5466f13d19be
Author:jqzuo
Date:  2011-02-21 14:18 -0500
URL:   http://hg.openjdk.java.net/jdk7/tl/rev/5466f13d19be

Merge

hg: jdk7/tl/corba: 5 new changesets

[lana . steuck]

Changeset: 30ecf5c90a30
Author:mfang
Date:  2011-02-10 11:07 -0800
URL:   http://hg.openjdk.java.net/jdk7/tl/corba/rev/30ecf5c90a30

7014477: pt_BR corba resource bundle is missing in jdk7 build
Reviewed-by: ohair

! make/common/Defs.gmk

Changeset: c08dff674e53
Author:mfang
Date:  2011-02-10 14:25 -0800
URL:   http://hg.openjdk.java.net/jdk7/tl/corba/rev/c08dff674e53

7017734: jdk7 message drop 1 translation integration
Reviewed-by: ogino, yhuang

! src/share/classes/com/sun/corba/se/impl/orbutil/resources/sunorb_de.properties
! src/share/classes/com/sun/corba/se/impl/orbutil/resources/sunorb_es.properties
! src/share/classes/com/sun/corba/se/impl/orbutil/resources/sunorb_fr.properties
! src/share/classes/com/sun/corba/se/impl/orbutil/resources/sunorb_it.properties
! src/share/classes/com/sun/corba/se/impl/orbutil/resources/sunorb_ja.properties
! src/share/classes/com/sun/corba/se/impl/orbutil/resources/sunorb_ko.properties
! 
src/share/classes/com/sun/corba/se/impl/orbutil/resources/sunorb_pt_BR.properties
! src/share/classes/com/sun/corba/se/impl/orbutil/resources/sunorb_sv.properties
! 
src/share/classes/com/sun/corba/se/impl/orbutil/resources/sunorb_zh_CN.properties
! 
src/share/classes/com/sun/corba/se/impl/orbutil/resources/sunorb_zh_TW.properties

Changeset: e0f0b358cd2c
Author:mfang
Date:  2011-02-11 22:50 -0800
URL:   http://hg.openjdk.java.net/jdk7/tl/corba/rev/e0f0b358cd2c

Merge


Changeset: 563a8f8b5be3
Author:mfang
Date:  2011-02-11 23:35 -0800
URL:   http://hg.openjdk.java.net/jdk7/tl/corba/rev/563a8f8b5be3

Merge


Changeset: 49a96611c870
Author:cl
Date:  2011-02-18 14:23 -0800
URL:   http://hg.openjdk.java.net/jdk7/tl/corba/rev/49a96611c870

Added tag jdk7-b130 for changeset 563a8f8b5be3

! .hgtags

hg: jdk7/tl/langtools: 7 new changesets

[lana . steuck]

Changeset: 2cbaa43eb075
Author:lana
Date:  2011-02-14 16:31 -0800
URL:   http://hg.openjdk.java.net/jdk7/tl/langtools/rev/2cbaa43eb075

Merge

- test/tools/javac/TryWithResources/TwrInference.java
- test/tools/javac/TryWithResources/TwrIntersection.java
- test/tools/javac/TryWithResources/TwrIntersection02.java
- test/tools/javac/TryWithResources/TwrIntersection02.out

Changeset: a21c7f194d31
Author:mfang
Date:  2011-02-10 16:51 -0800
URL:   http://hg.openjdk.java.net/jdk7/tl/langtools/rev/a21c7f194d31

7017734: jdk7 message drop 1 translation integration
Reviewed-by: ogino, yhuang

! src/share/classes/com/sun/tools/apt/resources/apt_ja.properties
! src/share/classes/com/sun/tools/apt/resources/apt_zh_CN.properties
! 
src/share/classes/com/sun/tools/doclets/formats/html/resources/standard_ja.properties
! 
src/share/classes/com/sun/tools/doclets/formats/html/resources/standard_zh_CN.properties
! 
src/share/classes/com/sun/tools/doclets/internal/toolkit/resources/doclets_ja.properties
! 
src/share/classes/com/sun/tools/doclets/internal/toolkit/resources/doclets_zh_CN.properties
! src/share/classes/com/sun/tools/javac/resources/compiler_ja.properties
! src/share/classes/com/sun/tools/javac/resources/compiler_zh_CN.properties
! src/share/classes/com/sun/tools/javac/resources/javac_ja.properties
! src/share/classes/com/sun/tools/javac/resources/javac_zh_CN.properties
! src/share/classes/com/sun/tools/javadoc/resources/javadoc_ja.properties
! src/share/classes/com/sun/tools/javadoc/resources/javadoc_zh_CN.properties
! src/share/classes/com/sun/tools/javah/resources/l10n_ja.properties
! src/share/classes/com/sun/tools/javah/resources/l10n_zh_CN.properties

Changeset: 4cdea0752a48
Author:mfang
Date:  2011-02-11 22:58 -0800
URL:   http://hg.openjdk.java.net/jdk7/tl/langtools/rev/4cdea0752a48

Merge


Changeset: 26071d11c613
Author:mfang
Date:  2011-02-11 23:49 -0800
URL:   http://hg.openjdk.java.net/jdk7/tl/langtools/rev/26071d11c613

Merge


Changeset: 7a98db8cbfce
Author:ohair
Date:  2011-02-15 12:34 -0800
URL:   http://hg.openjdk.java.net/jdk7/tl/langtools/rev/7a98db8cbfce

Merge


Changeset: 6cdb76cf4d1a
Author:cl
Date:  2011-02-18 14:23 -0800
URL:   http://hg.openjdk.java.net/jdk7/tl/langtools/rev/6cdb76cf4d1a

Added tag jdk7-b130 for changeset 7a98db8cbfce

! .hgtags

Changeset: 4b0491db73af
Author:lana
Date:  2011-02-23 10:34 -0800
URL:   http://hg.openjdk.java.net/jdk7/tl/langtools/rev/4b0491db73af

Merge

hg: jdk7/tl/jaxws: Added tag jdk7-b130 for changeset ba1fac1c2083

[lana . steuck]

Changeset: a8ffd75ad5df
Author:cl
Date:  2011-02-18 14:23 -0800
URL:   http://hg.openjdk.java.net/jdk7/tl/jaxws/rev/a8ffd75ad5df

Added tag jdk7-b130 for changeset ba1fac1c2083

! .hgtags

Re: Review request for 7020513 : Add com.sun.xml.internal to the "package.access" property in java.security

[Rama Pulavarthi]

Hi Alan,
On 2/23/11 12:44 AM, Alan Bateman wrote:

Rama Pulavarthi wrote:

Hi,
Need reviewer for CR 7020513: Add com.sun.xml.internal to the
"package.access" property in $JAVA_HOME/lib/security/java.security
Webrev is available at
http://cr.openjdk.java.net/~ohair/openjdk7/jdk7-tl-jaxws-7020513/webrev/

thanks,
Rama Pulavarthi

Rama - it might be better to bring this to security-dev@openjdk as 
that is where the security property files and XML DSIG implementation 
are maintained.



CCing security-dev.

For background on this issue, this is not a new one. I am trying to port 
the old fixes made in jdk repo as part of earlier jax-ws integrations in 
to Open JDK 6.
The original issue CR 6592792 was fixed in JDK 6u7 and then ported to 
Open JDK 6.  When Open JDK 6 transistioned to Hg, it was committed as 
[1]. As fix for CR 6831313 
:update jaxws in 
OpenJDK7 to 2.1 plus bug fixes from OpenJDK 6, Only the changes in jaxws 
sources are ported to JDK 7 [2]. But, there are other changes that are 
made in jdk repo required for this fix.


Just on the test, it would be great if it didn't require the script as 
these can be problematic (looks like it will fail if run on Cygwin for 
example). Also looks like the dates on the tests are 2009.


Just porting the fix along with tests from Open JDK 6 workspace, that's 
why I kept the old date. Does it need to be changed? This test was added 
then following the convention of other tests. I will check other tests 
in JDK 7 to see if it needs any update.


thanks,
Rama Pulavarthi

[1] http://hg.openjdk.java.net/jdk6/jdk6/jdk/rev/586feec8273d 

[2] http://hg.openjdk.java.net/jdk7/build/jaxws/rev/31822b475baa 


-Alan.

Re: Review CR #6611830: UUID thread-safety and performance improvements

[Brian Goetz]

Ignore my comment -- I was reading the diffs backwards :(

On 2/22/2011 11:53 PM, Brian Goetz wrote:

I think you have a potential visibility problem here. You use -1 as the
initial value, but observing threads might see instead the default value
if the initializing write is not visible, and mistakenly think that the
zero default value represents a computed value. (This is different from
the trick employed by String.hashCode(), which does not use an
initializer. Can you get the same result using zero instead?)

The key to making the String.hashCode() trick work is that, while there
is definitely a data race, it is benign because the field can only ever
take on one non-default value, and that value is computed
deterministically from immutable state.

On 2/22/2011 7:29 PM, Mike Duigou wrote:

Daniel Aioanei reported via Josh Bloch a data race issue with the UUID
accessor and hashCode() methods. I've prepared a webrev with the
suggested changes:

http://cr.openjdk.java.net/~mduigou/6611830/webrev.0/webrev/

I've tested the change against the standard UUID tests and did a
microbenchmark test of one method, variant(), to see what impact not
caching had on performance. Since there was only negligible change in
performance vs. the existing UUID implementation I am comfortable with
eliminating the cache values. It would appear that a field access plus
a shift is not a significant cost over a field access.

Mike

hg: jdk7/tl/jdk: 7017493: ConcurrentLinkedDeque: Unexpected initialization order can lead to crash due to use of Unsafe

[chris . hegarty]

Changeset: 892c3fc7249e
Author:dl
Date:  2011-02-23 14:56 +
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/892c3fc7249e

7017493: ConcurrentLinkedDeque: Unexpected initialization order can lead to 
crash due to use of Unsafe
Reviewed-by: chegar

! src/share/classes/java/util/concurrent/ConcurrentLinkedDeque.java
! src/share/classes/java/util/concurrent/ConcurrentLinkedQueue.java
! src/share/classes/java/util/concurrent/ConcurrentSkipListMap.java
! src/share/classes/java/util/concurrent/ConcurrentSkipListSet.java
! src/share/classes/java/util/concurrent/CopyOnWriteArrayList.java
! src/share/classes/java/util/concurrent/LinkedTransferQueue.java
! src/share/classes/java/util/concurrent/Phaser.java
! src/share/classes/java/util/concurrent/PriorityBlockingQueue.java
! src/share/classes/java/util/concurrent/SynchronousQueue.java

java.util.Objects.requireNonNull: an opportunity for NullArgumentException?

[Martin Desruisseaux]

Hello all

I apologize if this question was already debated previously. I just wonder: 
since JDK 7 defines an Object.requireNonNull(…) method for explicit checks of 
argument value, does the Project-Coin expert group has considered the addition 
of an explicit NullArgumentException extends NullPointerException for making 
clear that this exception is the result of an argument check rather than some 
bug hidden in the middle of a code?


Some kind of NullArgumentException seems a common addition in many libraries 
built on top of JDK. To name just a few from a quick Google search:


http://commons.apache.org/lang/api-2.3/org/apache/commons/lang/NullArgumentException.html
http://www.croftsoft.com/library/code/javadoc/core/com/croftsoft/core/lang/NullArgumentException.html
http://api.dpml.net/dpml/1.1.0/net/dpml/transit/NullArgumentException.html
http://www.geotoolkit.org/apidocs/org/geotoolkit/util/NullArgumentException.html

A blog:
http://closingbraces.net/2007/02/26/nullargumentexception/

Regards,

Martin

Re: Review CR #6611830: UUID thread-safety and performance improvements

[Alan Bateman]

Mike Duigou wrote:

Daniel Aioanei reported via Josh Bloch a data race issue with the UUID accessor 
and hashCode() methods. I've prepared a webrev with the suggested changes:

http://cr.openjdk.java.net/~mduigou/6611830/webrev.0/webrev/

I've tested the change against the standard UUID tests and did a microbenchmark 
test of one method, variant(), to see what impact not caching had on 
performance. Since there was only negligible change in performance vs. the 
existing UUID implementation I am comfortable with eliminating the cache 
values. It would appear that a field access plus a shift is not a significant 
cost over a field access.

Mike
This looks good to me, but I had to pause on the variant method to prove 
to myself that it's right - I don't know if a comment would help there.


-Alan.

hg: jdk7/tl/langtools: 2 new changesets

[maurizio . cimadamore]

Changeset: 015dc9a63efc
Author:mcimadamore
Date:  2011-02-23 14:16 +
URL:   http://hg.openjdk.java.net/jdk7/tl/langtools/rev/015dc9a63efc

7020657: Javac rejects a fairly common idiom with raw override and interfaces
Summary: name clash should not be reported if subinterface/implementing class 
resolves the clash by defining common overrider
Reviewed-by: jjg

! src/share/classes/com/sun/tools/javac/comp/Check.java
+ test/tools/javac/generics/7020657/T7020657neg.java
+ test/tools/javac/generics/7020657/T7020657neg.out
+ test/tools/javac/generics/7020657/T7020657pos.java

Changeset: 3ab7bb46c5c1
Author:mcimadamore
Date:  2011-02-23 14:17 +
URL:   http://hg.openjdk.java.net/jdk7/tl/langtools/rev/3ab7bb46c5c1

7019631: issues in test headers in b130
Summary: fix to test headers not containing correct bug ID
Reviewed-by: jjg

! test/tools/javac/AnonStaticMember_2.java
! test/tools/javac/InterfaceInInner.java
! test/tools/javac/QualifiedNew.java
! test/tools/javac/generics/6969184/T6969184.java

Re: Review CR #6611830: UUID thread-safety and performance improvements

[Vitaly Davidovich]

resending as I just realized I replied only to Brian.

On Wed, Feb 23, 2011 at 12:19 AM, Vitaly Davidovich wrote:

> Hi David/Brian,
>
> Yes, I meant whether the "entire" string.hashcode technique can be used,
> including the default zero values.  I agree on the initial -1 assignment
> possibly not being visible across cores, leading to incorrect results.
>
> David, I did not see the actual bug report -- is there a link to it?
>
> Cheers,
> Vitaly
>
> On Tue, Feb 22, 2011 at 11:53 PM, Brian Goetz wrote:
>
>> I think you have a potential visibility problem here.  You use -1 as the
>> initial value, but observing threads might see instead the default value if
>> the initializing write is not visible, and mistakenly think that the zero
>> default value represents a computed value.  (This is different from the
>> trick employed by String.hashCode(), which does not use an initializer.  Can
>> you get the same result using zero instead?)
>>
>> The key to making the String.hashCode() trick work is that, while there is
>> definitely a data race, it is benign because the field can only ever take on
>> one non-default value, and that value is computed deterministically from
>> immutable state.
>>
>>
>> On 2/22/2011 7:29 PM, Mike Duigou wrote:
>>
>>> Daniel Aioanei reported via Josh Bloch a data race issue with the UUID
>>> accessor and hashCode() methods. I've prepared a webrev with the suggested
>>> changes:
>>>
>>> http://cr.openjdk.java.net/~mduigou/6611830/webrev.0/webrev/
>>>
>>> I've tested the change against the standard UUID tests and did a
>>> microbenchmark test of one method, variant(), to see what impact not caching
>>> had on performance. Since there was only negligible change in performance
>>> vs. the existing UUID implementation I am comfortable with eliminating the
>>> cache values. It would appear that a field access plus a shift is not a
>>> significant cost over a field access.
>>>
>>> Mike
>>>
>>
>
>
> --
> Vitaly
> 617-548-7007 (mobile)
>



-- 
Vitaly
617-548-7007 (mobile)

Re: Review request for 7020513 : Add com.sun.xml.internal to the "package.access" property in java.security

[Alan Bateman]

Rama Pulavarthi wrote:

Hi,
Need reviewer for CR 7020513: Add com.sun.xml.internal to the
"package.access" property in $JAVA_HOME/lib/security/java.security
Webrev is available at
http://cr.openjdk.java.net/~ohair/openjdk7/jdk7-tl-jaxws-7020513/webrev/

thanks,
Rama Pulavarthi

  
Rama - it might be better to bring this to security-dev@openjdk as that 
is where the security property files and XML DSIG implementation are 
maintained.


Just on the test, it would be great if it didn't require the script as 
these can be problematic (looks like it will fail if run on Cygwin for 
example). Also looks like the dates on the tests are 2009.


-Alan.