0

I have a requirement which is as below :

1) A class which has all static methods and a static list. This list stores some objects on which I perform some operation.

2) Now this operation is called from multiple threads.

3) this operation call is not sharing any common data so this methods is not synchronized.

4) now whenever this list gets updated with new objects, I have to stop this operation call.

class StaticClass{
    static List<SomeObjects>=new List<>();
    static performOperation()
    {
        //call operation on all objects in the list
    }

    static updateList()
    {
        //update list, add, update or remove objects 
    }
}

Possible solutions :

1) Make performOperation() and updateList() synchronized. But the frequency at which performOperation() gets called is too high and udpate list frequency is too low.

2) use read write locks. Use read locks in performOperation() and write locks in updateList(). Sample is shown below :

class StaticClass{
    static List<SomeObjects>=new List<>();
    static final ReadWriteLock readWriteLock = new ReentrantReadWriteLock();

    static performOperation()
    {
        readWriteLock.readLock().lock();
        //call operation on all objects in the list
        readWriteLock.readLock().unlock();
    }

    static updateList()
    {
        readWriteLock.writeLock().lock();
        //update list, add, update or remove objects 
        readWriteLock.writeLock().unlock();
    }

So which solution is better? Is this a correct usage of readwrite locks. Why I am confused in going with approach 2 is there is not such data in performOperation() which needs read access or write access. I just cannot call this method when list is being updated. So I am not sure whether its a appropriate usage of read write locks or not.

Onki
  • 1,879
  • 6
  • 38
  • 58
  • I don't understand your reasoning for not using `synchronized`. Your usage of read-write locks is correct, however you do not gain much (anything?) in this situation. Read-write locks mostly provide benefits when there are multiple readers, as they allow all readers to read concurrently. – Michael Jul 10 '18 at 13:26
  • Re, "this operation call is not sharing any common data..." What you mean by that. The list itself _is_ data, and the list is shared. Are you saying that `performOperation()` does not _update_ any shared data? Are you saying that no two threads will ever update the same object? – Solomon Slow Jul 10 '18 at 13:31
  • yes @jameslarge performOperations() is thread safe. They never perform any update on list. Basically they just call some methods on the objects stored in the list. – Onki Jul 11 '18 at 05:03
  • @Michael performOperations() is thread safe. They never perform any operation on list. Basically they just call some methods on the objects stored in the list. As as it is not updating any shared data, I wanted to keep it non synchrnozied so that it can be called in parallel by all threads. – Onki Jul 11 '18 at 05:06

1 Answers1

0

ReadWriteLock is more efficient when a lot of reading occurs as synchronized will block everything. That said ReadWriteLock is more error prone. For example your example will actually end up in a deadlock, because everytime you invoke readWriteLock.writeLock() or readWriteLock.readLock() it will create a new instance and it will never be unlocked, causing it to end up in a dead lock. So you example should look more like:

class StaticClass{
    static List<SomeObjects>=new List<>();
    static final ReadWriteLock readWriteLock = new ReentrantReadWriteLock();
    static final Lock readLock = readWriteLock.readLock();
    static final Lock writeLock = readWriteLock.writeLock();

    static void performOperation()
    {
        readLock.lock();
        try {
            //call operation on all objects in the list
        } finally {
            // This ensures read lock is unlocked even when exception occurs
            readLock.unlock();
        }
    }

    static void updateList()
    {
        writeLock.lock();
        try {
            //update list, add, update or remove objects 
        } finally {
            // This ensures read lock is unlocked even when exception occurs
            writeLock.unlock();
        }
    }
}

Please note I also added the try/finally in here to avoid possible issues with exceptions. As you can see this is quite more work then the simple synchronized part.

Also there is a possible alternative CopyOnWriteArrayList. Which is thread safe and you don't have to use locks or synchronized keyword. It will impact your performance when there are a lot of writes to it.

Albert Bos
  • 2,012
  • 1
  • 15
  • 26
  • public ReentrantReadWriteLock.WriteLock writeLock() { return writerLock; } public ReentrantReadWriteLock.ReadLock readLock() { return readerLock; } – Onki Jul 11 '18 at 05:13
  • Now if you look at this implementations of getting the read write locks, it does not seem to be creating a new lock everytime. your finally idea is good but I guess having readLock and writeLock will not give any benefit – Onki Jul 11 '18 at 05:14