CODEWITHSUNDEEP
javamultithreadinglockingwait
Sujith R
Sujith R

Reputation: 11

Java multithreading - locking without waiting

I have a java multithreading issue. I have 2 threads accessing a methodA() which has a for loop inside and in the loop calls a methodB(). The method A should be locked using a thread-name lock and the method B should be locked on the object id that the method B operates on. Check the below code.

Currrent code

        private static final ConcurrentHashMap<Object, Object> LOCKS = new ConcurrentHashMap<Object, Object>();   
        private void methodA(){
         LOCKS.putIfAbsent(Thread.currentThread().getName(), new Object()))  
         synchronized (LOCKS.putIfAbsent(Thread.currentThread().getName(), new Object()))        {                
               for(loop through all objects) {
                       methodB(Object1);
               }
         }
        }

    private void methodB(Object1 object1) {      
      LOCKS.putIfAbsent(object1.getObjectId(), new Object()))    
      synchronized(LOCKS.putIfAbsent(object1.getObjectId(), new Object())){         
         //<Work on object1>
      }   
    }

I had done the above code to ensure that 2 different threads should be able to paralleley access methodA(), but should not work on same Object1 at a time in methodB() (which is invoked by methodA()). ie; Though I want Thread A and Thread B to access methodA() at the same time, which in turn will loop through all objects in 'for' loop and will operate on each by calling methodB(), I dont want thread A and B to act on the SAME object instance at a time. Hence the above code to lock methodB() based on object instance ID.

The improvement needed.

In the above code if Thread A and Thread B comes to methodB() and finds that they both want to work over the same object 'obj1', right now with the above code either Thread A will wait or Thread B will wait for the other one to finish depending on who reached and locked methodB() first.

But imagine a case, where Thread A getting the lock first and executing methodB() takes 9 hours to finish processing 'obj1'. Thread B in this case will need to wait for the entire 9 hours before getting a chance to execute methodB() and thereby processing 'obj1'.

I don't want this to happen. Thread B, once it finds that methodB() is locked in the name of 'obj1' by Thread A should move on (and come back to obj1 later) to try locking and processing other objects. ie; it should try processing other objects in the 'for' loop like obj1, obj2 etc in the list of objects.

Any inputs to solve this 'lock without wait' problem will be appreciated.

Many thanks in advance for any help.

Some clarifications to improve the answers.

  1. Both methodA() and methodB() is in same class. The methodB() is not in the Object class.
  2. Actually Thread A and Thread B are timer threads that calls many methods including A and B. Hence the thread level lock (because thread gets called every 15 mins or so and there is a chance that the first execution of methodA() wont be completed before a second call to it).
  3. methodB(Obj1) always takes a Object1 param and must be locked on it. The reason is, in this class there are be other methods say methodC(Obj1) and methodD(Obj1) that also takes in an Object1 param. These methods should not get executed at the same time for same instance of Object1. Hence the lock on Object1 param needed.
  4. The Thread B finding that methodB(Obj1 obj) is already locked by Thread A() on obj1 needs to somehow call methodB() again but with a different object, say obj2. It should come back to obj1 once it finishes with the others.

Upvotes: 1

Views: 1418

Answers (4)

Peter Lawrey
Peter Lawrey

Reputation: 533472

The best thing you can do is to keep things simple.

The method A should be locked using a thread-name lock

Only locking shared objects makes sense. Lock a thread local lock is pointless.

synchronized(LOCKS.putIfAbsent(object1.getObjectId(), new Object()))

That will return null and throw a NullPointerException the first time it is run.

I would replace the code with

private void methodA(){  
    List<Object1> objects = new ArrayList<>(this.objectList);
    while(true) {
       for(Iterator<Object1> iter = objects.iterator() : objects)
          if(object1.methodB())
             iter.remove();
       if(objects.isEmpty()) break;
       Thread.sleep(WAIT_TIME_BEFORE_TRYING_AGAIN);
    }
}

// in class for Object1
final Lock lock = new ReentrantLock();

public boolean methodB() {          
    if (!lock.tryLock()) 
        return false;
    try {
       // work on this
       return true;
    } finally {
       lock.unlock();
    }
}

Depending on how you want to handle object which cannot be locked, you could add them to a background ExecutorService. You could have methodA repeatedly call all the remaining objects for which this fails.

Ideally you would find a way to minimise the time locked or even remove the need for a lock entirely. e.g. Classes like AtomicReference and CopyOnWriteArrayList are thread safe and lockless.

Upvotes: 5

Aaron Digulla
Aaron Digulla

Reputation: 328556

I suggest a different approach. Instead of directly invoking the method, put a command object in a queue and have a thread (or an executor) process the commands.

When a command appears in the queue, try to get the lock. If that doesn't work, add the command to the end of the queue. This will make sure the command is tried again eventually.

Drawback: A command could be postponed indefinitely if some thread happens to have lock on it every time it's tried for execution.

The solution here would be to make sure you only lock what you need. That way, when you see "oh, this is locked", you know that someone is already working on the task and you can simply forget about the command (-> don't do work twice).

Upvotes: 0

Martin James
Martin James

Reputation: 24847

Does it matter if one thread pass occasionally 'misses' an object on a pass-through? If not:

Store all the objects in a lockable queue-style container. Have threads A, B etc. pop one out, call methods on it and then push it back in. It is then not possible for the threads to operate on the same object at the same time. The only lock is then on the container push/pop and no thread needs to block for any extended time.

..or something like that. I always try to avoid complex locking schemes - they always seem to screw up :(

Upvotes: 0

Werner Henze
Werner Henze

Reputation: 16726

I am not a Java guy, but in my opinion you will not achieve this with synchronized. I believe you will need to do your own locking.

If you create a java.util.concurrent.locks.ReentrantLock you can use tryLock to enter the lock if it is not yet locked. methodA needs to know which methodB call was successful or which was canceled because the lock was not possible. So you could do the lock handling in methodA, giving you full control there. Or you could do the locking in methodB, but then you need some return value or exception handling to signal back to methodA if methodB did its work or if it did not get the lock.

Of course you will also need to keep a list in methodA of the objects you already went through or of the objects you still need to work on.

Upvotes: 1

Related Questions