Fixes for bugs found using FindBugs.

19 files changed, 104 insertions, 60 deletions

diff --git a/src/main/java/com/amazon/carbonado/CorruptEncodingException.java b/src/main/java/com/amazon/carbonado/CorruptEncodingException.java
index d35fbe4..1786cb8 100644
--- a/src/main/java/com/amazon/carbonado/CorruptEncodingException.java
+++ b/src/main/java/com/amazon/carbonado/CorruptEncodingException.java
@@ -27,7 +27,7 @@ public class CorruptEncodingException extends FetchException {
 

     private static final long serialVersionUID = 4543503149683482362L;

 

-    private Storable mStorable;

+    private transient Storable mStorable;

 

     public CorruptEncodingException() {

         super();

diff --git a/src/main/java/com/amazon/carbonado/OptimisticLockException.java b/src/main/java/com/amazon/carbonado/OptimisticLockException.java
index 909e0c6..ccd894c 100644
--- a/src/main/java/com/amazon/carbonado/OptimisticLockException.java
+++ b/src/main/java/com/amazon/carbonado/OptimisticLockException.java
@@ -30,7 +30,7 @@ public class OptimisticLockException extends PersistException {
 

     private static final long serialVersionUID = 4081788711829580886L;

 

-    private final Storable mStorable;

+    private final transient Storable mStorable;

 

     public OptimisticLockException() {

         super();

diff --git a/src/main/java/com/amazon/carbonado/adapter/TextAdapter.java b/src/main/java/com/amazon/carbonado/adapter/TextAdapter.java
index 8a51c3f..8a7542d 100644
--- a/src/main/java/com/amazon/carbonado/adapter/TextAdapter.java
+++ b/src/main/java/com/amazon/carbonado/adapter/TextAdapter.java
@@ -233,8 +233,11 @@ public @interface TextAdapter {
 

             Writer w = new OutputStreamWriter(blob.openOutputStream(), encoder);

             try {

-                w.write(text, 0, text.length());

-                w.close();

+                try {

+                    w.write(text, 0, text.length());

+                } finally {

+                    w.close();

+                }

             } catch (IOException e) {

                 throw toPersistException(e);

             }

diff --git a/src/main/java/com/amazon/carbonado/cursor/WorkFilePool.java b/src/main/java/com/amazon/carbonado/cursor/WorkFilePool.java
index 94c7b84..442d45c 100644
--- a/src/main/java/com/amazon/carbonado/cursor/WorkFilePool.java
+++ b/src/main/java/com/amazon/carbonado/cursor/WorkFilePool.java
@@ -141,6 +141,7 @@ class WorkFilePool {
     void unregisterWorkFileUser(MergeSortBuffer<?> buffer) {

         synchronized (mWorkFileUsers) {

             mWorkFileUsers.remove(buffer);

+            // Only one wait condition, so okay to not call notifyAll.

             mWorkFileUsers.notify();

         }

     }

diff --git a/src/main/java/com/amazon/carbonado/filter/BinaryOpFilter.java b/src/main/java/com/amazon/carbonado/filter/BinaryOpFilter.java
index 462acf4..d17d466 100644
--- a/src/main/java/com/amazon/carbonado/filter/BinaryOpFilter.java
+++ b/src/main/java/com/amazon/carbonado/filter/BinaryOpFilter.java
@@ -41,7 +41,7 @@ public abstract class BinaryOpFilter<S extends Storable> extends Filter<S> {
 

     BinaryOpFilter(Filter<S> left, Filter<S> right) {

         super(left == null ? null : left.getStorableType());

-        if (right == null) {

+        if (left == null || right == null) {

             throw new IllegalArgumentException();

         }

         if (left.getStorableType() != right.getStorableType()) {

diff --git a/src/main/java/com/amazon/carbonado/filter/PropertyFilter.java b/src/main/java/com/amazon/carbonado/filter/PropertyFilter.java
index 4d47ef5..71e575f 100644
--- a/src/main/java/com/amazon/carbonado/filter/PropertyFilter.java
+++ b/src/main/java/com/amazon/carbonado/filter/PropertyFilter.java
@@ -222,7 +222,11 @@ public class PropertyFilter<S extends Storable> extends Filter<S> {
      */

     public PropertyFilter<S> constant(Object value) {

         if (mBindID == BOUND_CONSTANT) {

-            if (mConstant == null && value == null || mConstant.equals(value)) {

+            if (mConstant == null) {

+                if (value == null) {

+                    return this;

+                }

+            } else if (mConstant.equals(value)) {

                 return this;

             }

         }

diff --git a/src/main/java/com/amazon/carbonado/info/StorableIndex.java b/src/main/java/com/amazon/carbonado/info/StorableIndex.java
index c1a5c47..49697c0 100644
--- a/src/main/java/com/amazon/carbonado/info/StorableIndex.java
+++ b/src/main/java/com/amazon/carbonado/info/StorableIndex.java
@@ -115,7 +115,7 @@ public class StorableIndex<S extends Storable> implements Appender {
 

         int size = properties.size();

         if (size == 0 || size != directions.size()) {

-            new IllegalArgumentException("No properties specified");

+            throw new IllegalArgumentException("No properties specified");

         }

 

         StorableIndex<S> index = new StorableIndex<S>

diff --git a/src/main/java/com/amazon/carbonado/info/StorableIntrospector.java b/src/main/java/com/amazon/carbonado/info/StorableIntrospector.java
index 29158f1..8be3d02 100644
--- a/src/main/java/com/amazon/carbonado/info/StorableIntrospector.java
+++ b/src/main/java/com/amazon/carbonado/info/StorableIntrospector.java
@@ -250,6 +250,12 @@ public class StorableIntrospector {
             this.name = name;

             this.direction = direction;

         }

+

+        @Override

+        public int hashCode() {

+            return name.hashCode() + direction.hashCode();

+        }

+

         @Override

         public boolean equals(Object obj) {

             if (this == obj) {

diff --git a/src/main/java/com/amazon/carbonado/lob/AbstractBlob.java b/src/main/java/com/amazon/carbonado/lob/AbstractBlob.java
index 3da80e9..6440978 100644
--- a/src/main/java/com/amazon/carbonado/lob/AbstractBlob.java
+++ b/src/main/java/com/amazon/carbonado/lob/AbstractBlob.java
@@ -90,30 +90,34 @@ public abstract class AbstractBlob implements Blob {
 

             char[] buffer = new char[(int) charLength];

 

-            Reader r = new InputStreamReader(openInputStream(), decoder);

-

             try {

-                int offset = 0;

-                int amt;

-                while ((amt = r.read(buffer, offset, buffer.length - offset)) >= 0) {

-                    offset += amt;

-                    if (amt == 0 && offset >= buffer.length) {

-                        // Expand capacity.

-                        charLength *= 2;

-                        if (charLength >= Integer.MAX_VALUE) {

-                            charLength = Integer.MAX_VALUE;

-                        }

-                        if (charLength <= buffer.length) {

-                            throw new IllegalArgumentException

-                                ("Blob is too long to fit in a String");

+                Reader r = new InputStreamReader(openInputStream(), decoder);

+

+                try {

+                    int offset = 0;

+                    int amt;

+                    while ((amt = r.read(buffer, offset, buffer.length - offset)) >= 0) {

+                        offset += amt;

+                        if (amt == 0 && offset >= buffer.length) {

+                            // Expand capacity.

+                            charLength *= 2;

+                            if (charLength >= Integer.MAX_VALUE) {

+                                charLength = Integer.MAX_VALUE;

+                            }

+                            if (charLength <= buffer.length) {

+                                throw new IllegalArgumentException

+                                    ("Blob is too long to fit in a String");

+                            }

+                            char[] newBuffer = new char[(int) charLength];

+                            System.arraycopy(buffer, 0, newBuffer, 0, buffer.length);

+                            buffer = newBuffer;

                         }

-                        char[] newBuffer = new char[(int) charLength];

-                        System.arraycopy(buffer, 0, newBuffer, 0, buffer.length);

-                        buffer = newBuffer;

                     }

-                }

 

-                return new String(buffer, 0, offset);

+                    return new String(buffer, 0, offset);

+                } finally {

+                    r.close();

+                }

             } catch (IOException e) {

                 throw AbstractClob.toFetchException(e);

             }

@@ -145,8 +149,11 @@ public abstract class AbstractBlob implements Blob {
             setLength(0);

             try {

                 Writer w = new OutputStreamWriter(openOutputStream(), charset);

-                w.write(value);

-                w.close();

+                try {

+                    w.write(value);

+                } finally {

+                    w.close();

+                }

             } catch (IOException e) {

                 throw AbstractClob.toPersistException(e);

             }

@@ -159,8 +166,11 @@ public abstract class AbstractBlob implements Blob {
                 try {

                     DataOutputStream out = new DataOutputStream(openOutputStream());

                     Writer w = new OutputStreamWriter(out, charset);

-                    w.write(value);

-                    w.close();

+                    try {

+                        w.write(value);

+                    } finally {

+                        w.close();

+                    }

                     newLength = out.size();

                 } catch (IOException e) {

                     throw AbstractClob.toPersistException(e);

diff --git a/src/main/java/com/amazon/carbonado/lob/AbstractClob.java b/src/main/java/com/amazon/carbonado/lob/AbstractClob.java
index 3ccb0f7..6e4e273 100644
--- a/src/main/java/com/amazon/carbonado/lob/AbstractClob.java
+++ b/src/main/java/com/amazon/carbonado/lob/AbstractClob.java
@@ -91,19 +91,22 @@ public abstract class AbstractClob implements Clob {
             try {

                 Reader r = openReader();

 

-                char[] buf = new char[iLen];

-                int offset = 0;

-                int amt;

-                while ((amt = r.read(buf, offset, iLen - offset)) > 0) {

-                    offset += amt;

-                }

-                r.close();

-

-                if (offset <= 0) {

-                    return "";

+                try {

+                    char[] buf = new char[iLen];

+                    int offset = 0;

+                    int amt;

+                    while ((amt = r.read(buf, offset, iLen - offset)) > 0) {

+                        offset += amt;

+                    }

+

+                    if (offset <= 0) {

+                        return "";

+                    }

+

+                    return new String(buf, 0, offset);

+                } finally {

+                    r.close();

                 }

-

-                return new String(buf, 0, offset);

             } catch (IOException e) {

                 throw toFetchException(e);

             }

@@ -140,8 +143,11 @@ public abstract class AbstractClob implements Clob {
 

                 try {

                     Writer w = openWriter();

-                    w.write(value);

-                    w.close();

+                    try {

+                        w.write(value);

+                    } finally {

+                        w.close();

+                    }

                 } catch (IOException e) {

                     throw toPersistException(e);

                 }

diff --git a/src/main/java/com/amazon/carbonado/qe/IndexedQueryAnalyzer.java b/src/main/java/com/amazon/carbonado/qe/IndexedQueryAnalyzer.java
index 6e87c7a..366c2d7 100644
--- a/src/main/java/com/amazon/carbonado/qe/IndexedQueryAnalyzer.java
+++ b/src/main/java/com/amazon/carbonado/qe/IndexedQueryAnalyzer.java
@@ -113,7 +113,7 @@ public class IndexedQueryAnalyzer<S extends Storable> {
 

         // Now try to find best foreign index.

 

-        if (bestLocalScore.getFilteringScore().isKeyMatch()) {

+        if (bestLocalScore != null && bestLocalScore.getFilteringScore().isKeyMatch()) {

             // Don't bother checking foreign indexes. The local one is perfect.

             return new Result(filter, bestLocalScore, bestLocalIndex, null, null);

         }

diff --git a/src/main/java/com/amazon/carbonado/qe/UnionQueryAnalyzer.java b/src/main/java/com/amazon/carbonado/qe/UnionQueryAnalyzer.java
index 64888ba..ea60490 100644
--- a/src/main/java/com/amazon/carbonado/qe/UnionQueryAnalyzer.java
+++ b/src/main/java/com/amazon/carbonado/qe/UnionQueryAnalyzer.java
@@ -330,7 +330,7 @@ public class UnionQueryAnalyzer<S extends Storable> implements QueryExecutorFact
         OrderingScore<S> score = result.getCompositeScore().getOrderingScore();

         OrderingList<S> handled = score.getHandledOrdering();

         for (OrderedProperty<S> property : handled) {

-            if (chained.equals(property)) {

+            if (chained.equals(property.getChainedProperty())) {

                 return property.getDirection();

             }

         }

diff --git a/src/main/java/com/amazon/carbonado/raw/RawCursor.java b/src/main/java/com/amazon/carbonado/raw/RawCursor.java
index 865cdb2..7962c45 100644
--- a/src/main/java/com/amazon/carbonado/raw/RawCursor.java
+++ b/src/main/java/com/amazon/carbonado/raw/RawCursor.java
@@ -82,7 +82,9 @@ public abstract class RawCursor<S> extends AbstractCursor<S> {
         if (maxPrefix <= 0 || startBound == null && endBound == null) {

             mPrefixLength = 0;

         } else {

-            int len = Math.min(maxPrefix, Math.min(startBound.length, endBound.length));

+            int len = Math.min(maxPrefix, 

+                               Math.min(startBound == null ? 0 : startBound.length, 

+                                        endBound == null ? 0 : endBound.length));

             int i;

             for (i=0; i<len; i++) {

                 if (startBound[i] != endBound[i]) {

diff --git a/src/main/java/com/amazon/carbonado/repo/indexed/IndexedRepository.java b/src/main/java/com/amazon/carbonado/repo/indexed/IndexedRepository.java
index 6fc6d57..b9380fc 100644
--- a/src/main/java/com/amazon/carbonado/repo/indexed/IndexedRepository.java
+++ b/src/main/java/com/amazon/carbonado/repo/indexed/IndexedRepository.java
@@ -169,12 +169,12 @@ class IndexedRepository implements Repository,
 

     public boolean isSupported(Class<Storable> type) {

         StorableInfoCapability cap = mRepository.getCapability(StorableInfoCapability.class);

-        return (cap == null) ? null : cap.isSupported(type);

+        return (cap == null) ? false : cap.isSupported(type);

     }

 

     public boolean isPropertySupported(Class<Storable> type, String name) {

         StorableInfoCapability cap = mRepository.getCapability(StorableInfoCapability.class);

-        return (cap == null) ? null : cap.isPropertySupported(type, name);

+        return (cap == null) ? false : cap.isPropertySupported(type, name);

     }

 

     public void close() {

diff --git a/src/main/java/com/amazon/carbonado/repo/replicated/ReplicatedRepository.java b/src/main/java/com/amazon/carbonado/repo/replicated/ReplicatedRepository.java
index 5edc42c..c292d9c 100644
--- a/src/main/java/com/amazon/carbonado/repo/replicated/ReplicatedRepository.java
+++ b/src/main/java/com/amazon/carbonado/repo/replicated/ReplicatedRepository.java
@@ -385,7 +385,7 @@ class ReplicatedRepository
             if (orderBy == null) {

                 Set<String> pkSet =

                     StorableIntrospector.examine(type).getPrimaryKeyProperties().keySet();

-                orderBy = pkSet.toArray(new String[0]);

+                orderBy = pkSet.toArray(new String[pkSet.size()]);

             }

         }

 

@@ -565,11 +565,15 @@ class ReplicatedRepository
                     lastMasterEntry = masterEntry;

                     masterEntry = null;

                 } else {

+                    // If compare is zero, replicaEntry and masterEntry are

+                    // either both null or both non-null.

+

                     if (replicaEntry == null && masterEntry == null) {

                         // Both cursors exhausted -- resync is complete.

                         break;

                     }

 

+                    // Both replicaEntry and masterEntry are non-null.

                     if (!replicaEntry.equalProperties(masterEntry)) {

                         // Replica is stale.

                         resyncTask = prepareResyncTask(trigger, replicaEntry, masterEntry);

diff --git a/src/main/java/com/amazon/carbonado/repo/sleepycat/BDBRepository.java b/src/main/java/com/amazon/carbonado/repo/sleepycat/BDBRepository.java
index 6aeb89e..7792536 100644
--- a/src/main/java/com/amazon/carbonado/repo/sleepycat/BDBRepository.java
+++ b/src/main/java/com/amazon/carbonado/repo/sleepycat/BDBRepository.java
@@ -718,6 +718,7 @@ abstract class BDBRepository<Txn>
                     } finally {

                         synchronized (this) {

                             mInProgress = false;

+                            // Only wait condition is mInProgress, so okay to not call notifyAll.

                             notify();

                         }

                         repository = null;

@@ -726,6 +727,7 @@ abstract class BDBRepository<Txn>
             } finally {

                 synchronized (this) {

                     mInProgress = false;

+                    // Only wait condition is mInProgress, so okay to not call notifyAll.

                     notify();

                 }

             }

@@ -802,12 +804,10 @@ abstract class BDBRepository<Txn>
 

         public void run() {

             while (true) {

-                synchronized (this) {

-                    try {

-                        wait(mSleepInterval);

-                    } catch (InterruptedException e) {

-                        break;

-                    }

+                try {

+                    Thread.sleep(mSleepInterval);

+                } catch (InterruptedException e) {

+                    break;

                 }

 

                 BDBRepository repository = mRepository.get();

diff --git a/src/main/java/com/amazon/carbonado/spi/LobEngine.java b/src/main/java/com/amazon/carbonado/spi/LobEngine.java
index e91bf59..a897cac 100644
--- a/src/main/java/com/amazon/carbonado/spi/LobEngine.java
+++ b/src/main/java/com/amazon/carbonado/spi/LobEngine.java
@@ -257,7 +257,7 @@ public class LobEngine {
                 throw new PersistNoneException("Lob deleted: " + this);

             }

 

-            OutputStream out = new Output(lob, 0, txn);

+            Output out = new Output(lob, 0, txn);

 

             byte[] buffer = new byte[lob.getBlockSize()];

 

@@ -271,12 +271,18 @@ public class LobEngine {
             } finally {

                 data.close();

             }

-            out.close();

 

             if (total < lob.getLength()) {

                 new BlobImpl(lob).setLength(total);

             }

 

+            // Note: Closing Output commits the transaction. No other resources

+            // are freed. This close is explicitly not put into a finally block

+            // in order for an exception to cause the transaction to rollback.

+            out.close();

+

+            // This isn't really needed due to closing Output, but it is good

+            // practice to include it anyhow.

             txn.commit();

         } catch (IOException e) {

             if (e.getCause() instanceof RepositoryException) {

diff --git a/src/main/java/com/amazon/carbonado/spi/RepairExecutor.java b/src/main/java/com/amazon/carbonado/spi/RepairExecutor.java
index adf2f34..0bc45ab 100644
--- a/src/main/java/com/amazon/carbonado/spi/RepairExecutor.java
+++ b/src/main/java/com/amazon/carbonado/spi/RepairExecutor.java
@@ -132,6 +132,7 @@ public class RepairExecutor {
         while (true) {

             synchronized (this) {

                 mIdle = true;

+                // Only one wait condition, so okay to not call notifyAll.

                 notify();

             }

             Runnable task = mQueue.poll(mKeepAliveSeconds, TimeUnit.SECONDS);

@@ -141,6 +142,7 @@ public class RepairExecutor {
                     return task;

                 }

                 if (mQueue.size() == 0) {

+                    // Only one wait condition, so okay to not call notifyAll.

                     notify();

                     mWorker = null;

                     return null;

diff --git a/src/main/java/com/amazon/carbonado/spi/SequenceValueGenerator.java b/src/main/java/com/amazon/carbonado/spi/SequenceValueGenerator.java
index 21b0676..efe8ebd 100644
--- a/src/main/java/com/amazon/carbonado/spi/SequenceValueGenerator.java
+++ b/src/main/java/com/amazon/carbonado/spi/SequenceValueGenerator.java
@@ -262,7 +262,7 @@ public class SequenceValueGenerator extends AbstractSequenceValueProducer {
             long next = mStoredSequence.getNextValue();

             long nextStored = next + mReserveAmount * mIncrement;

 

-            if (next >= 0 & nextStored < 0) {

+            if (next >= 0 && nextStored < 0) {

                 // Wrapped around. There might be just a few values left.

                 long avail = (Long.MAX_VALUE - next) / mIncrement;

                 if (avail > 0) {