RFR: 8060130: Simplify the synchronization of defining and getting java.lang.Package

[Peter Levart]

On 10/12/2014 04:09 AM, Claes Redestad wrote:
>
>
> http://cr.openjdk.java.net/~redestad/8060130/webrev.03/
>

Hi Claes,

First, one more nit:

1635                 if (!map.containsKey(pkgName)) {
1636                     map.put(pkgName, pkg);
1637                 }

..could be written as:

map.putIfAbsent(pkgName, pkg);



And second, I re-read the code of CachedManifest and noticed the 
following. You initialize the volatile instance field 'manifest' like that:

  609         private volatile Manifest manifest = EMPTY_MANIFEST;


So in following code:

  631         public Manifest getManifest() {
  632             resolveManifest();
  633             return manifest;
  634         }
  635
  636         private void resolveManifest() {
  637             if (manifest != null || url == null) {
  638                 return;
  639             }
  640             synchronized(this) {
  641                 if (manifest == null) {
  642                     manifest = AccessController.doPrivileged(new PrivilegedAction<Manifest>() {
  643                         public Manifest run() {
  644                             return loadManifest(fileName);
  645                         }
  646                     });
  647                 }
  648             }
  649         }



... loadManifest() will never be executed. getManifest() will always 
return EMPTY_MANIFEST.

You probably wanted line 637 to be written as:

if (manivest != EMPTY_MANIFEST || url == null) { ...


Likewise in loadManifest(), wou write:

  586     private static Manifest loadManifest(String fn) {
  587         try (FileInputStream fis = new FileInputStream(fn);
  588              JarInputStream jis = new JarInputStream(fis, false))
  589         {
  590             return jis.getManifest();
  591         } catch (IOException e) {
  592             return EMPTY_MANIFEST;
  593         }
  594     }

..but you probably wanted the line 592 to return 'null' instead.


Regards, Peter