ISPN-1135 Fix issues suggested in Klockwork report

infinispan · May 27, 2011 · 3bf19c7 · 3bf19c7
1 parent cead539
commit 3bf19c7
Show file tree

Hide file tree

Showing 41 changed files with 382 additions and 192 deletions.
diff --git a/cachestore/bdbje/src/main/java/org/infinispan/loaders/bdbje/BdbjeCacheStoreConfig.java b/cachestore/bdbje/src/main/java/org/infinispan/loaders/bdbje/BdbjeCacheStoreConfig.java
@@ -25,6 +25,7 @@
 import org.infinispan.loaders.AbstractCacheStoreConfig;
 import org.infinispan.loaders.CacheLoaderException;
 import org.infinispan.util.FileLookup;
+import org.infinispan.util.Util;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -161,6 +162,8 @@ public Properties readEnvironmentProperties() throws CacheLoaderException {
             p.load(i);
          } catch (IOException ioe) {
             throw new CacheLoaderException("Unable to read environment properties file " + environmentPropertiesFile, ioe);
+         } finally {
+            Util.close(i);
          }
          return p;
       } else {

diff --git a/...e/cassandra/src/main/java/org/infinispan/loaders/cassandra/CassandraCacheStoreConfig.java b/...e/cassandra/src/main/java/org/infinispan/loaders/cassandra/CassandraCacheStoreConfig.java
@@ -32,6 +32,7 @@
 import org.infinispan.loaders.LockSupportCacheStoreConfig;
 import org.infinispan.loaders.keymappers.DefaultTwoWayKey2StringMapper;
 import org.infinispan.util.FileLookup;
+import org.infinispan.util.Util;
 
 /**
  * Configures {@link CassandraCacheStore}.
@@ -208,7 +209,10 @@ private void readConfigurationProperties() throws CacheLoaderException {
 				p.load(i);
 			} catch (IOException ioe) {
 				throw new CacheLoaderException("Unable to read environment properties file " + configurationPropertiesFile, ioe);
-			}
+         } finally {
+            Util.close(i);
+         }
+
 			// Apply all properties to the PoolProperties object
 			for(String propertyName : p.stringPropertyNames()) {
 				poolProperties.set(propertyName, p.getProperty(propertyName));

diff --git a/cachestore/cloud/src/main/java/org/infinispan/loaders/cloud/CloudCacheStore.java b/cachestore/cloud/src/main/java/org/infinispan/loaders/cloud/CloudCacheStore.java
@@ -414,7 +414,7 @@ private void writeToBlob(Blob blob, Bucket bucket) throws CacheLoaderException {
 
    private Bucket readFromBlob(Blob blob, String bucketName) throws CacheLoaderException {
       if (blob == null) {
-        return null;
+        throw new CacheLoaderException("Blob not found");
       }
       try {
          Bucket bucket;

diff --git a/cachestore/remote/src/main/java/org/infinispan/loaders/remote/RemoteCacheStoreConfig.java b/cachestore/remote/src/main/java/org/infinispan/loaders/remote/RemoteCacheStoreConfig.java
@@ -26,6 +26,7 @@
 import org.infinispan.loaders.AbstractCacheStoreConfig;
 import org.infinispan.manager.CacheContainer;
 import org.infinispan.util.FileLookup;
+import org.infinispan.util.Util;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -89,6 +90,8 @@ public void setHotRodClientPropertiesFile(String hotRodClientProperties) {
       } catch (IOException e) {
          log.error("Issues while loading properties from file", e);
          throw new CacheException(e);
+      } finally {
+         Util.close(inputStream);
       }
    }
 }
diff --git a/client/hotrod-client/src/main/java/org/infinispan/client/hotrod/RemoteCacheManager.java b/client/hotrod-client/src/main/java/org/infinispan/client/hotrod/RemoteCacheManager.java
@@ -33,6 +33,7 @@
 import org.infinispan.manager.CacheContainer;
 import org.infinispan.marshall.Marshaller;
 import org.infinispan.client.hotrod.logging.Log;
+import org.infinispan.util.Util;
 import org.infinispan.util.logging.LogFactory;
 
 import java.io.IOException;
@@ -229,7 +230,11 @@ public RemoteCacheManager(boolean start) {
          log.couldNotFindPropertiesFile(HOTROD_CLIENT_PROPERTIES);
          config = new ConfigurationProperties();
       } else {
-         loadFromStream(stream);
+         try {
+            loadFromStream(stream);
+         } finally {
+            Util.close(stream);
+         }
       }
       if (start) start();
    }
@@ -282,10 +287,19 @@ public RemoteCacheManager(String servers) {
     * @throws HotRodClientException if properties could not be loaded
     */
    public RemoteCacheManager(URL config, boolean start) {
+      InputStream stream = null;
       try {
-         loadFromStream(config.openStream());
+         stream = config.openStream();
+         loadFromStream(stream);
       } catch (IOException e) {
          throw new HotRodClientException("Could not read URL:" + config, e);
+      } finally {
+         try {
+            if (stream != null)
+               stream.close();
+         } catch (IOException e) {
+            // ignore
+         }
       }
       if (start)
          start();

diff --git a/...od-client/src/main/java/org/infinispan/client/hotrod/impl/transport/tcp/TcpTransport.java b/...od-client/src/main/java/org/infinispan/client/hotrod/impl/transport/tcp/TcpTransport.java
@@ -40,6 +40,7 @@
 import org.infinispan.client.hotrod.impl.transport.AbstractTransport;
 import org.infinispan.client.hotrod.impl.transport.TransportFactory;
 import org.infinispan.client.hotrod.logging.Log;
+import org.infinispan.util.Util;
 import org.infinispan.util.logging.LogFactory;
 
 /**
@@ -58,6 +59,7 @@ public class TcpTransport extends AbstractTransport {
    private static final boolean trace = log.isTraceEnabled();
 
    private final Socket socket;
+   private final SocketChannel socketChannel;
    private final InputStream socketInputStream;
    private final BufferedOutputStream socketOutputStream;
    private final InetSocketAddress serverAddress;
@@ -69,7 +71,7 @@ public TcpTransport(InetSocketAddress serverAddress, TransportFactory transportF
       super(transportFactory);
       this.serverAddress = serverAddress;
       try {
-         SocketChannel socketChannel = SocketChannel.open(serverAddress);
+         socketChannel = SocketChannel.open(serverAddress);
          socket = socketChannel.socket();
          socket.setTcpNoDelay(transportFactory.isTcpNoDelay());
          socket.setSoTimeout(transportFactory.getSoTimeout());
@@ -259,13 +261,19 @@ public int hashCode() {
 
    public void destroy() {
       try {
-         socket.close();
+         if (socketInputStream != null) socketInputStream.close();
+         if (socketOutputStream != null) socketOutputStream.close();
+         if (socketChannel != null) socketChannel.close();
+         if (socket != null) socket.close();
          if (trace) {
             log.tracef("Successfully closed socket: %s", socket);
          }
       } catch (IOException e) {
          invalid = true;
          log.errorClosingSocket(this, e);
+         // Just in case an exception is thrown, make sure they're fully closed
+         Util.close(socketInputStream, socketOutputStream, socketChannel);
+         Util.close(socket);
       }
    }
 

diff --git a/...trod-client/src/main/java/org/infinispan/client/hotrod/marshall/ApacheAvroMarshaller.java b/...trod-client/src/main/java/org/infinispan/client/hotrod/marshall/ApacheAvroMarshaller.java
@@ -328,9 +328,8 @@ void write(Object o, Encoder encoder) throws IOException {
             marshaller.objectToBuffer(element, encoder);
       }
 
-      Collection createCollection(int size) {
-         return null;
-      }
+      abstract Collection createCollection(int size);
+
    }
 
    private static class ListMarshallableType extends CollectionMarshallableType {
@@ -369,6 +368,11 @@ void write(Object o, Encoder encoder) throws IOException {
          }
 
       }
+
+      @Override
+      Collection createCollection(int size) {
+         return null; // Ignored for this class
+      }
    }
 
    private class SetMarshallableType extends CollectionMarshallableType {

diff --git a/core/src/main/java/org/infinispan/affinity/KeyAffinityServiceImpl.java b/core/src/main/java/org/infinispan/affinity/KeyAffinityServiceImpl.java
@@ -120,6 +120,9 @@ public Object getKeyForAddress(Address address) {
       if (address == null)
          throw new NullPointerException("Null address not supported!");
       BlockingQueue queue = address2key.get(address);
+      if (queue == null)
+         return null; // No address for key, unlikely
+
       try {
          maxNumberInvariant.readLock().lock();
          Object result;

diff --git a/core/src/main/java/org/infinispan/commands/read/EntrySetCommand.java b/core/src/main/java/org/infinispan/commands/read/EntrySetCommand.java
@@ -106,7 +106,7 @@ public boolean contains(Object o) {
          @SuppressWarnings("rawtypes")
          Map.Entry e = (Map.Entry) o;
          CacheEntry ce = lookedUpEntries.get(e.getKey());
-         if (ce.isRemoved()) {
+         if (ce == null || ce.isRemoved()) {
             return false;
          }
          if (ce.isChanged() || ce.isCreated()) {

diff --git a/core/src/main/java/org/infinispan/commands/read/KeySetCommand.java b/core/src/main/java/org/infinispan/commands/read/KeySetCommand.java
@@ -98,7 +98,7 @@ public int size() {
       @Override
       public boolean contains(Object o) {
          CacheEntry e = lookedUpEntries.get(o);
-         if (e.isRemoved()) {
+         if (e == null || e.isRemoved()) {
             return false;
          } else if (e.isChanged() || e.isCreated()) {
             return true;

diff --git a/core/src/main/java/org/infinispan/commands/read/MapReduceCommand.java b/core/src/main/java/org/infinispan/commands/read/MapReduceCommand.java
@@ -125,6 +125,9 @@ public Object perform(InvocationContext context) throws Throwable {
                selectedKeys.add(key);               
             }
          }
+         if (keys == null)
+            keys = new HashSet<Object>();
+
          keys.addAll(selectedKeys);
       }
       log.tracef("For %s at %s invoking mapper on keys %s", this, localAddress, keys);

diff --git a/core/src/main/java/org/infinispan/config/Configuration.java b/core/src/main/java/org/infinispan/config/Configuration.java
@@ -496,7 +496,7 @@ public void setCacheMode(String cacheMode) {
    }
 
    public String getCacheModeString() {
-      return clustering.mode == null ? null : clustering.mode.toString();
+      return clustering.mode == null ? "none" : clustering.mode.toString();
    }
 
    /**

diff --git a/core/src/main/java/org/infinispan/config/InfinispanConfiguration.java b/core/src/main/java/org/infinispan/config/InfinispanConfiguration.java
@@ -27,6 +27,7 @@
 import org.infinispan.config.parsing.XmlConfigurationParser;
 import org.infinispan.util.FileLookup;
 import org.infinispan.util.StringPropertyReplacer;
+import org.infinispan.util.Util;
 import org.infinispan.util.logging.Log;
 import org.infinispan.util.logging.LogFactory;
 import org.xml.sax.InputSource;
@@ -127,7 +128,11 @@ public static InfinispanConfiguration newInfinispanConfiguration(String configFi
 
       InputStream inputStream = configFileName != null ? findInputStream(configFileName) : null;
       InputStream schemaIS = schemaFileName != null ? findSchemaInputStream(schemaFileName) : null;
-      return newInfinispanConfiguration(inputStream, schemaIS, cbv);
+      try {
+         return newInfinispanConfiguration(inputStream, schemaIS, cbv);
+      } finally {
+         Util.close(inputStream, schemaIS);
+      }
    }
 
    /**

diff --git a/core/src/main/java/org/infinispan/config/parsing/ConfigFilesConvertor.java b/core/src/main/java/org/infinispan/config/parsing/ConfigFilesConvertor.java
@@ -77,37 +77,44 @@ public void parse(InputStream is, OutputStream os, String xsltFile) throws Excep
          throw new IllegalStateException("Cold not find xslt file! : " + xsltFile);
       }
 
-      Document document = getInputDocument(is);
-
-      // Use a Transformer for output
-      Transformer transformer = getTransformer(xsltInStream);
-
-      DOMSource source = new DOMSource(document);
-      ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
-      StreamResult result = new StreamResult(byteArrayOutputStream);
-      transformer.transform(source, result);
-
-      InputStream indentation = new FileLookup().lookupFile("xslt/indent.xslt");
-      // Use a Transformer for output
-      transformer = getTransformer(indentation);
-      StreamResult finalResult = new StreamResult(os);
-      StreamSource rawResult = new StreamSource(new ByteArrayInputStream(byteArrayOutputStream.toByteArray()));
-      transformer.transform(rawResult, finalResult);
-      xsltInStream.close();
+      try {
+         Document document = getInputDocument(is);
+
+         // Use a Transformer for output
+         Transformer transformer = getTransformer(xsltInStream);
+
+         DOMSource source = new DOMSource(document);
+         ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
+         StreamResult result = new StreamResult(byteArrayOutputStream);
+         transformer.transform(source, result);
+
+         InputStream indentation = new FileLookup().lookupFile("xslt/indent.xslt");
+         try {
+            // Use a Transformer for output
+            transformer = getTransformer(indentation);
+            StreamResult finalResult = new StreamResult(os);
+            StreamSource rawResult = new StreamSource(new ByteArrayInputStream(byteArrayOutputStream.toByteArray()));
+            transformer.transform(rawResult, finalResult);
+         } finally {
+            Util.close(indentation);
+         }
+      } finally {
+         Util.close(xsltInStream);
+      }
    }
 
    /**
     * Writes to the <b>os</b> the infinispan 4.x configuration file resulted by transforming configuration file passed
     * in as <b>inputFile</b>. Transformation is performed according to the <b>xsltFile</b>. Both <b>inputFile</b> and he
-    * xslt file are looked up using a {@link org.jboss.cache.util.FileLookup}
+    * xslt file are looked up using a {@link org.infinispan.util.FileLookup}
     */
    public void parse(String inputFile, OutputStream os, String xsltFile) throws Exception {
-      InputStream stream = new FileLookup().lookupFile(inputFile);
+      InputStream stream = new FileLookup().lookupFileStrict(inputFile);
       try {
          parse(stream, os, xsltFile);
       }
       finally {
-         stream.close();
+         Util.close(stream);
       }
    }
 

diff --git a/core/src/main/java/org/infinispan/interceptors/CacheMgmtInterceptor.java b/core/src/main/java/org/infinispan/interceptors/CacheMgmtInterceptor.java
@@ -168,10 +168,13 @@ public long getEvictions() {
    @ManagedAttribute(description = "Percentage hit/(hit+miss) ratio for the cache")
    @Metric(displayName = "Hit ratio", units = Units.PERCENTAGE, displayType = DisplayType.SUMMARY)
    public double getHitRatio() {
-      double total = hits.get() + misses.get();
-      if (total == 0)
+      long hitsL = hits.get();
+      double total = hitsL + misses.get();
+      // The reason for <= is that equality checks
+      // should be avoided for floating point numbers.
+      if (total <= 0)
          return 0;
-      return (hits.get() / total);
+      return (hitsL / total);
    }
 
    @ManagedAttribute(description = "read/writes ratio for the cache")