threadsafe Hash (#597)

(1) Made Hash thread-safe without needing full synchronization.

All access to `bytes`, `trits` and `hashcode` done via a null check against an inner class object that is instantiated in a synchronized block with a double check. `ByteSafe` and `TritSafe` are inner classes and the data hidden in them is not available until the classes are fully instantiated (classloading ensures this).

(2)  The (1) above fixes the race condition to `hashcode()` and `equals()` and `bytes()` that was proven to be happening and might also have cause the FIFO cache overrun.

(3) Fixed `compareTo` to do an identity check and not need to make a new `Hash` every time - because Hash extends Indexable and the the indexible provided could already be a Hash.

(4) Commented on bounds check. This is easy to fix but so many tests are abusing it and they won't pass until the tests are also fixed.

(5) Added a null check in `fullread' because the RocksDB does provide null byte[] to read methods sometimes and they can be safely passed over.

(6) Added a 2nd time `read` persistence check. The class should be effectively immutable (not possible) but at least check to see if it has already been initialized and reject a second initialization.

(7) No other classes extend `Hash` . I have made it `final` and that can be changed in the future if it ever needs to be extended.

All tests pass.
Live run passed.

Signed-off-by: footloosejava <[email protected]>

src/main/java/com/iota/iri/model/Hash.java

@@ -8,51 +8,39 @@
 
 import java.io.Serializable;
 import java.util.Arrays;
+import java.util.Objects;
 
-public class Hash implements Serializable, Indexable {
+public final class Hash implements Serializable, Indexable {
 
     public static final int SIZE_IN_TRITS = 243;
     public static final int SIZE_IN_BYTES = 49;
 
     public static final Hash NULL_HASH = new Hash(new int[Curl.HASH_LENGTH]);
 
-    private byte[] bytes;
-    private int[] trits;
-    private int hashCode;
-
-    // constructors' bill
+    private final Object lock = new Object();
+    private ByteSafe byteSafe;
+    private TritSafe tritSafe;
 
-    public Hash(final byte[] bytes, final int offset, final int size) {
-        fullRead(bytes, offset, size);
-    }
-
-    public Hash(){}
+    private final class ByteSafe {
+        private final byte[] bytes;
+        private final Integer hashcode;
 
-    public Hash(final byte[] bytes) {
-        this(bytes, 0, SIZE_IN_BYTES);
+        private ByteSafe(byte[] bytes) {
+            Objects.requireNonNull(bytes, "ByteSafe is attempted to be initialized with a null byte array");
+            this.bytes = bytes;
+            this.hashcode = Arrays.hashCode(bytes);
+        }
     }
 
-    public Hash(final int[] trits, final int offset) {
-        this.trits = new int[SIZE_IN_TRITS];
-        System.arraycopy(trits, offset, this.trits, 0, SIZE_IN_TRITS);
-        //this(Converter.bytes(trits, offset, trits.length));
-    }
+    private final class TritSafe {
+        private final int[] trits;
 
-    public Hash(final int[] trits) {
-        this(trits, 0);
+        private TritSafe(int[] trits) {
+            this.trits = Objects.requireNonNull(trits, "TritSafe is attempted to be initialized with a null int array");
+        }
     }
 
-    public Hash(final String trytes) {
-        this.trits = new int[SIZE_IN_TRITS];
-        Converter.trits(trytes, this.trits, 0);
-    }
 
-    //
-    /*
-    public static Hash calculate(byte[] bytes) {
-        return calculate(bytes, SIZE_IN_TRITS, new Curl());
-    }
-    */
     public static Hash calculate(SpongeFactory.Mode mode, int[] trits) {
         return calculate(trits, 0, trits.length, SpongeFactory.create(mode));
     }
@@ -62,6 +50,7 @@ public static Hash calculate(byte[] bytes, int tritsLength, final Sponge curl) {
         Converter.getTrits(bytes, trits);
         return calculate(trits, 0, tritsLength, curl);
     }
+
     public static Hash calculate(final int[] tritsToCalculate, int offset, int length, final Sponge curl) {
         int[] hashTrits = new int[SIZE_IN_TRITS];
         curl.reset();
@@ -70,24 +59,74 @@ public static Hash calculate(final int[] tritsToCalculate, int offset, int lengt
         return new Hash(hashTrits);
     }
 
+
+    public Hash() {
+    }
+
+
+    public Hash(final byte[] source, final int sourceOffset, final int sourceSize) {
+        byte[] dest = new byte[SIZE_IN_BYTES];
+        System.arraycopy(source, sourceOffset, dest, 0, sourceSize - sourceOffset > source.length ? source.length - sourceOffset : sourceSize);
+        this.byteSafe = new ByteSafe(dest);
+    }
+
+    public Hash(final byte[] bytes) {
+        this(bytes, 0, SIZE_IN_BYTES);
+    }
+
+    public Hash(final int[] trits, final int offset) {
+        int[] dest = new int[SIZE_IN_TRITS];
+        System.arraycopy(trits, offset, dest, 0, dest.length);
+        this.tritSafe = new TritSafe(dest);
+    }
+
+    public Hash(final int[] trits) {
+        this(trits, 0);
+    }
+
+    public Hash(final String trytes) {
+        this.tritSafe = new TritSafe(new int[SIZE_IN_TRITS]);
+        Converter.trits(trytes, this.tritSafe.trits, 0);
+    }
+
+    private void fullRead(byte[] src) {
+        if (src != null) {
+            synchronized (lock) {
+                if (byteSafe != null || tritSafe != null) {
+                    throw new IllegalStateException("I cannot be initialized with data twice.");
+                }
+                byte[] dest = new byte[SIZE_IN_BYTES];
+                System.arraycopy(src, 0, dest, 0, Math.min(dest.length, src.length));
+                byteSafe = new ByteSafe(dest);
+            }
+        }
+    }
+
     public int trailingZeros() {
-        int index, zeros;
-        final int[] trits;
-        index = SIZE_IN_TRITS;
-        zeros = 0;
-        trits = trits();
-        while(index-- > 0 && trits[index] == 0) {
+        final int[] trits = trits();
+        int index = SIZE_IN_TRITS;
+        int zeros = 0;
+        while (index-- > 0 && trits[index] == 0) {
             zeros++;
         }
         return zeros;
     }
 
     public int[] trits() {
-        if(trits == null) {
-            trits = new int[Curl.HASH_LENGTH];
-            Converter.getTrits(bytes, trits);
+        TritSafe safe = tritSafe;
+        if (safe == null) {
+            synchronized (lock) {
+                if (tritSafe == null) {
+                    Objects.requireNonNull(byteSafe, "I need my bytes to be initialized in order to construct trits.");
+                    byte[] src = bytes();
+                    int[] dest = new int[Curl.HASH_LENGTH];
+                    Converter.getTrits(src, dest);
+                    tritSafe = new TritSafe(dest);
+                }
+                safe = tritSafe;
+            }
         }
-        return trits;
+        return safe.trits;
     }
 
     @Override
@@ -104,35 +143,36 @@ public boolean equals(Object o) {
 
     @Override
     public int hashCode() {
-        if(bytes == null) {
-            bytes();
-        }
-        return hashCode;
+        bytes();
+        return byteSafe.hashcode;
     }
 
     @Override
     public String toString() {
         return Converter.trytes(trits());
     }
-    
+
     public byte[] bytes() {
-        if(bytes == null) {
-            bytes = new byte[SIZE_IN_BYTES];
-            Converter.bytes(trits, 0, bytes, 0, trits.length);
-            hashCode = Arrays.hashCode(this.bytes);
+        ByteSafe safe = byteSafe;
+        if (safe == null) {
+            synchronized (lock) {
+                if (byteSafe == null) {
+                    Objects.requireNonNull(tritSafe, "I need my trits to be initialized in order to construct bytes.");
+                    int[] src = trits();
+                    byte[] dest = new byte[SIZE_IN_BYTES];
+                    Converter.bytes(src, 0, dest, 0, src.length);
+                    byteSafe = new ByteSafe(dest);
+                }
+                safe = byteSafe;
+            }
         }
-        return bytes;
+        return safe.bytes;
     }
 
-    private void fullRead(byte[] bytes, int offset, int size) {
-        this.bytes = new byte[SIZE_IN_BYTES];
-        System.arraycopy(bytes, offset, this.bytes, 0, size - offset > bytes.length ? bytes.length-offset: size);
-        hashCode = Arrays.hashCode(this.bytes);
-    }
 
     @Override
-    public void read(byte[] bytes) {
-        fullRead(bytes, 0, SIZE_IN_BYTES);
+    public void read(byte[] src) {
+        fullRead(src);
     }
 
     @Override
@@ -147,7 +187,7 @@ public Indexable decremented() {
 
     @Override
     public int compareTo(Indexable indexable) {
-        Hash hash = new Hash(indexable.bytes());
+        Hash hash = (indexable instanceof Hash) ? (Hash) indexable : new Hash(indexable.bytes());
         if (this.equals(hash)) {
             return 0;
         }
@@ -157,5 +197,4 @@ public int compareTo(Indexable indexable) {
         }
         return (int) diff;
     }
-}
-
+}