Skip to content

Commit

Permalink
fixed bottleneck in getHash(); fixed NPEs when destoyed FCQueue is ac…
Browse files Browse the repository at this point in the history
…cessed

Signed-off-by: Oleg Mazurov <oleg.mazurov@swirldslabs.com>
  • Loading branch information
OlegMazurov committed May 24, 2023
1 parent 610f33a commit 3c78aed
Showing 1 changed file with 20 additions and 10 deletions.
Expand Up @@ -88,6 +88,9 @@ private static class ClassVersion {

private static final long HASH_RADIX = 3;

/** A hash value representing a null element or a destroyed queue */
private static final ImmutableHash NULL_HASH = new ImmutableHash(new byte[DIGEST_TYPE.digestLength()]);

/** the number of elements in this queue */
private int size;

Expand All @@ -101,7 +104,7 @@ private static class ClassVersion {
private final AtomicReference<Node<E>> unhashed;

/** the hash of this queue once it becomes immutable */
private ImmutableHash hash;
private volatile ImmutableHash hash;

static class Node<E extends FastCopyable> {
/** the element in the list */
Expand Down Expand Up @@ -140,15 +143,21 @@ public FCQueue() {
* {@inheritDoc}
*/
@Override
public synchronized Hash getHash() {
public Hash getHash() {
if (hash != null) {
return hash;
}
final ImmutableHash result = new ImmutableHash(computeHash());
if (isImmutable()) {
hash = result;

synchronized (this) {
ImmutableHash result = hash;
if (result == null) {
result = computeHash();
if (isImmutable()) {
hash = result;
}
}
return result;
}
return result;
}

/**
Expand Down Expand Up @@ -183,7 +192,7 @@ public synchronized Hash getHash() {
* shared data structure are invariant. Volatile <code>runningHash</code> helps to reduce overlap between
* threads.</p>
*/
private byte[] computeHash() {
private ImmutableHash computeHash() {
// Ensure we have tail's running hash
if (tail.runningHash == null) {
Node<E> node = unhashed.get();
Expand All @@ -210,7 +219,7 @@ private byte[] computeHash() {
for (int i = 0; i < headHash.length; ++i) {
longToByteArray(tailHash[i] - headHash[i] * exponent, result, i * Long.BYTES);
}
return result;
return new ImmutableHash(result);
}

/**
Expand Down Expand Up @@ -418,9 +427,10 @@ public synchronized FCQueue<E> copy() {

@Override
protected synchronized void destroyNode() {
setImmutable(true);
head = tail = null;
size = 0;
hash = null;
hash = NULL_HASH;
}

//////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -791,7 +801,7 @@ private void deserializeV3(final SerializableDataInputStream dis) throws IOExcep
byte[] getHash(final E element) {
// Handle cases where list methods return null if the list is empty
if (element == null) {
return new byte[DIGEST_TYPE.digestLength()];
return NULL_HASH.getValue();
}
final Cryptography crypto = CryptographyHolder.get();
// return a hash of a hash, in order to make state proofs smaller in the future
Expand Down

0 comments on commit 3c78aed

Please sign in to comment.