1

I have a Spring service, and I want to test its correctness for concurrent thread.

@RunWith(SpringJUnit4ClassRunner.class)
@SpringBootTest
public class SerialNumberServiceTest {

@Autowired
private SerialNumberService service;

private final static int NUM_THREADS = 10;
private final static int NUM_ITERATIONS = 100;

@Test
public void testSynchronized(){
    ExecutorService executor = Executors.newFixedThreadPool(10);
    final Set<String> result = new HashSet<String>();
    int size = 0;

    for(int i = 0; i < NUM_THREADS; i++){
        executor.submit(new Runnable() {
            @Override
            public void run() {
                for (int j = 0; j < NUM_ITERATIONS; j++) {
                    String code = service.generateSerialNumberByModelCode("LP");
                    result.add(code);
                }
            }
        });
    }

    assertEquals(NUM_ITERATIONS * NUM_THREADS, result.size());

}
}

I want assert how many codes was generated and check whether there is duplicate code. But the result Set is empty. I do not know why.

On the other hand, if I try to update values of the service instance in the threads, I also find the values are not saved. e.g.

   public void run() {
            for (int j = 0; j < NUM_ITERATIONS; j++) {
                String code = service.generateSerialNumberByModelCode("LP");
                service.incrValue();
                result.add(code);
            }
   }

And at last, service.getValue() is non-changed.

Anyone can explain this for me?

BurnetZhong
  • 438
  • 1
  • 5
  • 14

2 Answers2

3

There is a lot wrong this code.

1) You're using a HashSet and updating it in multiple threads but HashSet is documented as not thread-safe. This is gonna result in unexpected behaviour or if you're lucky a ConcurrentModificationException. Wrap it in a synchronized set.

2) You are not waiting for the submitted threads to be completed before doing your assert. Either use invokeAll to start multiple threads, or wait for all threads to complete by doing shutdown on the executor and then awaitTermination. For a better explanation, see this question: ExecutorService, how to wait for all tasks to finish

Community
  • 1
  • 1
john16384
  • 7,800
  • 2
  • 30
  • 44
  • Thanks @john16384, I changed Set to ConcurrentHashMap, and add executor.awaitTermination(5, TimeUnit.SECONDS). Is this OK? – BurnetZhong Apr 14 '17 at 08:47
  • That has a lot higher chance of doing what you expect. Test it some more and see if it now works as you expect. Don't forget to also call `shutdown` before `awaitTermination`. – john16384 Apr 14 '17 at 08:51
3

Is there any case that "code" output string conflicts with a string already added to hashset? In this case, code string just overwritten on hashset. So, I'm trying to say that maybe each code string value in the hashset should be unique. You may also reduce iteration number for better debugging...

Akiner Alkan
  • 6,145
  • 3
  • 32
  • 68
Alican Beydemir
  • 343
  • 2
  • 13