mumrah/flake-java — pull request #2 by heymjo (2013-07-05 13:52:28+00:00)
Title: thread safety issue

Hi,

Moving the bytebuffer construction into the synchronized block seems to resolve the fact that the current implementation is not thread safe. Here is my unit test (uses guava and fest for convenience) :

```java
import com.google.common.collect.Sets;
import org.junit.Test;

import java.util.Set;

import static org.fest.assertions.Assertions.assertThat;

public class UniqueIdGeneratorTest {

private static final int NUMBER_OF_MESSAGES = 10;

// attempt to test the thread safety of the id generator
@Test
public void test0() throws InterruptedException {
Set one = Sets.newHashSetWithExpectedSize(NUMBER_OF_MESSAGES);
Set two = Sets.newHashSetWithExpectedSize(NUMBER_OF_MESSAGES);
Set three = Sets.newHashSetWithExpectedSize(NUMBER_OF_MESSAGES);
Set four = Sets.newHashSetWithExpectedSize(NUMBER_OF_MESSAGES);
Set five = Sets.newHashSetWithExpectedSize(NUMBER_OF_MESSAGES);
UniqueId generator = new UniqueId();

Thread t1 = new Thread(new ThreadedUniqueId(generator, one));
Thread t2 = new Thread(new ThreadedUniqueId(generator, two));
Thread t3 = new Thread(new ThreadedUniqueId(generator, three));
Thread t4 = new Thread(new ThreadedUniqueId(generator, four));
Thread t5 = new Thread(new ThreadedUniqueId(generator, five));

t1.start();
t2.start();
t3.start();
t4.start();
t5.start();

t1.join();
t2.join();
t3.join();
t4.join();
t5.join();

// poor mans comparison matrix
assertThat(Sets.intersection(one, two)).isEmpty();
assertThat(Sets.intersection(one, three)).isEmpty();
assertThat(Sets.intersection(one, four)).isEmpty();
assertThat(Sets.intersection(one, five)).isEmpty();
assertThat(Sets.intersection(two, three)).isEmpty();
assertThat(Sets.intersection(two, four)).isEmpty();
assertThat(Sets.intersection(two, five)).isEmpty();
assertThat(Sets.intersection(three, four)).isEmpty();
assertThat(Sets.intersection(three, five)).isEmpty();
assertThat(Sets.intersection(four, five)).isEmpty();

}

class ThreadedUniqueId implements Runnable {

private UniqueId generator;

private Set resultList;

public ThreadedUniqueId(UniqueId generator, Set resultList) {
this.resultList = resultList;
this.generator = generator;
}

@Override
public void run() {
long begin = System.currentTimeMillis();
for (int i = 0; i < NUMBER_OF_MESSAGES; i++) {
resultList.add(generator.getStringId());
}
System.out.println(Thread.currentThread().getName() + " generated " + NUMBER_OF_MESSAGES + " messages in "
+ (System.currentTimeMillis() - begin) + " ms");
}
}
}

```
This gives:

```java
java.lang.AssertionError: expecting empty, but was:<['0001373032211297-bc305baaf12-0003', '0001373032211297-bc305baaf12-0007', '0001373032211297-bc305baaf12-0005', '0001373032211299-bc305baaf12-0002']>
at org.fest.assertions.Fail.failure(Fail.java:228)
at org.fest.assertions.Assert.failure(Assert.java:149)
at org.fest.assertions.Assert.fail(Assert.java:140)
at org.fest.assertions.GroupAssert.isEmpty(GroupAssert.java:59)
at UniqueIdGeneratorTest.test0(UniqueIdGeneratorTest.java:48)
```