From 0947834ac950fb4dd59b719cbdddc1726e68674b Mon Sep 17 00:00:00 2001 From: "Brian S. O'Neill" Date: Thu, 11 Jan 2007 23:19:44 +0000 Subject: Fixes for bugs found using FindBugs. --- .../amazon/carbonado/CorruptEncodingException.java | 2 +- .../amazon/carbonado/OptimisticLockException.java | 2 +- .../com/amazon/carbonado/adapter/TextAdapter.java | 7 ++- .../com/amazon/carbonado/cursor/WorkFilePool.java | 1 + .../amazon/carbonado/filter/BinaryOpFilter.java | 2 +- .../amazon/carbonado/filter/PropertyFilter.java | 6 ++- .../com/amazon/carbonado/info/StorableIndex.java | 2 +- .../carbonado/info/StorableIntrospector.java | 6 +++ .../com/amazon/carbonado/lob/AbstractBlob.java | 58 +++++++++++++--------- .../com/amazon/carbonado/lob/AbstractClob.java | 34 +++++++------ .../amazon/carbonado/qe/IndexedQueryAnalyzer.java | 2 +- .../amazon/carbonado/qe/UnionQueryAnalyzer.java | 2 +- .../java/com/amazon/carbonado/raw/RawCursor.java | 4 +- .../carbonado/repo/indexed/IndexedRepository.java | 4 +- .../repo/replicated/ReplicatedRepository.java | 6 ++- .../carbonado/repo/sleepycat/BDBRepository.java | 12 ++--- .../java/com/amazon/carbonado/spi/LobEngine.java | 10 +++- .../com/amazon/carbonado/spi/RepairExecutor.java | 2 + .../carbonado/spi/SequenceValueGenerator.java | 2 +- 19 files changed, 104 insertions(+), 60 deletions(-) (limited to 'src/main/java/com/amazon/carbonado') 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 extends Filter { BinaryOpFilter(Filter left, Filter 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 extends Filter { */ public PropertyFilter 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 implements Appender { int size = properties.size(); if (size == 0 || size != directions.size()) { - new IllegalArgumentException("No properties specified"); + throw new IllegalArgumentException("No properties specified"); } StorableIndex index = new StorableIndex 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 { // 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 implements QueryExecutorFact OrderingScore score = result.getCompositeScore().getOrderingScore(); OrderingList handled = score.getHandledOrdering(); for (OrderedProperty 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 extends AbstractCursor { 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 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 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 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 } 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 } finally { synchronized (this) { mInProgress = false; + // Only wait condition is mInProgress, so okay to not call notifyAll. notify(); } } @@ -802,12 +804,10 @@ abstract class BDBRepository 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) { -- cgit v1.2.3