Fix #124: Manage entropy bits in TimeBasedEpochGenerator

src/main/java/com/fasterxml/uuid/impl/TimeBasedEpochGenerator.java

@@ -3,8 +3,7 @@
 import java.security.SecureRandom;
 import java.util.Random;
 import java.util.UUID;
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
+import java.util.function.Consumer;
 
 import com.fasterxml.uuid.NoArgGenerator;
 import com.fasterxml.uuid.UUIDClock;
@@ -35,9 +34,9 @@ public class TimeBasedEpochGenerator extends NoArgGenerator
      */
 
     /**
-     * Random number generator that this generator uses.
+     * Random number generator that fills a byte array with entropy
      */
-    protected final Random _random;
+    protected final Consumer<byte[]> _randomNextBytes;
 
     /**
      * Underlying {@link UUIDClock} used for accessing current time, to use for
@@ -49,7 +48,6 @@ public class TimeBasedEpochGenerator extends NoArgGenerator
 
     private long _lastTimestamp = -1;
     private final byte[] _lastEntropy  = new byte[ENTROPY_BYTE_LENGTH];
-    private final Lock lock = new ReentrantLock();
 
     /*
     /**********************************************************************
@@ -76,10 +74,12 @@ public TimeBasedEpochGenerator(Random rnd) {
      */
     public TimeBasedEpochGenerator(Random rnd, UUIDClock clock)
     {
-        if (rnd == null) {
-            rnd = LazyRandom.sharedSecureRandom(); 
-        }
-        _random = rnd;
+        this((rnd == null ? LazyRandom.sharedSecureRandom() : rnd)::nextBytes, clock);
+    }
+
+    TimeBasedEpochGenerator(Consumer<byte[]> randomNextBytes, UUIDClock clock)
+    {
+        _randomNextBytes = randomNextBytes;
         _clock = clock;
     }
 
@@ -120,28 +120,39 @@ public UUID generate()
      */
     public UUID construct(long rawTimestamp)
     {
-        lock.lock();
-        try { 
+        final long mostSigBits, leastSigBits;
+        synchronized (_lastEntropy) {
             if (rawTimestamp == _lastTimestamp) {
-                boolean c = true;
-                for (int i = ENTROPY_BYTE_LENGTH - 1; i >= 0; i--) {
-                    if (c) {
-                        byte temp = _lastEntropy[i];
-                        temp = (byte) (temp + 0x01);
-                        c = _lastEntropy[i] == (byte) 0xff;
-                        _lastEntropy[i] = temp;
+                carry:
+                {
+                    for (int i = ENTROPY_BYTE_LENGTH - 1; i > 0; i--) {
+                        _lastEntropy[i] = (byte) (_lastEntropy[i] + 1);
+                        if (_lastEntropy[i] != 0x00) {
+                            break carry;
+                        }
+                    }
+                    _lastEntropy[0] = (byte) (_lastEntropy[0] + 1);
+                    if (_lastEntropy[0] >= 0x04) {
+                        throw new IllegalStateException("overflow on same millisecond");
                     }
-                }
-                if (c) {
-                    throw new IllegalStateException("overflow on same millisecond");
                 }
             } else {
                 _lastTimestamp = rawTimestamp;
-                _random.nextBytes(_lastEntropy);
+                _randomNextBytes.accept(_lastEntropy);
+                // In the most significant byte, only 2 bits will fit in the UUID, and one of those should be cleared
+                // to guard against overflow.
+                _lastEntropy[0] &= 0x01;
             }
-            return UUIDUtil.constructUUID(UUIDType.TIME_BASED_EPOCH, (rawTimestamp << 16) | _toShort(_lastEntropy, 0), _toLong(_lastEntropy, 2));
-        } finally {
-            lock.unlock();
+            mostSigBits = rawTimestamp << 16 |
+                    (long) UUIDType.TIME_BASED_EPOCH.raw() << 12 |
+                    Byte.toUnsignedLong(_lastEntropy[0]) << 10 |
+                    Byte.toUnsignedLong(_lastEntropy[1]) << 2 |
+                    Byte.toUnsignedLong(_lastEntropy[2]) >>> 6;
+            long right62Mask = (1L << 62) - 1;
+            long variant = 0x02;
+            leastSigBits = variant << 62 |
+                    _toLong(_lastEntropy, 2) & right62Mask;
         }
+        return new UUID(mostSigBits, leastSigBits);
     }
 }

src/test/java/com/fasterxml/uuid/impl/TimeBasedEpochGeneratorTest.java

@@ -0,0 +1,84 @@
+package com.fasterxml.uuid.impl;
+
+import com.fasterxml.uuid.UUIDClock;
+import junit.framework.TestCase;
+
+import java.math.BigInteger;
+import java.util.Arrays;
+import java.util.UUID;
+import java.util.function.Consumer;
+
+public class TimeBasedEpochGeneratorTest extends TestCase {
+
+    public void testFormat() {
+        BigInteger minEntropy = BigInteger.ZERO;
+        long minTimestamp = 0;
+        TimeBasedEpochGenerator generatorEmpty = new TimeBasedEpochGenerator(staticEntropy(minEntropy), staticClock(minTimestamp));
+        UUID uuidEmpty = generatorEmpty.generate();
+        assertEquals(0x07, uuidEmpty.version());
+        assertEquals(0x02, uuidEmpty.variant());
+        assertEquals(minTimestamp, getTimestamp(uuidEmpty));
+        assertEquals(minEntropy, getEntropy(uuidEmpty));
+
+        Consumer<byte[]> entropyFull = bytes -> Arrays.fill(bytes, (byte) 0xFF);
+        long maxTimestamp = rightBitmask(48);
+        TimeBasedEpochGenerator generatorFull = new TimeBasedEpochGenerator(entropyFull, staticClock(maxTimestamp));
+        UUID uuidFull = generatorFull.generate();
+        assertEquals(0x07, uuidFull.version());
+        assertEquals(0x02, uuidFull.variant());
+        assertEquals(maxTimestamp, getTimestamp(uuidFull));
+        assertEquals(BigInteger.ONE.shiftLeft(73).subtract(BigInteger.ONE), getEntropy(uuidFull));
+    }
+
+    public void testIncrement() {
+        TimeBasedEpochGenerator generator = new TimeBasedEpochGenerator(staticEntropy(BigInteger.ZERO), staticClock(0));
+        assertEquals(BigInteger.valueOf(0), getEntropy(generator.generate()));
+        assertEquals(BigInteger.valueOf(1), getEntropy(generator.generate()));
+        assertEquals(BigInteger.valueOf(2), getEntropy(generator.generate()));
+        assertEquals(BigInteger.valueOf(3), getEntropy(generator.generate()));
+    }
+
+    public void testCarryOnce() {
+        TimeBasedEpochGenerator generator = new TimeBasedEpochGenerator(staticEntropy(BigInteger.valueOf(0xFF)), staticClock(0));
+        assertEquals(BigInteger.valueOf(0xFF), getEntropy(generator.generate()));
+        assertEquals(BigInteger.valueOf(0x100), getEntropy(generator.generate()));
+    }
+
+    public void testCarryAll() {
+        BigInteger largeEntropy = BigInteger.ONE.shiftLeft(73).subtract(BigInteger.ONE);
+        TimeBasedEpochGenerator generator = new TimeBasedEpochGenerator(staticEntropy(largeEntropy), staticClock(0));
+        assertEquals(largeEntropy, getEntropy(generator.generate()));
+        assertEquals(BigInteger.ONE.shiftLeft(73), getEntropy(generator.generate()));
+    }
+
+    private long getTimestamp(UUID uuid) {
+        return uuid.getMostSignificantBits() >>> 16;
+    }
+
+    private BigInteger getEntropy(UUID uuid) {
+        return BigInteger.valueOf(uuid.getMostSignificantBits() & rightBitmask(12)).shiftLeft(62).or(
+                BigInteger.valueOf(uuid.getLeastSignificantBits() & rightBitmask(62)));
+    }
+
+    private Consumer<byte[]> staticEntropy(BigInteger entropy) {
+        byte[] entropyBytes = entropy.toByteArray();
+        return bytes -> {
+            int offset = bytes.length - entropyBytes.length;
+            Arrays.fill(bytes, 0, offset, (byte) 0x00);
+            System.arraycopy(entropyBytes, 0, bytes, offset, entropyBytes.length);
+        };
+    }
+
+    private UUIDClock staticClock(long timestamp) {
+        return new UUIDClock() {
+            @Override
+            public long currentTimeMillis() {
+                return timestamp;
+            }
+        };
+    }
+
+    private long rightBitmask(int bits) {
+        return (1L << bits) - 1;
+    }
+}