CODEWITHSUNDEEP
javamultithreadingthread-synchronization
dushkin
dushkin

Reputation: 2101

Java: Fail in synchronizing threads

I have the following code:

for (int iThreadCounter = 1; iThreadCounter <= CONNECTIONS_NUM; iThreadCounter++){ 
  WorkThread wt = new WorkThread(iThreadCounter);
  new Thread(wt).start(); 
  m_arrWorkThreadsToCreate.add(wt); 
}

Those threads calls the following code:

int res = m_spLegJoin.call(m_workTread, m_workTread.getConfId());

And this is the call method inside LegJoinSp class:

public class LegJoinSp extends ConnEventSp {

    private static final int _LEG_JOIN_ACTION_CODE = 22;
    private static int m_nLegId = Integer.valueOf(IniUtils.getIniValue("General", "LEG_ID_START"));
    private final Lock m_lock = new ReentrantLock();

    public int call(WorkThread a_workThread, String a_sConfId) {

        synchronized (this) {

            //m_lock.lock();
            m_nLegId++;

            boolean bPass = false;

            Log4jWrapper.writeLog(LogLevelEnum.DEBUG, "LegJoinSp - call", "a_workThread = " + a_workThread.getThreadId() + " a_sConfId = " + a_sConfId);

            if (super.call(a_workThread, a_sConfId, _LEG_JOIN_ACTION_CODE, "" + m_nLegId) == 0) {

                bPass = true;

            } else {
                bPass = false;
            }

            //m_lock.unlock();

            if (bPass) {

                Log4jWrapper.writeLog(LogLevelEnum.DEBUG, "LegJoinSp - call", "a_workThread = " + a_workThread.getThreadId() + " a_sConfId = " + a_sConfId + " returned leg id " + m_nLegId);

                return m_nLegId;
            } else {

                return -1;
            }
        }

    }

    public Lock getLock() {
        return m_lock;
    }

}

I've got 2 threads calling this call() method. m_nLegId is initiated with 100. As you can see I have tried to lock the method with both

synchronized(this)

and

m_lock.lock() and m_lock.unlock()

The problem is that when I first get to if (bPass) inner code, it write 102 to my log as the m_nLegId value. However I expect it to be 101 because of the m_nLegId++; statement. It seems that the second thread manage to get inside the code before the synchronize block ends for the first thread execution.

How can I fix that?

Thank you

Upvotes: 1

Views: 150

Answers (2)

Nicolas Filotto
Nicolas Filotto

Reputation: 44965

For me your bug is related to the fact that m_nLegId is a static field and you try to synchronize access on the current instance instead of the class such that you don't properly prevent concurrent modifications of your field.

I mean

synchronized (this) {

Should rather be

synchronized (LegJoinSp.class) {

NB: In case you only need a counter, consider using an AtomicInteger for your field instead of an int.

Upvotes: 2

zstring
zstring

Reputation: 187

The thing is you are creating a new object with every thread, but the way you applied the lock is applicable only to same object (as you applied the lock on the this).
So if you want to apply the lock on the class level, then you can create a static object and apply the lock on that object which can serve the purpose you wanted to achieve (if I understood your problem correctly based on the comments)

Upvotes: 1

Related Questions