SONAR-8332 fix broken thread safety of UUID generation

diff --git a/sonar-core/src/main/java/org/sonar/core/util/UuidGeneratorImpl.java b/sonar-core/src/main/java/org/sonar/core/util/UuidGeneratorImpl.java
index f76adf7e0ccaa76268dd6f648006f5e31f5d6ece..2b3b13d7f92dc62684c5f78f243055a9e97a2568 100644 (file)
--- a/sonar-core/src/main/java/org/sonar/core/util/UuidGeneratorImpl.java
+++ b/sonar-core/src/main/java/org/sonar/core/util/UuidGeneratorImpl.java
@@ -38,7 +38,6 @@ public final class UuidGeneratorImpl implements UuidGenerator {
   }
 
   private static class UuidGeneratorBase {
-    private final byte[] buffer = new byte[15];
     // We only use bottom 3 bytes for the sequence number. Paranoia: init with random int so that if JVM/OS/machine goes down, clock slips
     // backwards, and JVM comes back up, we are less likely to be on the same sequenceNumber at the same time:
     private final AtomicInteger sequenceNumber = new AtomicInteger(new SecureRandom().nextInt());
@@ -46,7 +45,7 @@ public final class UuidGeneratorImpl implements UuidGenerator {
     // Used to ensure clock moves forward
     private long lastTimestamp = 0L;
 
-    void initBase(int sequenceId) {
+    void initBase(byte[] buffer, int sequenceId) {
       long timestamp = System.currentTimeMillis();
 
       synchronized (this) {
@@ -70,7 +69,7 @@ public final class UuidGeneratorImpl implements UuidGenerator {
       System.arraycopy(secureMungedAddress, 0, buffer, 6, secureMungedAddress.length);
     }
 
-    protected byte[] generate(int increment) {
+    protected byte[] generate(byte[] buffer, int increment) {
       // Sequence number adds 3 bytes
       putLong(buffer, increment, 12, 3);
 
@@ -93,21 +92,26 @@ public final class UuidGeneratorImpl implements UuidGenerator {
 
     @Override
     public byte[] get() {
+      byte[] buffer = new byte[15];
       int sequenceId = getSequenceId();
-      initBase(sequenceId);
-      return super.generate(sequenceId);
+      initBase(buffer, sequenceId);
+      return super.generate(buffer, sequenceId);
     }
   }
 
   private static class FixedBasedUuidGenerator extends UuidGeneratorBase implements WithFixedBase {
+    private final byte[] base = new byte[15];
+
     FixedBasedUuidGenerator() {
       int sequenceId = getSequenceId();
-      initBase(sequenceId);
+      initBase(base, sequenceId);
     }
 
     @Override
     public byte[] generate(int increment) {
-      return super.generate(increment);
+      byte[] buffer = new byte[15];
+      System.arraycopy(base, 0, buffer, 0, buffer.length);
+      return super.generate(buffer, increment);
     }
   }
 }

diff --git a/sonar-core/src/test/java/org/sonar/core/util/UuidGeneratorImplTest.java b/sonar-core/src/test/java/org/sonar/core/util/UuidGeneratorImplTest.java
index 21200b0aa4096e44c78b9628191f3d5ab7264709..175aec720c689d28773b62faa1f09a11633def11 100644 (file)
--- a/sonar-core/src/test/java/org/sonar/core/util/UuidGeneratorImplTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/util/UuidGeneratorImplTest.java
@@ -19,10 +19,13 @@
  */
 package org.sonar.core.util;
 
+import java.util.ArrayList;
 import java.util.Base64;
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
 import org.junit.Test;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -31,7 +34,7 @@ public class UuidGeneratorImplTest {
   private UuidGeneratorImpl underTest = new UuidGeneratorImpl();
 
   @Test
-  public void generate_returns_unique_values_without_common_initial_letter_given_more_than_one_milisecond_between_generate_calls() throws InterruptedException {
+  public void generate_returns_unique_values_without_common_initial_letter_given_more_than_one_millisecond_between_generate_calls() throws InterruptedException {
     Base64.Encoder encoder = Base64.getEncoder();
     int count = 30;
     Set<String> uuids = new HashSet<>(count);
@@ -49,6 +52,33 @@ public class UuidGeneratorImplTest {
     }
   }
 
+  @Test
+  public void generate_concurrent_test() throws InterruptedException {
+    int rounds = 500;
+    List<byte[]> uuids1 = new ArrayList<>(rounds);
+    List<byte[]> uuids2 = new ArrayList<>(rounds);
+    Thread t1 = new Thread(() -> {
+      for (int i = 0; i < rounds; i++) {
+        uuids1.add(underTest.generate());
+      }
+    });
+    Thread t2 = new Thread(() -> {
+      for (int i = 0; i < rounds; i++) {
+        uuids2.add(underTest.generate());
+      }
+    });
+    t1.start();
+    t2.start();
+    t1.join();
+    t2.join();
+
+    Base64.Encoder encoder = Base64.getEncoder();
+    Set<String> uuids = new HashSet<>(rounds * 2);
+    uuids1.forEach(bytes -> uuids.add(encoder.encodeToString(bytes)));
+    uuids2.forEach(bytes -> uuids.add(encoder.encodeToString(bytes)));
+    assertThat(uuids).hasSize(rounds * 2);
+  }
+
   @Test
   public void generate_from_FixedBase_returns_unique_values_where_only_last_4_later_letter_change() {
     Base64.Encoder encoder = Base64.getEncoder();
@@ -68,4 +98,33 @@ public class UuidGeneratorImplTest {
       assertThat(iterator.next()).startsWith(base);
     }
   }
+
+  @Test
+  public void generate_from_FixedBase_concurrent_test() throws InterruptedException {
+    UuidGenerator.WithFixedBase withFixedBase = underTest.withFixedBase();
+    int rounds = 500;
+    List<byte[]> uuids1 = new ArrayList<>(rounds);
+    List<byte[]> uuids2 = new ArrayList<>(rounds);
+    AtomicInteger cnt = new AtomicInteger();
+    Thread t1 = new Thread(() -> {
+      for (int i = 0; i < rounds; i++) {
+        uuids1.add(withFixedBase.generate(cnt.getAndIncrement()));
+      }
+    });
+    Thread t2 = new Thread(() -> {
+      for (int i = 0; i < rounds; i++) {
+        uuids2.add(withFixedBase.generate(cnt.getAndIncrement()));
+      }
+    });
+    t1.start();
+    t2.start();
+    t1.join();
+    t2.join();
+
+    Base64.Encoder encoder = Base64.getEncoder();
+    Set<String> uuids = new HashSet<>(rounds * 2);
+    uuids1.forEach(bytes -> uuids.add(encoder.encodeToString(bytes)));
+    uuids2.forEach(bytes -> uuids.add(encoder.encodeToString(bytes)));
+    assertThat(uuids).hasSize(rounds * 2);
+  }
 }