3

I am enclosing processing of list of objects in a synchronized block, but ended up in race condition. Am I using the synchronized block wrongly or is there any other problem with my code? The list is actually instantiated with mapdb.

private static List<MessageHolder> msgHolders;
//messageSequenceDB instantiation
msgHolders = (List<MessageHolder>) messageSequenceDB.indexTreeList("tempStorage", Serializer.JAVA).createOrOpen();


synchronized (msgHolders) {
        System.out.println(Thread.currentThread().getName() +"->"+msgHolders.toString());
        for (MessageHolder holder : msgHolders) {
            if(holder.getStatus().equalsIgnoreCase(STATUS_INITIAL) {
                holder.setStatus(STATUS_DISPATCHED);
                LOGGER.info("MESSAGE SEQUENCER: Message {} dispatched", holder);
                //Remaining code
}}}

I am expecting one object to be processed by only one thread. If the second thread tries to dispatch it, the condition on status should fail. But I am getting output as, few messages are processed by all threads.

Providing sample output here.

pool-12-thread-1->[MessageHolder [key=TradeId.1, message=1, status=Initial], MessageHolder [key=TradeId.2, message=222, status=Initial]]

pool-12-thread-2->[MessageHolder [key=TradeId.2, message=222, status=Initial], MessageHolder [key=TradeId.1, message=1111, status=Initial]]

pool-12-thread-3->[MessageHolder [key=TradeId.2, message=222, status=Initial]]

pool-12-thread-1->[MessageHolder [key=TradeId.1, message=11, status=Initial]]

The message with ID 222 gets processed by all the 3 threads. what should I do to ensure synchronization?

Andrew Tobilko
  • 48,120
  • 14
  • 91
  • 142
Anitha.R
  • 344
  • 2
  • 15
  • how `msgHolders` got initialised doesn't really matter; no one can enter the synchronized block until the thread that inquired the lock first does its work. It's very strange – Andrew Tobilko Dec 26 '18 at 12:28
  • what's in the remaining code? do you re-initialise `msgHolders` anywhere? – Andrew Tobilko Dec 26 '18 at 12:29
  • Are you sure that `messageSequenceDB.indexTreeList(..)` return the same object? I mean that object id (like `@6ef123`) is the same for all threads? Or did you try to create singleton object `private static final Object LOCKER = new Object();` and put it to sync block? – ZhenyaM Dec 26 '18 at 12:46
  • Please post the whole method which is being executed by several threads. I suspect that each thread gets it's own `magHolders` list so `synchronized` block doesn't actually works as intended. – Ivan Dec 27 '18 at 03:51
  • @Ivan Yes, you are correct. My suspect is also the same. Each thread is getting separate msgHolders. This is confirmed by the output. The method is complete. The 'Remaining code' section contains only a call to another method with the status-modified holder object. – Anitha.R Jan 02 '19 at 10:35

1 Answers1

0

I have solved this by making the field status as volatile.

Anitha.R
  • 344
  • 2
  • 15