Revision: 10404
Author:   zun...@google.com
Date:     Mon Jun 27 09:39:39 2011
Log: Bug triggered when SourceFileCompilationUnit.asCachedCompilation() unit
was called before source was read for the first time.

Review at http://gwt-code-reviews.appspot.com/1461803

http://code.google.com/p/google-web-toolkit/source/detail?r=10404

Modified:
/trunk/dev/core/src/com/google/gwt/core/ext/linker/impl/StandardPublicResource.java
 /trunk/dev/core/src/com/google/gwt/dev/javac/CompilationUnitBuilder.java
 /trunk/dev/core/src/com/google/gwt/dev/javac/JavaSourceParser.java
 /trunk/dev/core/src/com/google/gwt/dev/javac/Shared.java
 /trunk/dev/core/src/com/google/gwt/dev/javac/SourceFileCompilationUnit.java
 /trunk/dev/core/src/com/google/gwt/dev/resource/Resource.java
 /trunk/dev/core/src/com/google/gwt/dev/resource/impl/FileResource.java
/trunk/dev/core/src/com/google/gwt/dev/resource/impl/ResourceOracleImpl.java
 /trunk/dev/core/src/com/google/gwt/dev/resource/impl/ZipFileResource.java
 /trunk/dev/core/src/com/google/gwt/dev/util/DiskCache.java
 /trunk/dev/core/src/com/google/gwt/dev/util/Util.java
 /trunk/dev/core/test/com/google/gwt/dev/javac/JdtBehaviorTest.java
 /trunk/dev/core/test/com/google/gwt/dev/resource/impl/FileResourceTest.java
/trunk/dev/core/test/com/google/gwt/dev/resource/impl/ResourceOracleImplTest.java
 /trunk/user/src/com/google/gwt/i18n/rebind/ResourceFactory.java
/trunk/user/src/com/google/gwt/uibinder/rebind/GwtResourceEntityResolver.java
 /trunk/user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java
 /trunk/user/src/com/google/gwt/uibinder/rebind/W3cDocumentBuilder.java
/trunk/user/test/com/google/gwt/uibinder/rebind/GwtResourceEntityResolverTest.java

=======================================
--- /trunk/dev/core/src/com/google/gwt/core/ext/linker/impl/StandardPublicResource.java Fri May 15 16:23:15 2009 +++ /trunk/dev/core/src/com/google/gwt/core/ext/linker/impl/StandardPublicResource.java Mon Jun 27 09:39:39 2011
@@ -47,6 +47,7 @@
       this.lastModified = lastModified;
     }

+    @SuppressWarnings("unused")
     @Override
     public InputStream getContents(TreeLogger logger)
         throws UnableToCompleteException {
@@ -80,7 +81,12 @@
   @Override
   public InputStream getContents(TreeLogger logger)
       throws UnableToCompleteException {
-    return resource.openContents();
+    try {
+      return resource.openContents();
+    } catch (IOException ex) {
+ logger.log (TreeLogger.ERROR, "Problem reading resource: " + resource.getLocation(), ex);
+      throw new UnableToCompleteException();
+    }
   }

   @Override
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/javac/CompilationUnitBuilder.java Wed Jun 22 09:13:57 2011 +++ /trunk/dev/core/src/com/google/gwt/dev/javac/CompilationUnitBuilder.java Mon Jun 27 09:39:39 2011
@@ -134,9 +134,9 @@
        * as too stale than too fresh.
        */
       lastModifed = resource.getLastModified();
-      InputStream in = resource.openContents();
       ByteArrayOutputStream out = new ByteArrayOutputStream(1024);
       try {
+        InputStream in = resource.openContents();
         Util.copy(in, out);
       } catch (IOException e) {
throw new RuntimeException("Unexpected error reading resource '" + resource + "'", e);
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/javac/JavaSourceParser.java Thu Dec 9 10:54:59 2010 +++ /trunk/dev/core/src/com/google/gwt/dev/javac/JavaSourceParser.java Mon Jun 27 09:39:39 2011
@@ -19,9 +19,10 @@
 import com.google.gwt.dev.javac.typemodel.JAbstractMethod;
 import com.google.gwt.dev.javac.typemodel.JClassType;
 import com.google.gwt.dev.javac.typemodel.JParameter;
+import com.google.gwt.dev.jjs.InternalCompilerException;
 import com.google.gwt.dev.resource.Resource;
-import com.google.gwt.dev.util.Util;
 import com.google.gwt.dev.util.Name.BinaryName;
+import com.google.gwt.dev.util.Util;

 import org.eclipse.jdt.internal.compiler.ast.AbstractMethodDeclaration;
 import org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration;
@@ -31,6 +32,7 @@
 import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
 import org.eclipse.jdt.internal.core.util.CodeSnippetParsingUtil;

+import java.io.IOException;
 import java.io.InputStream;
 import java.lang.ref.SoftReference;
 import java.util.ArrayList;
@@ -288,8 +290,13 @@
       Resource classSource = classSources.get(topType);
       String source = null;
       if (classSource != null) {
-        InputStream stream = classSource.openContents();
-        source = Util.readStreamAsString(stream);
+        try {
+          InputStream stream = classSource.openContents();
+          source = Util.readStreamAsString(stream);
+        } catch (IOException ex) {
+          throw new InternalCompilerException("Problem reading resource: "
+              + classSource.getLocation(), ex);
+        }
       }
       if (source == null) {
         // cache negative result so we don't try again
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/javac/Shared.java Fri Jun 3 12:47:44 2011 +++ /trunk/dev/core/src/com/google/gwt/dev/javac/Shared.java Mon Jun 27 09:39:39 2011
@@ -143,7 +143,7 @@
     }
   }

-  public static String readSource(Resource sourceFile) {
+  public static String readSource(Resource sourceFile) throws IOException {
     InputStream contents = sourceFile.openContents();
     return Util.readStreamAsString(contents);
   }
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/javac/SourceFileCompilationUnit.java Thu May 12 10:40:46 2011 +++ /trunk/dev/core/src/com/google/gwt/dev/javac/SourceFileCompilationUnit.java Mon Jun 27 09:39:39 2011
@@ -1,12 +1,12 @@
 /*
  * Copyright 2008 Google Inc.
- *
+ *
* Licensed under the Apache License, Version 2.0 (the "License"); you may not * use this file except in compliance with the License. You may obtain a copy of
  * the License at
- *
+ *
  * http://www.apache.org/licenses/LICENSE-2.0
- *
+ *
  * Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
  * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
@@ -17,9 +17,12 @@

 import com.google.gwt.dev.jjs.ast.JDeclaredType;
 import com.google.gwt.dev.resource.Resource;
+import com.google.gwt.util.tools.Utility;

 import org.eclipse.jdt.core.compiler.CategorizedProblem;

+import java.io.IOException;
+import java.io.InputStream;
 import java.util.Collection;
 import java.util.List;

@@ -56,7 +59,15 @@
   @Override
   public CachedCompilationUnit asCachedCompilationUnit() {
     if (sourceToken < 0) {
- sourceToken = diskCache.transferFromStream(sourceFile.openContents());
+      InputStream in = null;
+      try {
+        in = sourceFile.openContents();
+        sourceToken = diskCache.transferFromStream(in);
+      } catch (IOException ex) {
+ throw new RuntimeException("Can't read resource:" + sourceFile.getLocation(), ex);
+      } finally {
+        Utility.close(in);
+      }
     }
     return new CachedCompilationUnit(this, sourceToken, astToken);
   }
@@ -79,12 +90,16 @@
   @Deprecated
   @Override
   public String getSource() {
-    if (sourceToken < 0) {
-      String sourceCode = Shared.readSource(sourceFile);
-      sourceToken = diskCache.writeString(sourceCode);
-      return sourceCode;
-    } else {
-      return diskCache.readString(sourceToken);
+    try {
+      if (sourceToken < 0) {
+        String sourceCode = Shared.readSource(sourceFile);
+        sourceToken = diskCache.writeString(sourceCode);
+        return sourceCode;
+      } else {
+        return diskCache.readString(sourceToken);
+      }
+    } catch (IOException ex) {
+      throw new RuntimeException("Can't read resource:" + sourceFile, ex);
     }
   }

=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/resource/Resource.java Thu May 12 10:40:46 2011 +++ /trunk/dev/core/src/com/google/gwt/dev/resource/Resource.java Mon Jun 27 09:39:39 2011
@@ -15,6 +15,7 @@
  */
 package com.google.gwt.dev.resource;

+import java.io.IOException;
 import java.io.InputStream;
 import java.net.MalformedURLException;
 import java.net.URL;
@@ -81,11 +82,9 @@
   }

   /**
- * Returns the contents of the resource. May return <code>null</code> if this
-   * {@link Resource} has been invalidated by its containing
- * {@link ResourceOracle}. The caller is responsible for closing the stream. + * Returns the contents of the resource. The caller is responsible for closing the stream.
    */
-  public abstract InputStream openContents();
+  public abstract InputStream openContents() throws IOException;

   /**
    * Overridden to finalize; always returns {@link #getLocation()}.
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/resource/impl/FileResource.java Wed Aug 4 09:54:49 2010 +++ /trunk/dev/core/src/com/google/gwt/dev/resource/impl/FileResource.java Mon Jun 27 09:39:39 2011
@@ -17,7 +17,7 @@

 import java.io.File;
 import java.io.FileInputStream;
-import java.io.FileNotFoundException;
+import java.io.IOException;
 import java.io.InputStream;

 /**
@@ -29,8 +29,7 @@
   private final DirectoryClassPathEntry classPathEntry;
   private final File file;

-  public FileResource(DirectoryClassPathEntry classPathEntry,
-      String abstractPathName, File file) {
+ public FileResource(DirectoryClassPathEntry classPathEntry, String abstractPathName, File file) {
     assert (file.isFile()) : file + " is not a file.";
     this.classPathEntry = classPathEntry;
     this.abstractPathName = abstractPathName;
@@ -58,12 +57,8 @@
   }

   @Override
-  public InputStream openContents() {
-    try {
-      return new FileInputStream(file);
-    } catch (FileNotFoundException e) {
-      return null;
-    }
+  public InputStream openContents() throws IOException {
+    return new FileInputStream(file);
   }

   @Override
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/resource/impl/ResourceOracleImpl.java Thu May 12 10:40:46 2011 +++ /trunk/dev/core/src/com/google/gwt/dev/resource/impl/ResourceOracleImpl.java Mon Jun 27 09:39:39 2011
@@ -101,7 +101,7 @@
     }

     @Override
-    public InputStream openContents() {
+    public InputStream openContents() throws IOException {
       return resource.openContents();
     }

@@ -121,6 +121,7 @@
       this.pathPrefix = pathPrefix;
     }

+    @Override
     public int compareTo(ResourceData other) {
       // Rerooted takes precedence over not rerooted.
       if (this.resource.wasRerooted() != other.resource.wasRerooted()) {
@@ -393,12 +394,14 @@
     this(getAllClassPathEntries(logger, classLoader));
   }

+  @Override
   public void clear() {
     exposedPathNames = Collections.emptySet();
     exposedResourceMap = Collections.emptyMap();
     exposedResources = Collections.emptySet();
   }

+  @Override
   public Set<String> getPathNames() {
     return exposedPathNames;
   }
@@ -407,10 +410,12 @@
     return pathPrefixSet;
   }

+  @Override
   public Map<String, Resource> getResourceMap() {
     return exposedResourceMap;
   }

+  @Override
   public Set<Resource> getResources() {
     return exposedResources;
   }
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/resource/impl/ZipFileResource.java Thu Apr 21 10:55:39 2011 +++ /trunk/dev/core/src/com/google/gwt/dev/resource/impl/ZipFileResource.java Mon Jun 27 09:39:39 2011
@@ -71,13 +71,8 @@
   }

   @Override
-  public InputStream openContents() {
-    try {
- return classPathEntry.getZipFile().getInputStream(new ZipEntry(path));
-    } catch (IOException e) {
-      // The spec for this method says it can return null.
-      return null;
-    }
+  public InputStream openContents() throws IOException {
+    return classPathEntry.getZipFile().getInputStream(new ZipEntry(path));
   }

   @Override
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/util/DiskCache.java Fri Mar 25 16:15:23 2011 +++ /trunk/dev/core/src/com/google/gwt/dev/util/DiskCache.java Mon Jun 27 09:39:39 2011
@@ -47,6 +47,7 @@
    */

   private static class Shutdown implements Runnable {
+    @Override
     public void run() {
       for (WeakReference<DiskCache> ref : shutdownList) {
         try {
@@ -136,11 +137,15 @@
   }

   /**
-   * Write the rest of the data in an input stream to disk.
+ * Write the rest of the data in an input stream to disk. Note: this method
+   * does not close the InputStream.
+   *
+   * @param in open stream containing the data to write to the disk cache.
    *
    * @return a token to retrieve the data later
    */
   public synchronized long transferFromStream(InputStream in) {
+    assert in != null;
     byte[] buf = Util.takeThreadLocalBuf();
     try {
       long position = moveToEndPosition();
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/util/Util.java Mon Jun 20 09:36:25 2011 +++ /trunk/dev/core/src/com/google/gwt/dev/util/Util.java Mon Jun 27 09:39:39 2011
@@ -296,6 +296,7 @@
    */
public static void deleteFilesStartingWith(File dir, final String prefix) {
     File[] toDelete = dir.listFiles(new FilenameFilter() {
+      @Override
       public boolean accept(File dir, String name) {
         return name.startsWith(prefix);
       }
@@ -714,6 +715,10 @@
       throw new RuntimeException(
           "The JVM does not support the compiler's default encoding.", e);
     } catch (IOException e) {
+ // TODO(zundel): Consider allowing this exception out. The pattern in this
+      // file is to convert IOException to null, but in references to this
+ // method, there are few places that check for null and do something sane,
+      // the rest just throw an NPE and obscure the root cause.
       return null;
     }
   }
=======================================
--- /trunk/dev/core/test/com/google/gwt/dev/javac/JdtBehaviorTest.java Thu Apr 21 07:48:58 2011 +++ /trunk/dev/core/test/com/google/gwt/dev/javac/JdtBehaviorTest.java Mon Jun 27 09:39:39 2011
@@ -34,6 +34,7 @@
 import org.eclipse.jdt.internal.compiler.env.NameEnvironmentAnswer;
 import org.eclipse.jdt.internal.compiler.problem.DefaultProblemFactory;

+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -150,7 +151,12 @@
     }

     public char[] getContents() {
-      return Shared.readSource(sourceFile).toCharArray();
+      try {
+        return Shared.readSource(sourceFile).toCharArray();
+      } catch (IOException ex) {
+        fail("Couldn't read sourceFile: " + sourceFile + " - " + ex);
+      }
+      return null;
     }

     public char[] getFileName() {
=======================================
--- /trunk/dev/core/test/com/google/gwt/dev/resource/impl/FileResourceTest.java Wed Aug 4 09:54:49 2010 +++ /trunk/dev/core/test/com/google/gwt/dev/resource/impl/FileResourceTest.java Mon Jun 27 09:39:39 2011
@@ -21,6 +21,7 @@

 import java.io.File;
 import java.io.IOException;
+import java.io.InputStream;

 public class FileResourceTest extends TestCase {

@@ -72,7 +73,15 @@
     // Delete the file.
     f.delete();

-    // Get can't contents anymore, either.
-    assertNull(r.openContents());
+    /*
+ * The resource is no longer available. Check to make sure we can't access its contents
+     *  through the API.
+     */
+    InputStream in = null;
+    try {
+      in = r.openContents();
+      fail("Open contents unexpectedly succeeded.");
+    } catch (IOException expected) {
+    }
   }
 }
=======================================
--- /trunk/dev/core/test/com/google/gwt/dev/resource/impl/ResourceOracleImplTest.java Wed Mar 23 12:56:08 2011 +++ /trunk/dev/core/test/com/google/gwt/dev/resource/impl/ResourceOracleImplTest.java Mon Jun 27 09:39:39 2011
@@ -134,7 +134,7 @@
     }

     @SuppressWarnings("deprecation")
-    public void assertResourcesGetURL() {
+    public void assertResourcesGetURL() throws IOException {
       for (Resource resource : resources) {
         URL url = resource.getURL();
assertNotNull("Resource " + resource + " had a null getURL()", url);
@@ -237,7 +237,7 @@
     testGetURLOnResourcesInCPE(cpe2zip);
   }

-  private void testGetURLOnResourcesInCPE(ClassPathEntry cpe) {
+ private void testGetURLOnResourcesInCPE(ClassPathEntry cpe) throws IOException {
     TreeLogger logger = createTestTreeLogger();

     ResourceOracleImpl oracle = createResourceOracle(cpe);
@@ -346,7 +346,6 @@
* multiple ResourceOracleImpls created from the same classloader return the
    * same list of ClassPathEntries.
    *
-   * @throws MalformedURLException
    */
   public void testRemoveDuplicates() {
     TreeLogger logger = createTestTreeLogger();
=======================================
--- /trunk/user/src/com/google/gwt/i18n/rebind/ResourceFactory.java Tue Apr 26 08:02:24 2011 +++ /trunk/user/src/com/google/gwt/i18n/rebind/ResourceFactory.java Mon Jun 27 09:39:39 2011
@@ -29,6 +29,7 @@
 import com.google.gwt.i18n.shared.GwtLocale;
 import com.google.gwt.i18n.shared.GwtLocaleFactory;

+import java.io.IOException;
 import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -104,7 +105,7 @@
         resources = localizableCtx.getResourceList(key);
         if (resources == null) {
           resources = new ResourceList();
-          addFileResources(clazz, locale, resourceMap, resources);
+          addFileResources(logger, clazz, locale, resourceMap, resources);
           AnnotationsResource annotationsResource = annotations.get(key);
           if (annotationsResource != null) {
             resources.add(annotationsResource);
@@ -133,7 +134,7 @@
     return name;
   }

-  private static void addFileResources(JClassType clazz, GwtLocale locale,
+ private static void addFileResources(TreeLogger logger, JClassType clazz, GwtLocale locale,
       Map<String, Resource> resourceMap, ResourceList resources) {
     // TODO: handle classes in the default package?
     String targetPath = clazz.getPackage().getName() + '.'
@@ -156,7 +157,14 @@
         resource = resourceMap.get(path);
       }
       if (resource != null) {
- AbstractResource found = element.load(resource.openContents(), locale);
+        InputStream resourceStream = null;
+        try {
+          resourceStream = resource.openContents();
+        } catch (IOException ex) {
+ logger.log(TreeLogger.ERROR, "Error opening resource: " + resource.getLocation());
+          throw new RuntimeException(ex);
+        }
+        AbstractResource found = element.load(resourceStream, locale);
         found.setPath(path);
         resources.add(found);
       }
=======================================
--- /trunk/user/src/com/google/gwt/uibinder/rebind/GwtResourceEntityResolver.java Mon Jun 14 13:37:36 2010 +++ /trunk/user/src/com/google/gwt/uibinder/rebind/GwtResourceEntityResolver.java Mon Jun 27 09:39:39 2011
@@ -15,6 +15,7 @@
  */
 package com.google.gwt.uibinder.rebind;

+import com.google.gwt.core.ext.TreeLogger;
 import com.google.gwt.dev.resource.Resource;
 import com.google.gwt.dev.resource.ResourceOracle;
 import com.google.gwt.dev.util.Util;
@@ -23,6 +24,8 @@
 import org.xml.sax.EntityResolver;
 import org.xml.sax.InputSource;

+import java.io.IOException;
+import java.io.InputStream;
 import java.io.StringReader;
 import java.util.Collections;
 import java.util.Map;
@@ -47,13 +50,16 @@
   private String pathBase;

   private final ResourceOracle resourceOracle;
-
-  public GwtResourceEntityResolver(ResourceOracle resourceOracle,
+  private final TreeLogger logger;
+
+ public GwtResourceEntityResolver(TreeLogger logger, ResourceOracle resourceOracle,
       String pathBase) {
+    this.logger = logger;
     this.resourceOracle = resourceOracle;
     this.pathBase = pathBase;
   }

+  @Override
   public InputSource resolveEntity(String publicId, String systemId) {
     String matchingPrefix = findMatchingPrefix(systemId);

@@ -69,7 +75,14 @@
     }

     if (resource != null) {
-      String content = Util.readStreamAsString(resource.openContents());
+      String content;
+      try {
+        InputStream resourceStream = resource.openContents();
+        content = Util.readStreamAsString(resourceStream);
+      } catch (IOException ex) {
+ logger.log(TreeLogger.ERROR, "Error reading resource: " + resource.getLocation());
+        throw new RuntimeException(ex);
+      }
       InputSource inputSource = new InputSource(new StringReader(content));
       inputSource.setPublicId(publicId);
       inputSource.setSystemId(resource.getPath());
=======================================
--- /trunk/user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java Tue May 24 10:01:44 2011 +++ /trunk/user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java Mon Jun 27 09:39:39 2011
@@ -34,6 +34,7 @@
 import org.w3c.dom.Document;
 import org.xml.sax.SAXParseException;

+import java.io.IOException;
 import java.io.PrintWriter;
 import java.util.List;

@@ -200,6 +201,8 @@
       }
doc = new W3cDomHelper(logger.getTreeLogger(), resourceOracle).documentFor(
           content, resource.getPath());
+    } catch (IOException iex) {
+      logger.die("Error opening resource:" + resource.getLocation(), iex);
     } catch (SAXParseException e) {
       logger.die(
           "Error parsing XML (line " + e.getLineNumber() + "): "
=======================================
--- /trunk/user/src/com/google/gwt/uibinder/rebind/W3cDocumentBuilder.java Tue Apr 26 08:02:24 2011 +++ /trunk/user/src/com/google/gwt/uibinder/rebind/W3cDocumentBuilder.java Mon Jun 27 09:39:39 2011
@@ -50,7 +50,7 @@
     this.logger = logger;
document = DocumentBuilderFactory.newInstance().newDocumentBuilder().newDocument();
     eltStack.push(document);
-    resolver = new GwtResourceEntityResolver(resourceOracle, pathBase);
+ resolver = new GwtResourceEntityResolver(logger, resourceOracle, pathBase);
   }

   /**
=======================================
--- /trunk/user/test/com/google/gwt/uibinder/rebind/GwtResourceEntityResolverTest.java Thu Apr 21 07:48:58 2011 +++ /trunk/user/test/com/google/gwt/uibinder/rebind/GwtResourceEntityResolverTest.java Mon Jun 27 09:39:39 2011
@@ -15,6 +15,7 @@
  */
 package com.google.gwt.uibinder.rebind;

+import com.google.gwt.core.ext.TreeLogger;
 import com.google.gwt.dev.javac.testing.impl.MockResource;
 import com.google.gwt.dev.javac.testing.impl.MockResourceOracle;
 import com.google.gwt.dev.resource.Resource;
@@ -44,7 +45,7 @@

private MockResourceOracle oracle = new MockResourceOracle(xhtmlEntResource);

- private GwtResourceEntityResolver resolver = new GwtResourceEntityResolver( + private GwtResourceEntityResolver resolver = new GwtResourceEntityResolver(TreeLogger.NULL,
       oracle, "");

   public void testAlmostCorrectAndOnceWorked() {

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to