From 0947834ac950fb4dd59b719cbdddc1726e68674b Mon Sep 17 00:00:00 2001
From: "Brian S. O'Neill" <bronee@gmail.com>
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')
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) {
--
cgit v1.2.3